Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-654

HDFS needs to support new rename introduced for FileContext

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      New rename functionality with different semantics to overwrite the existing destination was introduced for use in FileContext. Currently the default implementation in FileSystem is not atomic. This change implements atomic rename operation for use by FileContext.

      1. hdfs-654.9.patch
        51 kB
        Suresh Srinivas
      2. hdfs-654.7.patch
        49 kB
        Suresh Srinivas
      3. hdfs-654.5.patch
        41 kB
        Suresh Srinivas
      4. hdfs-654.5.patch
        41 kB
        Suresh Srinivas
      5. hdfs-654.3.patch
        38 kB
        Suresh Srinivas
      6. hdfs-654.2.patch
        35 kB
        Suresh Srinivas
      7. hdfs-654.1.patch
        35 kB
        Suresh Srinivas
      8. HDFS-654.patch
        35 kB
        Suresh Srinivas

        Issue Links

          Activity

          Hide
          Suresh Srinivas added a comment -

          New patch.

          Show
          Suresh Srinivas added a comment - New patch.
          Hide
          dhruba borthakur added a comment -

          Would appreciate it a lot if you can please explain in short what the new semantics would be. what happens if the destination directory exists? what if the destination directory is non-empty? what if the destination is already a file.

          Show
          dhruba borthakur added a comment - Would appreciate it a lot if you can please explain in short what the new semantics would be. what happens if the destination directory exists? what if the destination directory is non-empty? what if the destination is already a file.
          Hide
          dhruba borthakur added a comment -

          Maybe I missed something, but does this do the right accounting for calculating the total number of inodes in the filesystem(to ensure correct working of namespace quotas)?

          Show
          dhruba borthakur added a comment - Maybe I missed something, but does this do the right accounting for calculating the total number of inodes in the filesystem(to ensure correct working of namespace quotas)?
          Hide
          Suresh Srinivas added a comment -

          Dhruba this is as per the discussion in HADOOP-6240. This introduced a non atomic new Rename, as described in FileSystem#rename(String, String, Rename...). This change is overrides the default implementation. In addition to new semantics, rename will be atomic.

          Maybe I missed something, but does this do the right accounting for calculating the total number of inodes in the filesystem(to ensure correct working of namespace quotas)?

          can you be more elaborate? I did not understand the concern.

          Show
          Suresh Srinivas added a comment - Dhruba this is as per the discussion in HADOOP-6240 . This introduced a non atomic new Rename, as described in FileSystem#rename(String, String, Rename...). This change is overrides the default implementation. In addition to new semantics, rename will be atomic. Maybe I missed something, but does this do the right accounting for calculating the total number of inodes in the filesystem(to ensure correct working of namespace quotas)? can you be more elaborate? I did not understand the concern.
          Hide
          dhruba borthakur added a comment -

          Thanks for the explanation. I was referring to the check in FSNameSystem.checkFsObjectLimit() that checks the total number of inodes in the FS. I was worried that you allow replacing a non-empty directory with an empty directory in which case the totalInode in the FS has to be adjusted accordingly.

          Show
          dhruba borthakur added a comment - Thanks for the explanation. I was referring to the check in FSNameSystem.checkFsObjectLimit() that checks the total number of inodes in the FS. I was worried that you allow replacing a non-empty directory with an empty directory in which case the totalInode in the FS has to be adjusted accordingly.
          Hide
          Suresh Srinivas added a comment -

          Attaching new patch for the latest trunk that includes append changes

          Show
          Suresh Srinivas added a comment - Attaching new patch for the latest trunk that includes append changes
          Hide
          Konstantin Shvachko added a comment -
          1. Parameter type Options.Rename would look better in rename() methods.
          2. Increment ClientProtocol version.
          3. import warning in FSDirectory.
          4. Decrement LAYOUT_VERSION as you introduce new journal record.
          5. I think that the whole stack of old rename() methods should be deprecated starting from DistributedFileSystem.rename(src, dst) down to FSDirectory.unprotectedRenameTo(). Because otherwise people writing tests or pure HDFS applications will not know which rename to use, and may by mistake use the one with the old semantics, which is not desirable. It will also clarify why you did not reuse code for 2 renames, but rather built a whole new branch of methods for the new semantics.
          6. It is better to put comments related to deprecation in JavaDoc under /** @deprecated **/ section.
          7. FSEditLog.OP_RENAME should be used for the new semantics. The old rename operation should be called FSEditLog.OP_RENAME_OLD so that we could remove it later.
          8. You should probably verify that old layout images do not contain new rename op.
          Show
          Konstantin Shvachko added a comment - Parameter type Options.Rename would look better in rename() methods. Increment ClientProtocol version. import warning in FSDirectory. Decrement LAYOUT_VERSION as you introduce new journal record. I think that the whole stack of old rename() methods should be deprecated starting from DistributedFileSystem.rename(src, dst) down to FSDirectory.unprotectedRenameTo() . Because otherwise people writing tests or pure HDFS applications will not know which rename to use, and may by mistake use the one with the old semantics, which is not desirable. It will also clarify why you did not reuse code for 2 renames, but rather built a whole new branch of methods for the new semantics. It is better to put comments related to deprecation in JavaDoc under /** @deprecated **/ section. FSEditLog.OP_RENAME should be used for the new semantics. The old rename operation should be called FSEditLog.OP_RENAME_OLD so that we could remove it later. You should probably verify that old layout images do not contain new rename op.
          Hide
          Konstantin Shvachko added a comment -

          > FSNameSystem.checkFsObjectLimit()

          I don't see that we check the total object limit in the old rename() and we don't do it in the new one. Should we?
          To me rename does not change the total number of objects in the system if it is atomic. So probably we should not.
          The directory quotes should be updated though. I think it is taken care of. Suresh could you please confirm this.

          Show
          Konstantin Shvachko added a comment - > FSNameSystem.checkFsObjectLimit() I don't see that we check the total object limit in the old rename() and we don't do it in the new one. Should we? To me rename does not change the total number of objects in the system if it is atomic. So probably we should not. The directory quotes should be updated though. I think it is taken care of. Suresh could you please confirm this.
          Hide
          Suresh Srinivas added a comment -

          > FSNameSystem.checkFsObjectLimit()
          The count changes when the destination is removed. FSDirectory.removeChild(dstInode) and FSNamesystem.removePathAndBlocks() decrements the total INode count and the number of blocks. Also the lease to the removed destination is also removed.

          Show
          Suresh Srinivas added a comment - > FSNameSystem.checkFsObjectLimit() The count changes when the destination is removed. FSDirectory.removeChild(dstInode) and FSNamesystem.removePathAndBlocks() decrements the total INode count and the number of blocks. Also the lease to the removed destination is also removed.
          Hide
          Suresh Srinivas added a comment -

          New patch incorporates all the comments from Konstantin, except deprecation. After hudson tests to confirm the patch is good, I will attach another patch with old Rename deprecated. This will result in warnings. These need to be ignored to submit the patch.

          Show
          Suresh Srinivas added a comment - New patch incorporates all the comments from Konstantin, except deprecation. After hudson tests to confirm the patch is good, I will attach another patch with old Rename deprecated. This will result in warnings. These need to be ignored to submit the patch.
          Hide
          Suresh Srinivas added a comment -

          New patch with deprecated methods as suggested by Konstantin. In three places deprecated warnings are suppressed. First one in FSEditLog which calls deprecated rename to replay a change recorded in the edits. Two others are in tests.

          Show
          Suresh Srinivas added a comment - New patch with deprecated methods as suggested by Konstantin. In three places deprecated warnings are suppressed. First one in FSEditLog which calls deprecated rename to replay a change recorded in the edits. Two others are in tests.
          Hide
          Suresh Srinivas added a comment -

          Latest patch passes all the tests. Here is the test-patch result:
          [exec] +1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] +1 tests included. The patch appears to include 16 new or modified tests.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

          Show
          Suresh Srinivas added a comment - Latest patch passes all the tests. Here is the test-patch result: [exec] +1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 16 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12421387/hdfs-654.5.patch
          against trunk revision 822153.

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

          +1 tests included. The patch appears to include 16 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 failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/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/12421387/hdfs-654.5.patch against trunk revision 822153. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 16 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 failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/56/console This message is automatically generated.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Talked to Jakob: We have to update TestOfflineImageViewer when FSConstants.LAYOUT_VERSION is changed.

          Show
          Tsz Wo Nicholas Sze added a comment - Talked to Jakob: We have to update TestOfflineImageViewer when FSConstants.LAYOUT_VERSION is changed.
          Hide
          Suresh Srinivas added a comment -

          New patch has the following additional changes:

          1. Made changes to fix the rename issue in HDFS-677. Before removing src and dst during rename quota is verified. On any failure adding src and dst back is done without checking for quota.
          2. RenameAspects.aj which interecepted the calls to FSDirectory.addChild() now intercepts FSDirectory.addChildNoQuotaCheck(). Instead of checked QuotaExceedsException, which cannot be thrown since addChildNoQuotaCheck() does not throw exception, RuntimeException is thrown.
          3. Addressed the comment from Nicholas.
          Show
          Suresh Srinivas added a comment - New patch has the following additional changes: Made changes to fix the rename issue in HDFS-677 . Before removing src and dst during rename quota is verified. On any failure adding src and dst back is done without checking for quota. RenameAspects.aj which interecepted the calls to FSDirectory.addChild() now intercepts FSDirectory.addChildNoQuotaCheck(). Instead of checked QuotaExceedsException, which cannot be thrown since addChildNoQuotaCheck() does not throw exception, RuntimeException is thrown. Addressed the comment from 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/12422588/hdfs-654.7.patch
          against trunk revision 826905.

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

          +1 tests included. The patch appears to include 25 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 failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/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/12422588/hdfs-654.7.patch against trunk revision 826905. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 25 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 failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/40/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/12422588/hdfs-654.7.patch
          against trunk revision 826905.

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

          +1 tests included. The patch appears to include 25 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 failed core unit tests.

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/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/12422588/hdfs-654.7.patch against trunk revision 826905. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 25 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 failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/46/console This message is automatically generated.
          Hide
          Suresh Srinivas added a comment -

          Failed test TestFileAppend2.testComplexAppend is unrelated to this change.

          Show
          Suresh Srinivas added a comment - Failed test TestFileAppend2.testComplexAppend is unrelated to this change.
          Hide
          Konstantin Shvachko added a comment -
          1. FSNamesystem.renameTo() should not be public.
          2. format new methods to fit 80 symbol boundary.
          3. In the new FSDirectory.unprotectedRenameTo() if collectSubtreeBlocksAndClear() throws a RuntimeException in line 649, then frinalized section will try to put removedDst back in the treeeven though there the renamedSrc is already there. I propose to move the assignment "removedDst = null;" three lines up.
          4. In FSEditLog.loadEditRecords() you should follow the same naming convention with statistical variables numOpRename as with the operation constants:
            numOpRename should count new renames, and numOpRenameOld - the old ones.
            This is only a log message, but it is going to be confusing otherwise.
          5. Is OIV will be able to read new rename edit log records? Just adding the new supported version probably does not fix the reading problem by itself.
          Show
          Konstantin Shvachko added a comment - FSNamesystem.renameTo() should not be public. format new methods to fit 80 symbol boundary. In the new FSDirectory.unprotectedRenameTo() if collectSubtreeBlocksAndClear() throws a RuntimeException in line 649, then frinalized section will try to put removedDst back in the treeeven though there the renamedSrc is already there. I propose to move the assignment " removedDst = null; " three lines up. In FSEditLog.loadEditRecords() you should follow the same naming convention with statistical variables numOpRename as with the operation constants: numOpRename should count new renames, and numOpRenameOld - the old ones. This is only a log message, but it is going to be confusing otherwise. Is OIV will be able to read new rename edit log records? Just adding the new supported version probably does not fix the reading problem by itself.
          Hide
          Suresh Srinivas added a comment -

          I have made all the changes suggested. Here are my comments:
          > 1. FSNamesystem.renameTo() should not be public.
          Currently all methods called from NameNode are public. I followed the same pattern. I have change renameTo methods both old and new to package private.

          > 5. Is OIV will be able to read new rename edit log records? Just adding the new supported version probably does not fix the reading problem by itself.
          Per Jakob, the OIV reads FSImage only and not edit log. It needs to know all the supported versions. Without updating it, it will not load the FSImage with the new version.

          Show
          Suresh Srinivas added a comment - I have made all the changes suggested. Here are my comments: > 1. FSNamesystem.renameTo() should not be public. Currently all methods called from NameNode are public. I followed the same pattern. I have change renameTo methods both old and new to package private. > 5. Is OIV will be able to read new rename edit log records? Just adding the new supported version probably does not fix the reading problem by itself. Per Jakob, the OIV reads FSImage only and not edit log. It needs to know all the supported versions. Without updating it, it will not load the FSImage with the new version.
          Hide
          Suresh Srinivas added a comment -

          New patch.

          Show
          Suresh Srinivas added a comment - New patch.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12423032/hdfs-654.9.patch
          against trunk revision 828926.

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

          +1 tests included. The patch appears to include 25 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/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/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/12423032/hdfs-654.9.patch against trunk revision 828926. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 25 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/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/56/console This message is automatically generated.
          Hide
          Konstantin Shvachko added a comment -

          That is right OIV does not care about edits log.
          +1 patch looks great.

          Show
          Konstantin Shvachko added a comment - That is right OIV does not care about edits log. +1 patch looks great.
          Hide
          Suresh Srinivas added a comment -

          Committed the patch to trunk.

          Show
          Suresh Srinivas added a comment - Committed the patch to trunk.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #82 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/82/)
          . Add support new atomic rename functionality in HDFS for supporting rename in FileContext. Contributed by Suresh Srinivas.

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #82 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/82/ ) . Add support new atomic rename functionality in HDFS for supporting rename in FileContext. Contributed by Suresh Srinivas.
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #59 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/59/)

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #59 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/59/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #120 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/120/)

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #120 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/120/ )
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #78 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/78/)

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #78 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/78/ )
          Hide
          Uma Maheswara Rao G added a comment -

          The count changes when the destination is removed. FSDirectory.removeChild(dstInode) and FSNamesystem.removePathAndBlocks() decrements the total INode count and the number of blocks. Also the lease to the removed destination is also removed.

          Here in new rename api, we are removing the blocks and adding to invalidates. We did not synced the edit log before adding to invalidates. This can leads to miss the blocks, as i explained the scenario in HDFS-2815.
          I did not verify this yet. Will file a separate JIRA, once i confirm this as a bug.

          Show
          Uma Maheswara Rao G added a comment - The count changes when the destination is removed. FSDirectory.removeChild(dstInode) and FSNamesystem.removePathAndBlocks() decrements the total INode count and the number of blocks. Also the lease to the removed destination is also removed. Here in new rename api, we are removing the blocks and adding to invalidates. We did not synced the edit log before adding to invalidates. This can leads to miss the blocks, as i explained the scenario in HDFS-2815 . I did not verify this yet. Will file a separate JIRA, once i confirm this as a bug.
          Hide
          Uma Maheswara Rao G added a comment -

          Just verified the above specified point, Should not be a problem because renameInternal already help the lock on FsNameSystem. Also it is syncing the edits once inside the FSDirectory#renameTo itself synced once. Until FSNameSystem lock released anyway block deletion work won't be processed. This scenario should be fine for rename. Also reviewed all other APIs considering crash scenario. Except delete api, all other apis are fine. Thanks.

          Show
          Uma Maheswara Rao G added a comment - Just verified the above specified point, Should not be a problem because renameInternal already help the lock on FsNameSystem. Also it is syncing the edits once inside the FSDirectory#renameTo itself synced once. Until FSNameSystem lock released anyway block deletion work won't be processed. This scenario should be fine for rename. Also reviewed all other APIs considering crash scenario. Except delete api, all other apis are fine. Thanks.
          Hide
          Uma Maheswara Rao G added a comment -

          Should not be a problem because renameInternal already help the lock on FsNameSystem. Also it is syncing the edits once inside the FSDirectory#renameTo itself synced once. Until FSNameSystem lock released anyway block deletion work won't be processed. This scenario should be fine for rename.

          Please ignore this comment above. mis-read the log entry and logSync call.

          Issue exist here. Just filed the JIRA HDFS-2975

          Show
          Uma Maheswara Rao G added a comment - Should not be a problem because renameInternal already help the lock on FsNameSystem. Also it is syncing the edits once inside the FSDirectory#renameTo itself synced once. Until FSNameSystem lock released anyway block deletion work won't be processed. This scenario should be fine for rename. Please ignore this comment above. mis-read the log entry and logSync call. Issue exist here. Just filed the JIRA HDFS-2975

            People

            • Assignee:
              Suresh Srinivas
              Reporter:
              Suresh Srinivas
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development