Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3418

Rename BlockWithLocationsProto datanodeIDs field to storageIDs

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha
    • Fix Version/s: 2.0.0-alpha
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The "datanodeIDs" field in BlockWithLocationsProto is actually a list of storageIDs. Let's rename it there and in BlocksWithLocations so the code is clear.

      1. hdfs-3418.txt
        7 kB
        Eli Collins
      2. hdfs-3418.txt
        7 kB
        Eli Collins

        Activity

        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1081 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1081/)
        HDFS-3418. Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830)

        Result = SUCCESS
        eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830
        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/protocolPB/PBHelper.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1081 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1081/ ) HDFS-3418 . Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830 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/protocolPB/PBHelper.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1045 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1045/)
        HDFS-3418. Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830)

        Result = FAILURE
        eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830
        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/protocolPB/PBHelper.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1045 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1045/ ) HDFS-3418 . Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830) Result = FAILURE eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830 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/protocolPB/PBHelper.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2265 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2265/)
        HDFS-3418. Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830)

        Result = ABORTED
        eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830
        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/protocolPB/PBHelper.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2265 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2265/ ) HDFS-3418 . Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830) Result = ABORTED eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830 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/protocolPB/PBHelper.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #2248 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2248/)
        HDFS-3418. Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830)

        Result = SUCCESS
        eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830
        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/protocolPB/PBHelper.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2248 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2248/ ) HDFS-3418 . Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830 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/protocolPB/PBHelper.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2322 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2322/)
        HDFS-3418. Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830)

        Result = SUCCESS
        eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830
        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/protocolPB/PBHelper.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2322 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2322/ ) HDFS-3418 . Rename BlockWithLocationsProto datanodeIDs field to storageIDs. Contributed by Eli Collins (Revision 1338830) Result = SUCCESS eli : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1338830 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/protocolPB/PBHelper.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/protocol/BlocksWithLocations.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/proto/hdfs.proto /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/protocolPB/TestPBHelper.java
        Hide
        Eli Collins added a comment -

        I've committed this to trunk and merged to branch-2.

        Show
        Eli Collins added a comment - I've committed this to trunk and merged to branch-2.
        Hide
        Eli Collins added a comment -

        From https://builds.apache.org/job/PreCommit-HDFS-Build/2442/console

        +1 overall.  Here are the results of testing the latest attachment 
          http://issues.apache.org/jira/secure/attachment/12527370/hdfs-3418.txt
          against trunk revision .
        
            +1 @author.  The patch does not contain any @author tags.
        
            +1 tests included.  The patch appears to include 2 new or modified test files.
        
            +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
        
            +1 javadoc.  The javadoc tool did not generate any warning messages.
        
            +1 eclipse:eclipse.  The patch built with eclipse:eclipse.
        
            +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 passed unit tests in hadoop-hdfs-project/hadoop-hdfs.
        
            +1 contrib tests.  The patch passed contrib unit tests.
        
        Show
        Eli Collins added a comment - From https://builds.apache.org/job/PreCommit-HDFS-Build/2442/console +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12527370/hdfs-3418.txt against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +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 passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests. The patch passed contrib unit tests.
        Hide
        Eli Collins added a comment -

        Patch attached.

        Show
        Eli Collins added a comment - Patch attached.
        Hide
        Eli Collins added a comment -

        You're right, this is compatible.

        Show
        Eli Collins added a comment - You're right, this is compatible.
        Hide
        Todd Lipcon added a comment -

        +1, looks good to me assuming it passes tests.

        I don't think this is in fact an incompatible change, right? The field is renamed but the content is the same, and given that it's serialized using the same protobuf tag, it won't change the wire format at all.

        Show
        Todd Lipcon added a comment - +1, looks good to me assuming it passes tests. I don't think this is in fact an incompatible change, right? The field is renamed but the content is the same, and given that it's serialized using the same protobuf tag, it won't change the wire format at all.
        Hide
        Eli Collins added a comment -

        Here's the delta atop hdfs-3148.txt that's needed:

        --- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protoco
        +++ hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protoco
        @@ -254,11 +254,11 @@ public class PBHelper {
           public static BlockWithLocationsProto convert(BlockWithLocations blk) {
             return BlockWithLocationsProto.newBuilder()
                 .setBlock(convert(blk.getBlock()))
        -        .addAllDatanodeIDs(Arrays.asList(blk.getStorageIDs())).build();
        +        .addAllStorageIDs(Arrays.asList(blk.getStorageIDs())).build();
           }
         
           public static BlockWithLocations convert(BlockWithLocationsProto b) {
        -    return new BlockWithLocations(convert(b.getBlock()), b.getDatanodeIDsList()
        +    return new BlockWithLocations(convert(b.getBlock()), b.getStorageIDsList()
                 .toArray(new String[0]));
           }
        
        Show
        Eli Collins added a comment - Here's the delta atop hdfs-3148.txt that's needed: --- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protoco +++ hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protoco @@ -254,11 +254,11 @@ public class PBHelper { public static BlockWithLocationsProto convert(BlockWithLocations blk) { return BlockWithLocationsProto.newBuilder() .setBlock(convert(blk.getBlock())) - .addAllDatanodeIDs(Arrays.asList(blk.getStorageIDs())).build(); + .addAllStorageIDs(Arrays.asList(blk.getStorageIDs())).build(); } public static BlockWithLocations convert(BlockWithLocationsProto b) { - return new BlockWithLocations(convert(b.getBlock()), b.getDatanodeIDsList() + return new BlockWithLocations(convert(b.getBlock()), b.getStorageIDsList() .toArray( new String [0])); }
        Hide
        Eli Collins added a comment -

        See hdfs-3418.txt (accidentally uploaded both)

        Show
        Eli Collins added a comment - See hdfs-3418.txt (accidentally uploaded both)
        Hide
        Todd Lipcon added a comment -

        Sure you uploaded the right patch here? Looks like you uploaded 3148 instead of 3418

        Show
        Todd Lipcon added a comment - Sure you uploaded the right patch here? Looks like you uploaded 3148 instead of 3418
        Hide
        Eli Collins added a comment -

        Updated patch, left out a place in PBHelper that I missed due to an unclean build.

        Show
        Eli Collins added a comment - Updated patch, left out a place in PBHelper that I missed due to an unclean build.
        Hide
        Eli Collins added a comment -

        Patch attached.

        Show
        Eli Collins added a comment - Patch attached.

          People

          • Assignee:
            Eli Collins
            Reporter:
            Eli Collins
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development