Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.24.0, 0.23.1
    • Fix Version/s: 0.24.0, 0.23.1
    • Component/s: datanode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      BPOfferService is currently rather difficult to follow and not really commented. This JIRA is to clean up the code a bit, add javadocs/comments where necessary, and improve the formatting of the log messages.

      1. hdfs-2563.txt
        28 kB
        Todd Lipcon
      2. hdfs-2563.txt
        29 kB
        Todd Lipcon

        Activity

        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #902 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/902/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #902 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/902/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-0.23-Build #98 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/98/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Build #98 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Build/98/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #81 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/81/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #81 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/81/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #868 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/868/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #868 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/868/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #1311 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1311/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #1311 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/1311/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-0.23-Commit #183 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/183/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-0.23-Commit #183 (See https://builds.apache.org/job/Hadoop-Common-0.23-Commit/183/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Commit #182 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/182/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Commit #182 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Commit/182/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #1359 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1359/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #1359 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/1359/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #1285 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1285/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #1285 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/1285/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203943 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-0.23-Commit #195 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/195/)
        HDFS-2563. Some cleanup in BPOfferService. Contributed by Todd Lipcon.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-0.23-Commit #195 (See https://builds.apache.org/job/Hadoop-Mapreduce-0.23-Commit/195/ ) HDFS-2563 . Some cleanup in BPOfferService. Contributed by Todd Lipcon. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1203942 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/DataNodeTestUtils.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDatanodeRegister.java
        Hide
        Todd Lipcon added a comment -

        Committed for 23 and trunk, thanks Eli.

        Show
        Todd Lipcon added a comment - Committed for 23 and trunk, thanks Eli.
        Hide
        Eli Collins added a comment -

        +1 lgtm

        Nit: lines 846 and 1267 can fold onto the previous line

        Show
        Eli Collins added a comment - +1 lgtm Nit: lines 846 and 1267 can fold onto the previous line
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12504269/hdfs-2563.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 6 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 (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hdfs.TestDfsOverAvroRpc

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1571//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1571//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/12504269/hdfs-2563.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hdfs.TestDfsOverAvroRpc +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1571//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1571//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        The previous version of the patch had an issue where the block pool wasn't getting registered in blockPoolManager early enough. New rev cleans this up, and the tests that failed seem to pass now locally.

        Also fixed a few of the log messages which were including the nnAddr twice (once as part of toString() and once in the log message)

        Show
        Todd Lipcon added a comment - The previous version of the patch had an issue where the block pool wasn't getting registered in blockPoolManager early enough. New rev cleans this up, and the tests that failed seem to pass now locally. Also fixed a few of the log messages which were including the nnAddr twice (once as part of toString() and once in the log message)
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12504188/hdfs-2563.txt
        against trunk revision .

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

        +1 tests included. The patch appears to include 6 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 (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
        org.apache.hadoop.hdfs.TestDatanodeRegistration
        org.apache.hadoop.hdfs.server.balancer.TestBalancer
        org.apache.hadoop.hdfs.TestDfsOverAvroRpc
        org.apache.hadoop.hdfs.TestDFSStartupVersions
        org.apache.hadoop.hdfs.TestDFSUpgrade
        org.apache.hadoop.hdfs.TestPread

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1568//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1568//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/12504188/hdfs-2563.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 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 (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes org.apache.hadoop.hdfs.TestDatanodeRegistration org.apache.hadoop.hdfs.server.balancer.TestBalancer org.apache.hadoop.hdfs.TestDfsOverAvroRpc org.apache.hadoop.hdfs.TestDFSStartupVersions org.apache.hadoop.hdfs.TestDFSUpgrade org.apache.hadoop.hdfs.TestPread +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/1568//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/1568//console This message is automatically generated.
        Hide
        Todd Lipcon added a comment -

        Here is a prelim patch (haven't run through all the unit tests locally yet)... we'll see what Hudson has to say.

        Summary of the changes:

        • Add javadoc for bpRegistration, bpNSInfo fields to clarify when they get set
        • Remove blockPoolId member and replace with an accessor, since this is already stored as part of bpNSInfo
        • Don't assign bpRegistration until the block pool is actually determined during the handshake process. Change use of bpRegistration in log messages to use this.toString() which now has a sensible output. We used to see log messages like:
          2011-11-18 01:13:24,244 INFO  datanode.DataNode (DataNode.java:run(1195)) - DatanodeRegistration(127.0.0.1:56326, storageID=, infoPort=39362, ipcPort=38231, storageInfo=lv=0;cid=;nsid=0;c=0)In BPOfferService.run, data = null;bp=null
          

          which looks like an error to the user. Now it will say something like:

          2011-11-18 01:19:08,209 INFO  datanode.DataNode (DataNode.java:run(1238)) - Block pool <registering> (storage id unknown) connecting to todd-w510/127.0.0.1:9930 starting to offer service
          

          and later

          2011-11-18 01:19:08,395 INFO  datanode.DataNode (DataNode.java:register(1197)) - Block pool BP-428670582-127.0.0.1-1321607947917 (storage id DS-329594984-127.0.0.1-49361-1321607948276) registered with todd-w510/127.0.0.1:9930 beginning handshake with NN at todd-w510/127.0.0.1:9930
          

          when the blockPoolId and storageId become known

        • Split the handshake method into two clearer parts: (1) retrieveNamespaceInfo which calls versionRequest to determine the NN's version info and namespace/cluster ID, and (2) register() like before, which actually registers the DN.
        • Refactor the code to check layout versions (used to be duplicated in two places) into a checkNNVersion class.
        • Some other trivial improvements of log messages, etc, to be more user readable.
        • Removed unused test function DataNodeTestUtils.setBPNamenodeByIndex. Fixed TestDatanodeRegister to correspond to above changes.
        Show
        Todd Lipcon added a comment - Here is a prelim patch (haven't run through all the unit tests locally yet)... we'll see what Hudson has to say. Summary of the changes: Add javadoc for bpRegistration, bpNSInfo fields to clarify when they get set Remove blockPoolId member and replace with an accessor, since this is already stored as part of bpNSInfo Don't assign bpRegistration until the block pool is actually determined during the handshake process. Change use of bpRegistration in log messages to use this.toString() which now has a sensible output. We used to see log messages like: 2011-11-18 01:13:24,244 INFO datanode.DataNode (DataNode.java:run(1195)) - DatanodeRegistration(127.0.0.1:56326, storageID=, infoPort=39362, ipcPort=38231, storageInfo=lv=0;cid=;nsid=0;c=0)In BPOfferService.run, data = null ;bp= null which looks like an error to the user. Now it will say something like: 2011-11-18 01:19:08,209 INFO datanode.DataNode (DataNode.java:run(1238)) - Block pool <registering> (storage id unknown) connecting to todd-w510/127.0.0.1:9930 starting to offer service and later 2011-11-18 01:19:08,395 INFO datanode.DataNode (DataNode.java:register(1197)) - Block pool BP-428670582-127.0.0.1-1321607947917 (storage id DS-329594984-127.0.0.1-49361-1321607948276) registered with todd-w510/127.0.0.1:9930 beginning handshake with NN at todd-w510/127.0.0.1:9930 when the blockPoolId and storageId become known Split the handshake method into two clearer parts: (1) retrieveNamespaceInfo which calls versionRequest to determine the NN's version info and namespace/cluster ID, and (2) register() like before, which actually registers the DN. Refactor the code to check layout versions (used to be duplicated in two places) into a checkNNVersion class. Some other trivial improvements of log messages, etc, to be more user readable. Removed unused test function DataNodeTestUtils.setBPNamenodeByIndex . Fixed TestDatanodeRegister to correspond to above changes.

          People

          • Assignee:
            Todd Lipcon
            Reporter:
            Todd Lipcon
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development