Details

    • Type: Test Test
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: datanode, hdfs-client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Need to add more unit tests for access token

      1. AccessTokenTestPlan.pdf
        69 kB
        Kan Zhang
      2. h409-23.patch
        37 kB
        Kan Zhang
      3. h409-25.patch
        38 kB
        Kan Zhang
      4. h409-27.patch
        38 kB
        Kan Zhang
      5. h409-29.patch
        38 kB
        Kan Zhang
      6. HDFS-409-0_20.4.patch
        39 kB
        Jitendra Nath Pandey

        Issue Links

          Activity

          Hide
          Jitendra Nath Pandey added a comment -

          patch for hadoop 20 added.

          Show
          Jitendra Nath Pandey added a comment - patch for hadoop 20 added.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #48 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/48/)
          . Add more access token tests. Contributed by Kan Zhang

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #48 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/48/ ) . Add more access token tests. Contributed by Kan Zhang
          Hide
          Tsz Wo Nicholas Sze added a comment -

          > No bug fix and no change of semantics ...
          That's great. I was not sure which section in CHANGES.txt should this belongs to when I tried to commit this yesterday.

          I have committed this. Thanks, Kan.

          Show
          Tsz Wo Nicholas Sze added a comment - > No bug fix and no change of semantics ... That's great. I was not sure which section in CHANGES.txt should this belongs to when I tried to commit this yesterday. I have committed this. Thanks, Kan.
          Hide
          Kan Zhang added a comment -

          > What are the changes about? Is there any bug fix?
          No bug fix and no change of semantics to DFSClient and DataXceiver. 2 package private methods are added to DFSClient to read access token for testing purposes. The rest of the changes are simply changes to logging messages.

          Show
          Kan Zhang added a comment - > What are the changes about? Is there any bug fix? No bug fix and no change of semantics to DFSClient and DataXceiver. 2 package private methods are added to DFSClient to read access token for testing purposes. The rest of the changes are simply changes to logging messages.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          The patch, which adds unit tests, also changes some codes in DFSClient and DataXceiver. What are the changes about? Is there any bug fix?

          Show
          Tsz Wo Nicholas Sze added a comment - The patch, which adds unit tests, also changes some codes in DFSClient and DataXceiver. What are the changes about? Is there any bug fix?
          Hide
          Suresh Srinivas added a comment -

          Failure of TestNameNodeMetrics is not related to this change. I have created HDFS-540 to address the problem.

          Show
          Suresh Srinivas added a comment - Failure of TestNameNodeMetrics is not related to this change. I have created HDFS-540 to address the problem.
          Hide
          Kan Zhang added a comment -

          > I am not sure about TestNameNodeMetrics.
          I took a look and as far as I can tell, it was unrelated. I also ran the test a few times on my local box, they all passed.

          Show
          Kan Zhang added a comment - > I am not sure about TestNameNodeMetrics. I took a look and as far as I can tell, it was unrelated. I also ran the test a few times on my local box, they all passed.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Many tests failed in Hudson but most of them are related to HDFS-534 and HADOOP-6188 but not this issue. I am not sure about TestNameNodeMetrics. Could you check it, Kan?

          Show
          Tsz Wo Nicholas Sze added a comment - Many tests failed in Hudson but most of them are related to HDFS-534 and HADOOP-6188 but not this issue. I am not sure about TestNameNodeMetrics. Could you check it, Kan?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12416147/h409-29.patch
          against trunk revision 802972.

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

          +1 tests included. The patch appears to include 15 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-vesta.apache.org/55/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/55/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/55/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/55/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/12416147/h409-29.patch against trunk revision 802972. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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-vesta.apache.org/55/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/55/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/55/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/55/console This message is automatically generated.
          Hide
          Kan Zhang added a comment -

          ran hdfs unit tests and all passed except one test (TestHDFSTrash), which was unrelated. 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 15 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
          Kan Zhang added a comment - ran hdfs unit tests and all passed except one test (TestHDFSTrash), which was unrelated. 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 15 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
          Jakob Homan added a comment -

          Looks good. +1.

          Show
          Jakob Homan added a comment - Looks good. +1.
          Hide
          Jakob Homan added a comment -

          Looks much better. The separate shouldWait method really improves the code readability, thanks for that. My only remaining concern is that within the sleep section, an Exception is caught rather than an InterruptedException, which will swallow any problems that occur. Once this is corrected, it should be good to go.

          Show
          Jakob Homan added a comment - Looks much better. The separate shouldWait method really improves the code readability, thanks for that. My only remaining concern is that within the sleep section, an Exception is caught rather than an InterruptedException , which will swallow any problems that occur. Once this is corrected, it should be good to go.
          Hide
          Kan Zhang added a comment -

          re-run Hudson

          Show
          Kan Zhang added a comment - re-run Hudson
          Hide
          Kan Zhang added a comment -

          new patch fixing the javac warning

          Show
          Kan Zhang added a comment - new patch fixing the javac warning
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12415913/h409-25.patch
          against trunk revision 802264.

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

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

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

          -1 javac. The applied patch generated 96 javac compiler warnings (more than the trunk's current 95 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-vesta.apache.org/52/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/52/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/52/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/52/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/12415913/h409-25.patch against trunk revision 802264. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 96 javac compiler warnings (more than the trunk's current 95 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-vesta.apache.org/52/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/52/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/52/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/52/console This message is automatically generated.
          Hide
          Kan Zhang added a comment -

          uploading a new patch with added javadoc. I didn't break waitActive() into 2 loops since that would add one more RPC call. Instead, I refactored the wait logic into a separate method, which should improve readability and also get rid of any redundant checking.

          Show
          Kan Zhang added a comment - uploading a new patch with added javadoc. I didn't break waitActive() into 2 loops since that would add one more RPC call. Instead, I refactored the wait logic into a separate method, which should improve readability and also get rid of any redundant checking.
          Hide
          Jakob Homan added a comment -

          Reviewed.

          • restartDataNode(DataNodeProperties dnprop, boolean keepPort) needs javadoc
          • I think the changes to MiniDFSCluster::waitActive could be improved by creating two loops, one for dn registration and one for dn heartbeats. You'd have the same number of RPC calls, but improve the number of times through the loops, as well as making the code easier to understand.

          Otherwise looks good. I really like the heavy documentation in the unit tests. Looks faithful to the test plan.

          Show
          Jakob Homan added a comment - Reviewed. restartDataNode(DataNodeProperties dnprop, boolean keepPort) needs javadoc I think the changes to MiniDFSCluster::waitActive could be improved by creating two loops, one for dn registration and one for dn heartbeats. You'd have the same number of RPC calls, but improve the number of times through the loops, as well as making the code easier to understand. Otherwise looks good. I really like the heavy documentation in the unit tests. Looks faithful to the test plan.
          Hide
          Kan Zhang added a comment -

          This patch depends on HADOOP-6176.

          Show
          Kan Zhang added a comment - This patch depends on HADOOP-6176 .
          Hide
          Kan Zhang added a comment -

          adding a patch that implements the attached test plan

          Show
          Kan Zhang added a comment - adding a patch that implements the attached test plan

            People

            • Assignee:
              Kan Zhang
              Reporter:
              Kan Zhang
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development