Hadoop Common
  1. Hadoop Common
  2. HADOOP-3177

Expose DFSOutputStream.fsync API though the FileSystem interface

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.18.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      Added a new public interface Syncable which declares the sync() operation. FSDataOutputStream implements Syncable. If the wrappedStream in FSDataOutputStream is Syncalbe, calling FSDataOutputStream.sync() is equivalent to call wrappedStream.sync(). Otherwise, FSDataOutputStream.sync() is a no-op. Both DistributedFileSystem and LocalFileSystem support the sync() operation.
      Show
      Added a new public interface Syncable which declares the sync() operation. FSDataOutputStream implements Syncable. If the wrappedStream in FSDataOutputStream is Syncalbe, calling FSDataOutputStream.sync() is equivalent to call wrappedStream.sync(). Otherwise, FSDataOutputStream.sync() is a no-op. Both DistributedFileSystem and LocalFileSystem support the sync() operation.

      Description

      In the current code, there is a DFSOutputStream.fsync() API that allows a client to flush all buffered data to the datanodes and also persist block locations on the namenode. This API should be exposed through the generic API in the org.hadoop.fs.

      1. 3177_20080603.patch
        9 kB
        Tsz Wo Nicholas Sze

        Issue Links

          Activity

          Hide
          dhruba borthakur added a comment -

          I just committed this. Thanks Nicholas!

          Show
          dhruba borthakur added a comment - I just committed this. Thanks Nicholas!
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12383320/3177_20080603.patch
          against trunk revision 662976.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12383320/3177_20080603.patch against trunk revision 662976. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2567/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12383320/3177_20080603.patch
          against trunk revision 662913.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified tests.

          -1 patch. The patch command could not apply the patch.

          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2561/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12383320/3177_20080603.patch against trunk revision 662913. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2561/console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12383320/3177_20080603.patch
          against trunk revision 662913.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 9 new or modified tests.

          -1 patch. The patch command could not apply the patch.

          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2558/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12383320/3177_20080603.patch against trunk revision 662913. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2558/console This message is automatically generated.
          Hide
          dhruba borthakur added a comment -

          +1. Code looks good.

          Show
          dhruba borthakur added a comment - +1. Code looks good.
          Hide
          Doug Cutting added a comment -

          This API looks fine to me.

          Show
          Doug Cutting added a comment - This API looks fine to me.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          3177_20080603.patch: added Syncable interface

          Show
          Tsz Wo Nicholas Sze added a comment - 3177_20080603.patch: added Syncable interface
          Hide
          Tsz Wo Nicholas Sze added a comment -

          > Furthermore FileOutputStream#getFD is final, as is FileDescriptor, so we can't use it here.

          Oops, I missed this point. We definitely cannot use FileDescriptor.

          I think the Syncable interface idea is good.

          Show
          Tsz Wo Nicholas Sze added a comment - > Furthermore FileOutputStream#getFD is final, as is FileDescriptor, so we can't use it here. Oops, I missed this point. We definitely cannot use FileDescriptor. I think the Syncable interface idea is good.
          Hide
          dhruba borthakur added a comment -

          The semantics of FileSystem.fsync is that the system makes every effort to put data on persistent storage. In the case of local file system, it will invoke the sync on the local file system. In the case of DFS, it will ensure that data has been flushed to OS buffers on all datanode(s) in the pipeline.

          If a file-system does not support fsync, then it should be a no-op rather than an exception.This is similar to other calls like setReplication which returns success on local file systems even though there isn't any replication for local file system.

          Show
          dhruba borthakur added a comment - The semantics of FileSystem.fsync is that the system makes every effort to put data on persistent storage. In the case of local file system, it will invoke the sync on the local file system. In the case of DFS, it will ensure that data has been flushed to OS buffers on all datanode(s) in the pipeline. If a file-system does not support fsync, then it should be a no-op rather than an exception.This is similar to other calls like setReplication which returns success on local file systems even though there isn't any replication for local file system.
          Hide
          Tom White added a comment -

          I think it should be java.io.FileOutputStream since we are doing FileSystem.

          But FileOutputStream is tied to Java's File abstraction which isn't general enough for Hadoop FileSystems. Furthermore FileOutputStream#getFD is final, as is FileDescriptor, so we can't use it here.

          How about an interface:

          public interface Syncable {
            void sync() throws IOException;
          }
          

          (Or should it be "Synchable"?) Then make DFSOutputStream implement Syncable, so FSDataOutputStream - which is also a Syncable - can see if it can call sync() on the underlying stream.

          What are the semantics of sync()? I think the expectation is that after sync returns the system has successfully sync'ed buffers to disk. So if this is not true, sync() should throw an exception. This is what java.io.FileDescriptor does. Using a subclass of IOException (java.io.SyncFailedException?) would make this easier for callers. I realize that this description is at odds with the current contract for DFSOutputStream#fsync, which doesn't guarantee that the data has been flushed to persistent storage, but I wondered whether DFSOutputStream could be strengthened to make this guarantee?

          If the FileSystem doesn't support sync then do we get an exception when calling sync(), or is it a no op?

          Show
          Tom White added a comment - I think it should be java.io.FileOutputStream since we are doing FileSystem. But FileOutputStream is tied to Java's File abstraction which isn't general enough for Hadoop FileSystems. Furthermore FileOutputStream#getFD is final, as is FileDescriptor, so we can't use it here. How about an interface: public interface Syncable { void sync() throws IOException; } (Or should it be "Synchable"?) Then make DFSOutputStream implement Syncable, so FSDataOutputStream - which is also a Syncable - can see if it can call sync() on the underlying stream. What are the semantics of sync()? I think the expectation is that after sync returns the system has successfully sync'ed buffers to disk. So if this is not true, sync() should throw an exception. This is what java.io.FileDescriptor does. Using a subclass of IOException (java.io.SyncFailedException?) would make this easier for callers. I realize that this description is at odds with the current contract for DFSOutputStream#fsync, which doesn't guarantee that the data has been flushed to persistent storage, but I wondered whether DFSOutputStream could be strengthened to make this guarantee? If the FileSystem doesn't support sync then do we get an exception when calling sync(), or is it a no op?
          Hide
          Tsz Wo Nicholas Sze added a comment -

          The class for the wrapperStream is OutputStream. I think it should be java.io.FileOutputStream since we are doing FileSystem. Then, the new method FSDataOutputStream.fsync() just has to call wrapperStream.getFD().sync(). This will work for all FileSystem

          For DFS, we need to define a new class, say DfsFileDescriptor, extending java.io.FileDescriptor and make DfsFileDescriptor.sync() calls DFSOutputStream.fsync().

          For other FileSystem subclass, if getFD() is not defined, we could throw IOException("not supported").

          Show
          Tsz Wo Nicholas Sze added a comment - The class for the wrapperStream is OutputStream. I think it should be java.io.FileOutputStream since we are doing FileSystem. Then, the new method FSDataOutputStream.fsync() just has to call wrapperStream.getFD().sync(). This will work for all FileSystem For DFS, we need to define a new class, say DfsFileDescriptor, extending java.io.FileDescriptor and make DfsFileDescriptor.sync() calls DFSOutputStream.fsync(). For other FileSystem subclass, if getFD() is not defined, we could throw IOException("not supported").
          Hide
          dhruba borthakur added a comment -

          I am saying the same thing as you: let's add a new method FSDataOutputStream.fsync().

          Show
          dhruba borthakur added a comment - I am saying the same thing as you: let's add a new method FSDataOutputStream.fsync().
          Hide
          Doug Cutting added a comment -

          > It would invoke reflection to see if the wrapperStream as has a method named "fsync".

          Yuk. What is the reason not to add a sync() method to FileSystem and/or FSDataOutputStream?

          Show
          Doug Cutting added a comment - > It would invoke reflection to see if the wrapperStream as has a method named "fsync". Yuk. What is the reason not to add a sync() method to FileSystem and/or FSDataOutputStream?
          Hide
          dhruba borthakur added a comment - - edited

          This issue blocks h-1700 because DFSOutputStream.fsync() is not a public API yet. More work is needed to make it accessible from an application.

          One option would be to introduce FSDataOutputStream.fsync() API. It would invoke reflection to see if the wrapperStream as has a method named "fsync". If so, then it will invoke wrapperStream.fsync(), otherwise it will invoke wrapperStream.flush(). Does this sound reasonable?

          Show
          dhruba borthakur added a comment - - edited This issue blocks h-1700 because DFSOutputStream.fsync() is not a public API yet. More work is needed to make it accessible from an application. One option would be to introduce FSDataOutputStream.fsync() API. It would invoke reflection to see if the wrapperStream as has a method named "fsync". If so, then it will invoke wrapperStream.fsync(), otherwise it will invoke wrapperStream.flush(). Does this sound reasonable?
          Hide
          Doug Cutting added a comment -

          I think sync is a fairly generic FileSystem operation.

          http://java.sun.com/javase/6/docs/api/java/io/FileDescriptor.html#sync()

          Show
          Doug Cutting added a comment - I think sync is a fairly generic FileSystem operation. http://java.sun.com/javase/6/docs/api/java/io/FileDescriptor.html#sync( )
          Hide
          dhruba borthakur added a comment -

          I think it is sufficient (for now) for an application to cast a OutputStream to a DFSOutputStream and then invoke fsync on it. However, it is better if we expose this API on the generic FileSystem API so that an application can work same on all FileSystems.

          Show
          dhruba borthakur added a comment - I think it is sufficient (for now) for an application to cast a OutputStream to a DFSOutputStream and then invoke fsync on it. However, it is better if we expose this API on the generic FileSystem API so that an application can work same on all FileSystems.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Do we really need to add this method to the FileSystem API? fsync seems DFS specific.

          Show
          Tsz Wo Nicholas Sze added a comment - Do we really need to add this method to the FileSystem API? fsync seems DFS specific.

            People

            • Assignee:
              Tsz Wo Nicholas Sze
              Reporter:
              dhruba borthakur
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development