Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-804

New unit tests for concurrent lease recovery

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.21.0, 0.22.0
    • Fix Version/s: 0.21.0
    • Component/s: test
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      FSNamesystem code which process concurrent lease recovery isn't tested. We need new test cases to cover these spots.

      1. HDFS-804.patch
        12 kB
        Konstantin Boudnik
      2. HDFS-804.patch
        12 kB
        Konstantin Boudnik
      3. HDFS-804.patch
        9 kB
        Konstantin Boudnik
      4. HDFS-804.patch
        6 kB
        Konstantin Boudnik

        Issue Links

          Activity

          Hide
          Konstantin Boudnik added a comment -

          The patch provides two new test cases to cover concurrent lease recovery code.

          Show
          Konstantin Boudnik added a comment - The patch provides two new test cases to cover concurrent lease recovery code.
          Hide
          Konstantin Boudnik added a comment -

          Seems to be ready for the verification.

          Show
          Konstantin Boudnik added a comment - Seems to be ready for the verification.
          Hide
          Konstantin Boudnik added a comment -

          Because test-patch process is untrustworthy (HDFS-794) I've ran local test-patch and unit tests

          There appear to be 118 release audit warnings before the patch and 118 release audit warnings after applying the patch.
          
          +1 overall.  
              +1 @author.  The patch does not contain any @author tags.
              +1 tests included.  The patch appears to include 3 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.
          
          % ant run-test-unit
          ...
              [junit] Running org.apache.hadoop.hdfs.server.namenode.TestNNLeaseRecovery
              [junit] Tests run: 6, Failures: 0, Errors: 0, Time elapsed: 2.055 sec
          
          checkfailure:
          
          BUILD SUCCESSFUL
          Total time: 25 seconds
          
          Show
          Konstantin Boudnik added a comment - Because test-patch process is untrustworthy ( HDFS-794 ) I've ran local test-patch and unit tests There appear to be 118 release audit warnings before the patch and 118 release audit warnings after applying the patch. +1 overall. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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. % ant run-test-unit ... [junit] Running org.apache.hadoop.hdfs.server.namenode.TestNNLeaseRecovery [junit] Tests run: 6, Failures: 0, Errors: 0, Time elapsed: 2.055 sec checkfailure: BUILD SUCCESSFUL Total time: 25 seconds
          Hide
          Hadoop QA added a comment -

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

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

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

          Test failure is unrelated.

          Show
          Konstantin Boudnik added a comment - Test failure is unrelated.
          Hide
          Hairong Kuang added a comment -

          For testing concurrent lease recoveries, I guess we need 3 test cases:
          1. If the last block is not UNDER_RECOVERY, somthing is wrong with the system. commitBlockSynchronization should throw an exception stating no block recovery is in progress.
          2. if the last block is UNDER_RECOVERY and
          2.a the last block recovery id is equal to the new reported recovery id, commitBlockSynchronization succeeds.
          2.b the last block recovery id is greater than the new reported recovery id, commitBlockSynchronization exits without making any change to the last block.
          2.c the last block recovery id is less than the new reported recovery id, something is wrong with the system, commitBlockSynchronization should throw an exception.

          Show
          Hairong Kuang added a comment - For testing concurrent lease recoveries, I guess we need 3 test cases: 1. If the last block is not UNDER_RECOVERY, somthing is wrong with the system. commitBlockSynchronization should throw an exception stating no block recovery is in progress. 2. if the last block is UNDER_RECOVERY and 2.a the last block recovery id is equal to the new reported recovery id, commitBlockSynchronization succeeds. 2.b the last block recovery id is greater than the new reported recovery id, commitBlockSynchronization exits without making any change to the last block. 2.c the last block recovery id is less than the new reported recovery id, something is wrong with the system, commitBlockSynchronization should throw an exception.
          Hide
          Konstantin Boudnik added a comment -

          Addressing Hairong's comments.

          Show
          Konstantin Boudnik added a comment - Addressing Hairong's comments.
          Hide
          Hairong Kuang added a comment -

          The new unit tests look good to me. Cos, could you please add two more tests: (1) lease recovery on a file with zero block; (1) lease recovery a file with 1 block? Sorry for the late new request. Hope it is not a lot of work.

          Show
          Hairong Kuang added a comment - The new unit tests look good to me. Cos, could you please add two more tests: (1) lease recovery on a file with zero block; (1) lease recovery a file with 1 block? Sorry for the late new request. Hope it is not a lot of work.
          Hide
          Konstantin Boudnik added a comment -

          Thanks for the review, Hairong. It shouldn't be difficult to add two more test cases. However, I'd expect that in case of 0 blocks FSNamesystem will crap out in the line 1934

                curBlock = blocks[nrCompleteBlocks];
          

          with ArrayIndexOutOfBoundException. Do you think we need this test until the code is fixed?

          Show
          Konstantin Boudnik added a comment - Thanks for the review, Hairong. It shouldn't be difficult to add two more test cases. However, I'd expect that in case of 0 blocks FSNamesystem will crap out in the line 1934 curBlock = blocks[nrCompleteBlocks]; with ArrayIndexOutOfBoundException . Do you think we need this test until the code is fixed?
          Hide
          Hairong Kuang added a comment -

          > Do you think we need this test until the code is fixed
          Yes, please file a jira on this. Thanks a lot for addressing my new request.

          Show
          Hairong Kuang added a comment - > Do you think we need this test until the code is fixed Yes, please file a jira on this. Thanks a lot for addressing my new request.
          Hide
          Konstantin Boudnik added a comment -

          This patch includes two more test cases according to Hairong's comment. However, the patch can't be committed at the moment because one of the tests will fail (blocked by HDFS-812). After that issue it resolved, the patch needs to be revisited to make sure testInternalReleaseLease_0blocks has a correct contract.

          Show
          Konstantin Boudnik added a comment - This patch includes two more test cases according to Hairong's comment. However, the patch can't be committed at the moment because one of the tests will fail (blocked by HDFS-812 ). After that issue it resolved, the patch needs to be revisited to make sure testInternalReleaseLease_0blocks has a correct contract.
          Hide
          Hairong Kuang added a comment -

          Cos, alternatively you could commit the patch uploaded 12/04 to this jira first. Then attach the new tests to HDFS-812. Thanks.

          Show
          Hairong Kuang added a comment - Cos, alternatively you could commit the patch uploaded 12/04 to this jira first. Then attach the new tests to HDFS-812 . Thanks.
          Hide
          Konstantin Boudnik added a comment -

          This new version has all new test cases but one affected by HDFS-812. I'll create a separate patch for that JIRA.

          Thanks you Hairong. I'm going to commit this as soon as the formal +1 is received

          Show
          Konstantin Boudnik added a comment - This new version has all new test cases but one affected by HDFS-812 . I'll create a separate patch for that JIRA. Thanks you Hairong. I'm going to commit this as soon as the formal +1 is received
          Hide
          Hairong Kuang added a comment -

          +1

          Show
          Hairong Kuang added a comment - +1
          Hide
          Konstantin Boudnik added a comment -

          I've just committed it and merged back to branch-0.21.

          Show
          Konstantin Boudnik added a comment - I've just committed it and merged back to branch-0.21.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #131 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/131/)
          . New unit tests for concurrent lease recovery. Contributed by Konstantin Boudnik

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #131 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/131/ ) . New unit tests for concurrent lease recovery. Contributed by Konstantin Boudnik
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #135 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/135/)
          . New unit tests for concurrent lease recovery. Contributed by Konstantin Boudnik

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #135 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/135/ ) . New unit tests for concurrent lease recovery. Contributed by Konstantin Boudnik
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #164 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/164/)
          . New unit tests for concurrent lease recovery. Contributed by Konstantin Boudnik

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #164 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/164/ ) . New unit tests for concurrent lease recovery. Contributed by Konstantin Boudnik
          Hide
          Hudson added a comment -

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

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

            People

            • Assignee:
              Konstantin Boudnik
              Reporter:
              Konstantin Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development