Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-907

Add tests for getBlockLocations and totalLoad metrics.

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.20.2
    • Fix Version/s: 0.20.2, 0.21.0
    • Component/s: namenode
    • Labels:
      None
    • Environment:

      This jira will add more tests for metrics reported by NameNode . numGetBlockLocations and totalLoad.

    • Hadoop Flags:
      Reviewed
    1. HDFS-907.patch
      4 kB
      Ravi Phulari
    2. HDFS-907.v2.patch
      5 kB
      Ravi Phulari
    3. HDFS-907-0.20.patch
      6 kB
      Ravi Phulari
    4. HDFS907s.patch
      6 kB
      Ravi Phulari
    5. HDFS-907v3.patch
      6 kB
      Ravi Phulari
    6. HDFS-907v3.patch
      6 kB
      Ravi Phulari

      Activity

      Transition Time In Source Status Execution Times Last Executer Last Execution Date
      Open Open Patch Available Patch Available
      13d 13m 1 Ravi Phulari 02/Feb/10 01:20
      Patch Available Patch Available Resolved Resolved
      3d 2h 48m 1 Konstantin Boudnik 05/Feb/10 04:08
      Resolved Resolved Closed Closed
      200d 16h 42m 1 Tom White 24/Aug/10 20:51
      Tom White made changes -
      Status Resolved [ 5 ] Closed [ 6 ]
      Tom White made changes -
      Fix Version/s 0.22.0 [ 12314241 ]
      Hide
      Hudson added a comment -

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

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

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

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

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

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

      Thanks for committing Cos.

      On 2/4/10 8:10 PM, "Konstantin Boudnik (JIRA)" <jira@apache.org> wrote:

      [ https://issues.apache.org/jira/browse/HDFS-907?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

      Konstantin Boudnik updated HDFS-907:
      ------------------------------------

      Resolution: Fixed
      Fix Version/s: 0.22.0
      0.21.0
      Hadoop Flags: [Reviewed]
      Status: Resolved (was: Patch Available)

      I've just committed this into two unreleased branches in trunk. Thank you Ravi.


      This message is automatically generated by JIRA.
      -
      You can reply to this email to add a comment to the issue online.

      Ravi

      Show
      Ravi Phulari added a comment - Thanks for committing Cos. On 2/4/10 8:10 PM, "Konstantin Boudnik (JIRA)" <jira@apache.org> wrote: [ https://issues.apache.org/jira/browse/HDFS-907?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Konstantin Boudnik updated HDFS-907 : ------------------------------------ Resolution: Fixed Fix Version/s: 0.22.0 0.21.0 Hadoop Flags: [Reviewed] Status: Resolved (was: Patch Available) I've just committed this into two unreleased branches in trunk. Thank you Ravi. – This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online. Ravi –
      Konstantin Boudnik made changes -
      Status Patch Available [ 10002 ] Resolved [ 5 ]
      Hadoop Flags [Reviewed]
      Fix Version/s 0.21.0 [ 12314046 ]
      Fix Version/s 0.22.0 [ 12314241 ]
      Resolution Fixed [ 1 ]
      Hide
      Konstantin Boudnik added a comment -

      I've just committed this into two unreleased branches in trunk. Thank you Ravi.

      Show
      Konstantin Boudnik added a comment - I've just committed this into two unreleased branches in trunk. Thank you Ravi.
      Ravi Phulari made changes -
      Attachment HDFS907s.patch [ 12434919 ]
      Hide
      Ravi Phulari added a comment -

      Attaching patch for Yahoo! distribution.

      Show
      Ravi Phulari added a comment - Attaching patch for Yahoo! distribution.
      Ravi Phulari made changes -
      Attachment HDFS-907-0.20.patch [ 12434585 ]
      Hide
      Ravi Phulari added a comment -

      Attaching patch for 0.20 branch.
      test-patch passed successfully , pasting test-patch result log.

      [exec] 
           [exec] 
           [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 3 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 Eclipse classpath. The patch retains Eclipse classpath integrity.
           [exec] 
           [exec] 
           [exec] 
           [exec] 
           [exec] ======================================================================
           [exec] ======================================================================
           [exec]     Finished build.
           [exec] ======================================================================
           [exec] ======================================================================
           [exec] 
           [exec] 
      
      BUILD SUCCESSFUL
      Total time: 22 minutes 27 seconds
      
      
      Show
      Ravi Phulari added a comment - Attaching patch for 0.20 branch. test-patch passed successfully , pasting test-patch result log. [exec] [exec] [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 3 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 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] [exec] [exec] [exec] ====================================================================== [exec] ====================================================================== [exec] Finished build. [exec] ====================================================================== [exec] ====================================================================== [exec] [exec] BUILD SUCCESSFUL Total time: 22 minutes 27 seconds
      Hide
      Ravi Phulari added a comment -

      Above test failures (core and contrib) are not related to patch. There is something terribly wrong going on with Patch reporting.
      I say so because *Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/testReport/* says that there are no test failures but above comment posts that there are failures.

      Show
      Ravi Phulari added a comment - Above test failures (core and contrib) are not related to patch. There is something terribly wrong going on with Patch reporting. I say so because *Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/testReport/* says that there are no test failures but above comment posts that there are failures.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12431853/HDFS-907v3.patch
      against trunk revision 903906.

      +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 failed contrib unit tests.

      Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/testReport/
      Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
      Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/artifact/trunk/build/test/checkstyle-errors.html
      Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/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/12431853/HDFS-907v3.patch against trunk revision 903906. +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 failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/109/console This message is automatically generated.
      Ravi Phulari made changes -
      Status Open [ 1 ] Patch Available [ 10002 ]
      Hide
      Konstantin Boudnik added a comment -

      +1 patch looks good. Since this is for 0.20 please run test-patch locally and post the results here.

      Show
      Konstantin Boudnik added a comment - +1 patch looks good. Since this is for 0.20 please run test-patch locally and post the results here.
      Ravi Phulari made changes -
      Attachment HDFS-907v3.patch [ 12431853 ]
      Hide
      Ravi Phulari added a comment -

      Good catch Cos. Duhhh lazy me .
      Attaching updated patch.

      Show
      Ravi Phulari added a comment - Good catch Cos. Duhhh lazy me . Attaching updated patch.
      Hide
      Konstantin Boudnik added a comment -

      More to go, Ravi

      +    // and previous interval are 0
      +    assertEquals(0,nnMetrics.numGetBlockLocations.getPreviousIntervalValue());
      +    assertEquals(0,nnMetrics.numGetBlockLocations.getCurrentIntervalValue());
      

      BTW, you don't need to track versions of your patches manually (e.g. v1, v2, etc.): simply keep attaching the file with the same name and JIRA will do the rest. No patches will be lost either.

      Show
      Konstantin Boudnik added a comment - More to go, Ravi + // and previous interval are 0 + assertEquals(0,nnMetrics.numGetBlockLocations.getPreviousIntervalValue()); + assertEquals(0,nnMetrics.numGetBlockLocations.getCurrentIntervalValue()); BTW, you don't need to track versions of your patches manually (e.g. v1, v2, etc.): simply keep attaching the file with the same name and JIRA will do the rest. No patches will be lost either.
      Ravi Phulari made changes -
      Attachment HDFS-907v3.patch [ 12431845 ]
      Hide
      Konstantin Boudnik added a comment -

      I think I like what I see. One more thing: please add some meaningful message to the assertion statements - it will easy the fault analysis effort.

      Show
      Konstantin Boudnik added a comment - I think I like what I see. One more thing: please add some meaningful message to the assertion statements - it will easy the fault analysis effort.
      Ravi Phulari made changes -
      Attachment HDFS-907.v2.patch [ 12431703 ]
      Hide
      Ravi Phulari added a comment -

      Cos, thanks for reviewing patch and useful comments and suggestions.
      Uploading new patch updated as per your suggestions.
      Thanks.

      Show
      Ravi Phulari added a comment - Cos, thanks for reviewing patch and useful comments and suggestions. Uploading new patch updated as per your suggestions. Thanks.
      Ravi Phulari made changes -
      Summary Add new metrics unit tests. Add tests for getBlockLocations and totalLoad metrics.
      Environment This jira will add more Unit tests for metrics reported by NameNode e.g - numGetBlockLocations . This jira will add more tests for metrics reported by NameNode . numGetBlockLocations and totalLoad.
      Priority Major [ 3 ] Minor [ 4 ]
      Hide
      Konstantin Boudnik added a comment -

      Oh, one more: the JIRA's title says "Add ... unit tests". These aren't unit tests. So please consider writing true unit tests, e.g. using Mockito, or change the title of the JIRA.

      Show
      Konstantin Boudnik added a comment - Oh, one more: the JIRA's title says "Add ... unit tests". These aren't unit tests. So please consider writing true unit tests, e.g. using Mockito, or change the title of the JIRA.
      Hide
      Konstantin Boudnik added a comment -

      A couple of comments:

      • method updateNNMetrics() sleeps for 1 second. The comment says that it happens according to dfs.replication.interval. Why don't you retrieve the actual value of this configuration parameter?
      • use some meaningful messages for the asserts
      • Thread.sleep() might throw InterruptedException which will terminate {{ updateNNMetrics()}}. Which in turn will terminate testGetBlockLocations(). Is it intended?
      • would suggest to call the test testGetBlockLocationsMetric()
      • all public methods have to have proper JavaDoc with all required tags. In this case @throws is missing

      And some nits:

      • identification suppose to be equal to 2 white spaces exactly
      • I think very last empty line is excessive
      • test method declaration doesn't have a white in throws Exception{
      • you have two empty lines modifications in
           }
        -  
        +
           private MiniDFSCluster cluster;
        

        and

           private FSNamesystem namesystem;
        -
        +  private NameNodeMetrics nnMetrics;
        +  private NameNode nn;
        +  
           private static Path getTestPath(String fileName) {
        
      Show
      Konstantin Boudnik added a comment - A couple of comments: method updateNNMetrics() sleeps for 1 second. The comment says that it happens according to dfs.replication.interval . Why don't you retrieve the actual value of this configuration parameter? use some meaningful messages for the asserts Thread.sleep() might throw InterruptedException which will terminate {{ updateNNMetrics()}}. Which in turn will terminate testGetBlockLocations() . Is it intended? would suggest to call the test testGetBlockLocationsMetric() all public methods have to have proper JavaDoc with all required tags. In this case @throws is missing And some nits: identification suppose to be equal to 2 white spaces exactly I think very last empty line is excessive test method declaration doesn't have a white in throws Exception{ you have two empty lines modifications in } - + private MiniDFSCluster cluster; and private FSNamesystem namesystem; - + private NameNodeMetrics nnMetrics; + private NameNode nn; + private static Path getTestPath(String fileName) {
      Ravi Phulari made changes -
      Fix Version/s 0.20.2 [ 12314204 ]
      Affects Version/s 0.20.2 [ 12314204 ]
      Hide
      Ravi Phulari added a comment -

      Attached first working patch.
      Will update this patch soon with tests for other NameNode metrics and fix for indentation errors.

      Show
      Ravi Phulari added a comment - Attached first working patch. Will update this patch soon with tests for other NameNode metrics and fix for indentation errors.
      Ravi Phulari made changes -
      Field Original Value New Value
      Attachment HDFS-907.patch [ 12430829 ]
      Ravi Phulari created issue -

        People

        • Assignee:
          Ravi Phulari
          Reporter:
          Ravi Phulari
        • Votes:
          0 Vote for this issue
          Watchers:
          2 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development