Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-800

The last block of a file under construction may change to the COMPLETE state in response to getAdditionalBlock or completeFileInternal

    Details

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

      Description

      Currently the last block changes to be the COMMITTED state. However, if the block already has a valid finalizeded replica, it should be changed to be the COMPLETE state.

      1. HDFS-800.patch
        5 kB
        Hairong Kuang
      2. HDFS-800.patch
        6 kB
        Hairong Kuang

        Activity

        Hide
        Hairong Kuang added a comment -

        This is a patch for fixing the problem.

        With this patch, we could change the method FSNamesystem#checkFileProgress from counting live valid replicas to be checking if every block is a complete block.

        Show
        Hairong Kuang added a comment - This is a patch for fixing the problem. With this patch, we could change the method FSNamesystem#checkFileProgress from counting live valid replicas to be checking if every block is a complete block.
        Hide
        Wang Xu added a comment -

        Hello Hairong,

        In this patch, commitBlock() invokes commitBlock.getNumBytes() but did remove the null check, is this matter? Or it could make sure anyone (seems only commitOrCompleteLastBlock() calls it) calls this method will check it in advance.

        Show
        Wang Xu added a comment - Hello Hairong, In this patch, commitBlock() invokes commitBlock.getNumBytes() but did remove the null check, is this matter? Or it could make sure anyone (seems only commitOrCompleteLastBlock() calls it) calls this method will check it in advance.
        Hide
        Hairong Kuang added a comment -

        commitBlock is a private method and currently is called only by commitOrCompleteLastBlock. So I think that no checking is ok.

        Also this patch does not have a unit test. HDFS already has a unit test TestBlockUnderConstruction#testBlockCreation that verifies the state of each block of a file.

        Show
        Hairong Kuang added a comment - commitBlock is a private method and currently is called only by commitOrCompleteLastBlock. So I think that no checking is ok. Also this patch does not have a unit test. HDFS already has a unit test TestBlockUnderConstruction#testBlockCreation that verifies the state of each block of a file.
        Hide
        Konstantin Shvachko added a comment -

        In addStoredBlock() you COMPLETE block only if there is enough valid replicas. Otherwise it will still be in COMMITTED state. Therefore, we still need logic that will COMPLETE penultimate blocks after checking the file progress. I don't think you can remove it. Do I miss anything here?
        The same with closeFileInternal(). Both the last and the penultimate blocks can be in COMMITTED state, because commitOrCompleteLastBlock() does not guarantee that the block is completed.

        Show
        Konstantin Shvachko added a comment - In addStoredBlock() you COMPLETE block only if there is enough valid replicas. Otherwise it will still be in COMMITTED state. Therefore, we still need logic that will COMPLETE penultimate blocks after checking the file progress. I don't think you can remove it. Do I miss anything here? The same with closeFileInternal() . Both the last and the penultimate blocks can be in COMMITTED state, because commitOrCompleteLastBlock() does not guarantee that the block is completed.
        Hide
        Hairong Kuang added a comment -

        The idea is that a block may become COMPLETE in commitOrCompleteLastBlock. Otherwise, it will become COMPLETE in addStoredBlock. If a block is in COMMITTED state, check progress won't return true. That's why I suggest with my change, check progress does not need to count live replicas, instead it just check if each block is in COMPLETE state or not.

        Show
        Hairong Kuang added a comment - The idea is that a block may become COMPLETE in commitOrCompleteLastBlock. Otherwise, it will become COMPLETE in addStoredBlock. If a block is in COMMITTED state, check progress won't return true. That's why I suggest with my change, check progress does not need to count live replicas, instead it just check if each block is in COMPLETE state or not.
        Hide
        dhruba borthakur added a comment -

        Hi Hairong, can you pl explain (again) what causes a block to move the COMPLETE state and what else causes it to move to the COMPLETE state? Thanks.

        Show
        dhruba borthakur added a comment - Hi Hairong, can you pl explain (again) what causes a block to move the COMPLETE state and what else causes it to move to the COMPLETE state? Thanks.
        Hide
        Hairong Kuang added a comment -

        COMPLETE = COMMITTE + #(valid finalized replicas) meets the min_replication requirement. So there is two cases that a blocks becomes COMPLETE:
        1. When finalizing a block's length & GS (in getAdditionalBlock and completeFileInternal), if the block has already met the min replication, change it to be COMPLETE. This is the change that I propose in this jira. Current code changes it to only COMMITTED.
        2.When a block is in COMMITTED state and addStoredBlock finds that it meets the min replication, change its state to COMPLETE.

        Show
        Hairong Kuang added a comment - COMPLETE = COMMITTE + #(valid finalized replicas) meets the min_replication requirement. So there is two cases that a blocks becomes COMPLETE: 1. When finalizing a block's length & GS (in getAdditionalBlock and completeFileInternal), if the block has already met the min replication, change it to be COMPLETE. This is the change that I propose in this jira. Current code changes it to only COMMITTED. 2.When a block is in COMMITTED state and addStoredBlock finds that it meets the min replication, change its state to COMPLETE.
        Hide
        Hairong Kuang added a comment -

        Just want to make it clear. When I filed this jira, I thought it was a bug. But while I worked on this, I realize that my proposed change is more like an optimization. Current trunk does not do case 1. However if a block missed the state change in (1), the block's state will be changed to be COMPLETE at a later time after a check if its replicas# meets the min replication factor returns true.

        Show
        Hairong Kuang added a comment - Just want to make it clear. When I filed this jira, I thought it was a bug. But while I worked on this, I realize that my proposed change is more like an optimization. Current trunk does not do case 1. However if a block missed the state change in (1), the block's state will be changed to be COMPLETE at a later time after a check if its replicas# meets the min replication factor returns true.
        Hide
        Hairong Kuang added a comment -

        This new patch additionally changed checkFileProgress to check if a block is complete instead of checking if it meets the min replication requirement.

        Show
        Hairong Kuang added a comment - This new patch additionally changed checkFileProgress to check if a block is complete instead of checking if it meets the min replication requirement.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12430461/HDFS-800.patch
        against trunk revision 902232.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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/199/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/199/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/199/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/199/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/12430461/HDFS-800.patch against trunk revision 902232. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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/199/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/199/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/199/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/199/console This message is automatically generated.
        Hide
        Tsz Wo Nicholas Sze added a comment -
        java.lang.NoClassDefFoundError: org/apache/hadoop/conf/Configuration
        	at java.lang.Class.forName0(Native Method)
        	...
        

        Many tests failed with NoClassDefFoundError. The Hudson machine definitely has problems. We got similar failure in HDFS-630.

        Show
        Tsz Wo Nicholas Sze added a comment - java.lang.NoClassDefFoundError: org/apache/hadoop/conf/Configuration at java.lang.Class.forName0(Native Method) ... Many tests failed with NoClassDefFoundError. The Hudson machine definitely has problems. We got similar failure in HDFS-630 .
        Hide
        Hairong Kuang added a comment -

        All unit tests are passed on my local machine.
        BUILD SUCCESSFUL
        Total time: 49 minutes 14 seconds

        No unit test is included because HDFS already has a unit test TestBlockUnderConstruction#testBlockCreation that verifies the state of each block of a file.

        Show
        Hairong Kuang added a comment - All unit tests are passed on my local machine. BUILD SUCCESSFUL Total time: 49 minutes 14 seconds No unit test is included because HDFS already has a unit test TestBlockUnderConstruction#testBlockCreation that verifies the state of each block of a file.
        Hide
        Konstantin Shvachko added a comment -

        Your last change was critical for both understanding and improving the performance of this code.
        +1

        Show
        Konstantin Shvachko added a comment - Your last change was critical for both understanding and improving the performance of this code. +1
        Hide
        Hairong Kuang added a comment -

        I've just committed this.

        Show
        Hairong Kuang added a comment - I've just committed this.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #177 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/177/)
        . The last block of a file under construction may change to the COMPLETE state in response to getAdditionalBlock or completeFileInternal. Contributed by Hairong Kuang.

        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #177 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/177/ ) . The last block of a file under construction may change to the COMPLETE state in response to getAdditionalBlock or completeFileInternal. Contributed by Hairong Kuang.
        Hide
        Hudson added a comment -

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

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

        Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #101 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/101/)
        . The last block of a file under construction may change to the COMPLETE state in response to getAdditionalBlock or completeFileInternal. Contributed by Hairong Kuang.

        Show
        Hudson added a comment - Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #101 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/101/ ) . The last block of a file under construction may change to the COMPLETE state in response to getAdditionalBlock or completeFileInternal. Contributed by Hairong Kuang.
        Hide
        Hudson added a comment -

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

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

          People

          • Assignee:
            Hairong Kuang
            Reporter:
            Hairong Kuang
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development