Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-3495

Update Balancer to support new NetworkTopology with NodeGroup

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2.0, 2.1.0-beta
    • Component/s: balancer & mover
    • Labels:
      None

      Description

      Since the Balancer is a Hadoop Tool, it was updated to be directly aware of four-layer hierarchy instead of creating an alternative Balancer implementation. To accommodate extensibility, a new protected method, doChooseNodesForCustomFaultDomain is now called from the existing chooseNodes method so that a subclass of the Balancer could customize the balancer algotirhm for other failure and locality topologies. An alternative option is to encapsulate the algorithm used for the four-layer hierarchy into a collaborating strategy class.
      The key changes introduced to support a four-layer hierarchy were to override the algorithm of choosing <source, target> pairs for balancing. Unit tests were created to test the new algorithm.
      The algorithm now makes sure to choose the target and source node on the same node group for balancing as the first priority. Then the overall balancing policy is: first doing balancing between nodes within the same nodegroup then the same rack and off rack at last. Also, we need to check no duplicated replicas live in the same node group after balancing.

      1. HADOOP-8473-Balancer-NodeGroup-aware.patch
        23 kB
        Junping Du
      2. HDFS-3495-branch-2.patch
        43 kB
        Junping Du
      3. HDFS-3495-v2.patch
        41 kB
        Junping Du
      4. HDFS-3495-v3.patch
        41 kB
        Junping Du
      5. HDFS-3495-v4.patch
        43 kB
        Junping Du
      6. HDFS-3495-v5.patch
        43 kB
        Junping Du

        Issue Links

          Activity

          Hide
          airbots Chen He added a comment -

          The same block is impossible to be on the same node. They have the same file name on the host File System. You may not move the block from a overloaded node to a less-loaded node. It will report your error.

          Show
          airbots Chen He added a comment - The same block is impossible to be on the same node. They have the same file name on the host File System. You may not move the block from a overloaded node to a less-loaded node. It will report your error.
          Hide
          airbots Chen He added a comment -

          How to keep the Hadoop Data availablility if the node group is as large as the whole cluster?

          Show
          airbots Chen He added a comment - How to keep the Hadoop Data availablility if the node group is as large as the whole cluster?
          Hide
          djp Junping Du added a comment -

          Hi Timer,
          Thanks for the comments. NodeGroup layer is trying to map to hypervisor layer in virtualization environment. So if whole cluster is only a single physical host with a bunch of VMs running on top of, then nothing can help on data availability. In fact, enhancing data availability is one reason we want to deliver in this JIRA Umbrella (HADOOP-8468) for hadoop running in cloud (with a high percentage built on virtualized platform). i.e: We want to let hadoop can aware "NodeGroup" layer so that not putting two replicas on two VMs running on the same physical host. Thoughts?

          Thanks,

          Junping

          Show
          djp Junping Du added a comment - Hi Timer, Thanks for the comments. NodeGroup layer is trying to map to hypervisor layer in virtualization environment. So if whole cluster is only a single physical host with a bunch of VMs running on top of, then nothing can help on data availability. In fact, enhancing data availability is one reason we want to deliver in this JIRA Umbrella ( HADOOP-8468 ) for hadoop running in cloud (with a high percentage built on virtualized platform). i.e: We want to let hadoop can aware "NodeGroup" layer so that not putting two replicas on two VMs running on the same physical host. Thoughts? Thanks, Junping
          Hide
          djp Junping Du added a comment -

          This patch depends on HADOOP-8439.

          Show
          djp Junping Du added a comment - This patch depends on HADOOP-8439 .
          Hide
          djp Junping Du added a comment -

          This patch has dependency on HADOOP-8469, and should be checked in after HADOOP-8469

          Show
          djp Junping Du added a comment - This patch has dependency on HADOOP-8469 , and should be checked in after HADOOP-8469
          Hide
          sanjay.radia Sanjay Radia added a comment -

          There are two separate problems here as mentioned in your description - please split into two separate jiras:

          • correctness - two replicas are not on the same node
          • performance optimization - "choose the target and source node on the same node group for balancing as the first priority".
          Show
          sanjay.radia Sanjay Radia added a comment - There are two separate problems here as mentioned in your description - please split into two separate jiras: correctness - two replicas are not on the same node performance optimization - "choose the target and source node on the same node group for balancing as the first priority".
          Hide
          djp Junping Du added a comment -

          Hey Sanjay, Thanks for the comments. Will update soon.

          Best,

          Junping

          Show
          djp Junping Du added a comment - Hey Sanjay, Thanks for the comments. Will update soon. Best, Junping
          Hide
          djp Junping Du added a comment -

          Link this JIRA with its parent Umbrella

          Show
          djp Junping Du added a comment - Link this JIRA with its parent Umbrella
          Hide
          djp Junping Du added a comment -

          Finished.

          Show
          djp Junping Du added a comment - Finished.
          Hide
          djp Junping Du added a comment -

          Update with combine patches in HDFS-3496 and HDFS-3497 v3 patch.

          Show
          djp Junping Du added a comment - Update with combine patches in HDFS-3496 and HDFS-3497 v3 patch.
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 3 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

          org.apache.hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3534//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3534//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12554064/HDFS-3495-v2.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3534//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3534//console This message is automatically generated.
          Hide
          djp Junping Du added a comment -

          One test failure seems irrelevant. Also, this test is running well on local environment. So rename v2 patch to v3 patch and submit again.

          Show
          djp Junping Du added a comment - One test failure seems irrelevant. Also, this test is running well on local environment. So rename v2 patch to v3 patch and submit again.
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 3 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.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3535//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3535//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12554100/HDFS-3495-v3.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 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. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3535//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3535//console This message is automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Thank Junping for the update. Looked at the patch. You did a great job on fixing/adding comments for the existing code

          • It seems that we are not going to subclass Balancer. If it is the case, we should not change the fields/methods to protected.
          • There are a few places initializing a NetworkTopology instance from conf. How about adding a new getInstance(conf) method to NetworkTopology so that Balancer, DatanodeManager and some tests could call it?
            +    cluster = ReflectionUtils.newInstance(
            +        conf.getClass(
            +            CommonConfigurationKeysPublic.NET_TOPOLOGY_IMPL_KEY,
            +            NetworkTopology.class, NetworkTopology.class), conf)
            
          • The one line method areDataNodesOnSameNodeGroup(..) seems unnecessary. How about using cluster.isOnSameNodeGroup(..) directly?
          • If we have an addTo(..) method in PendingBlockMove, chooseProxySource() can be simplified as below:
                private boolean addTo(BalancerDatanode bdn) {
                  if (bdn.addPendingBlock(this)) {
                    proxySource = bdn;
                    return true;
                  }
                  return false;
                }
            
                /* Now we find out source, target, and block, we need to find a proxy
                 * 
                 * @return true if a proxy is found; otherwise false
                 */
                private boolean chooseProxySource() {
                  final DatanodeInfo targetDn = target.getDatanode();
                  boolean found = false;
                  for (BalancerDatanode loc : block.getLocations()) {
                    // check if there is replica which is on the same rack with the target
                    if (cluster.isOnSameRack(loc.getDatanode(), targetDn) && addTo(loc)) {
                      // if cluster is not nodegroup aware or the proxy is on the same 
                      // nodegroup with target, then we already find the nearest proxy
                      if (!cluster.isNodeGroupAware() 
                          || cluster.isOnSameNodeGroup(loc.getDatanode(), targetDn)) {
                        return true;
                      }
                      found = true;
                    }
            
                    if (!found) {
                      // find out a non-busy replica out of rack of target
                      found = addTo(loc);
                    }
                  }
                  return found;
                }
            

            The addTo method could also be used in Source.chooseNextBlockToMove().

                     PendingBlockMove pendingBlock = new PendingBlockMove();
            -        if ( target.addPendingBlock(pendingBlock) ) { 
            +        if (pendingBlock.addTo(target)) { 
                       // target is not busy, so do a tentative block allocation
            -          pendingBlock.source = this;
                       pendingBlock.target = target;
            
          • Let's combine chooseNodes() and chooseNodesForCustomFaultDomains()?
          • chooseTargetOnSameNodeGroup and chooseSourceOnSameNodeGroup look the same. Similarly, chooseTargetsOnSameNodeGroup and chooseSourcesOnSameNodeGroup looks the same. I spent sometime to combine them. See if you think it is good.
              /* Decide all <source, target> pairs where source and target are 
               * on the same NodeGroup
               */
              private void chooseNodesOnSameNodeGroup() {
            
                /* first step: match each overUtilized datanode (source) to
                 * one or more underUtilized datanodes within same NodeGroup(targets).
                 */
                chooseOnSameNodeGroup(overUtilizedDatanodes, underUtilizedDatanodes);
            
                /* match each remaining overutilized datanode (source) to below average 
                 * utilized datanodes within the same NodeGroup(targets).
                 * Note only overutilized datanodes that haven't had that max bytes to move
                 * satisfied in step 1 are selected
                 */
                chooseOnSameNodeGroup(overUtilizedDatanodes, belowAvgUtilizedDatanodes);
            
                /* match each remaining underutilized datanode to above average utilized 
                 * datanodes within the same NodeGroup.
                 * Note only underutilized datanodes that have not had that max bytes to
                 * move satisfied in step 1 are selected.
                 */
                chooseOnSameNodeGroup(underUtilizedDatanodes, aboveAvgUtilizedDatanodes);
              }
            
              private <T extends BalancerDatanode> T chooseCandidateOnSameNodeGroup(
                  BalancerDatanode dn, Iterator<T> candidates) {
                if (dn.isMoveQuotaFull()) {
                  for(; candidates.hasNext(); ) {
                    final T c = candidates.next();
                    if (!c.isMoveQuotaFull()) {
                      candidates.remove();
                      continue;
                    }
                    if (cluster.isOnSameNodeGroup(dn.getDatanode(), c.getDatanode())) {
                      return c;
                    }
                  }
                }
                return null;
              }
              
              private void selectCandidateOnSameNodeGroup(
                  Source source, BalancerDatanode target) {
                long size = Math.min(source.availableSizeToMove(), target.availableSizeToMove());
                NodeTask nodeTask = new NodeTask(target, size);
                source.addNodeTask(nodeTask);
                target.incScheduledSize(nodeTask.getSize());
                sources.add(source);
                targets.add(target);
                LOG.info("Decided to move "+StringUtils.byteDesc(size)+" bytes from "
                    +source.datanode.getName() + " to " + target.datanode.getName());
              }
            
              private <T extends BalancerDatanode> boolean chooseOnSameNodeGroup(
                  BalancerDatanode dn, Iterator<T> candidates) {
                final T chosen = chooseCandidateOnSameNodeGroup(dn, candidates);
                if (chosen == null) {
                  return false;
                }
                if (dn instanceof Source) {
                  selectCandidateOnSameNodeGroup((Source)dn, chosen);
                } else {
                  selectCandidateOnSameNodeGroup((Source)chosen, dn);
                }
                if (!chosen.isMoveQuotaFull()) {
                  candidates.remove();
                }
                return true;
              }
            
              private <D extends BalancerDatanode, C extends BalancerDatanode>
                  void chooseOnSameNodeGroup(Collection<D> datanodes, Collection<C> candidates) {
                for (Iterator<D> i = datanodes.iterator(); i.hasNext();) {
                  final D source = i.next();
                  for(; chooseOnSameNodeGroup(source, candidates.iterator()); );
                  if (!source.isMoveQuotaFull()) {
                    i.remove();
                  }
                }
              }
            
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Thank Junping for the update. Looked at the patch. You did a great job on fixing/adding comments for the existing code It seems that we are not going to subclass Balancer. If it is the case, we should not change the fields/methods to protected. There are a few places initializing a NetworkTopology instance from conf. How about adding a new getInstance(conf) method to NetworkTopology so that Balancer, DatanodeManager and some tests could call it? + cluster = ReflectionUtils.newInstance( + conf.getClass( + CommonConfigurationKeysPublic.NET_TOPOLOGY_IMPL_KEY, + NetworkTopology.class, NetworkTopology.class), conf) The one line method areDataNodesOnSameNodeGroup(..) seems unnecessary. How about using cluster.isOnSameNodeGroup(..) directly? If we have an addTo(..) method in PendingBlockMove, chooseProxySource() can be simplified as below: private boolean addTo(BalancerDatanode bdn) { if (bdn.addPendingBlock( this )) { proxySource = bdn; return true ; } return false ; } /* Now we find out source, target, and block, we need to find a proxy * * @ return true if a proxy is found; otherwise false */ private boolean chooseProxySource() { final DatanodeInfo targetDn = target.getDatanode(); boolean found = false ; for (BalancerDatanode loc : block.getLocations()) { // check if there is replica which is on the same rack with the target if (cluster.isOnSameRack(loc.getDatanode(), targetDn) && addTo(loc)) { // if cluster is not nodegroup aware or the proxy is on the same // nodegroup with target, then we already find the nearest proxy if (!cluster.isNodeGroupAware() || cluster.isOnSameNodeGroup(loc.getDatanode(), targetDn)) { return true ; } found = true ; } if (!found) { // find out a non-busy replica out of rack of target found = addTo(loc); } } return found; } The addTo method could also be used in Source.chooseNextBlockToMove(). PendingBlockMove pendingBlock = new PendingBlockMove(); - if ( target.addPendingBlock(pendingBlock) ) { + if (pendingBlock.addTo(target)) { // target is not busy, so do a tentative block allocation - pendingBlock.source = this ; pendingBlock.target = target; Let's combine chooseNodes() and chooseNodesForCustomFaultDomains()? chooseTargetOnSameNodeGroup and chooseSourceOnSameNodeGroup look the same. Similarly, chooseTargetsOnSameNodeGroup and chooseSourcesOnSameNodeGroup looks the same. I spent sometime to combine them. See if you think it is good. /* Decide all <source, target> pairs where source and target are * on the same NodeGroup */ private void chooseNodesOnSameNodeGroup() { /* first step: match each overUtilized datanode (source) to * one or more underUtilized datanodes within same NodeGroup(targets). */ chooseOnSameNodeGroup(overUtilizedDatanodes, underUtilizedDatanodes); /* match each remaining overutilized datanode (source) to below average * utilized datanodes within the same NodeGroup(targets). * Note only overutilized datanodes that haven't had that max bytes to move * satisfied in step 1 are selected */ chooseOnSameNodeGroup(overUtilizedDatanodes, belowAvgUtilizedDatanodes); /* match each remaining underutilized datanode to above average utilized * datanodes within the same NodeGroup. * Note only underutilized datanodes that have not had that max bytes to * move satisfied in step 1 are selected. */ chooseOnSameNodeGroup(underUtilizedDatanodes, aboveAvgUtilizedDatanodes); } private <T extends BalancerDatanode> T chooseCandidateOnSameNodeGroup( BalancerDatanode dn, Iterator<T> candidates) { if (dn.isMoveQuotaFull()) { for (; candidates.hasNext(); ) { final T c = candidates.next(); if (!c.isMoveQuotaFull()) { candidates.remove(); continue ; } if (cluster.isOnSameNodeGroup(dn.getDatanode(), c.getDatanode())) { return c; } } } return null ; } private void selectCandidateOnSameNodeGroup( Source source, BalancerDatanode target) { long size = Math .min(source.availableSizeToMove(), target.availableSizeToMove()); NodeTask nodeTask = new NodeTask(target, size); source.addNodeTask(nodeTask); target.incScheduledSize(nodeTask.getSize()); sources.add(source); targets.add(target); LOG.info( "Decided to move " +StringUtils.byteDesc(size)+ " bytes from " +source.datanode.getName() + " to " + target.datanode.getName()); } private <T extends BalancerDatanode> boolean chooseOnSameNodeGroup( BalancerDatanode dn, Iterator<T> candidates) { final T chosen = chooseCandidateOnSameNodeGroup(dn, candidates); if (chosen == null ) { return false ; } if (dn instanceof Source) { selectCandidateOnSameNodeGroup((Source)dn, chosen); } else { selectCandidateOnSameNodeGroup((Source)chosen, dn); } if (!chosen.isMoveQuotaFull()) { candidates.remove(); } return true ; } private <D extends BalancerDatanode, C extends BalancerDatanode> void chooseOnSameNodeGroup(Collection<D> datanodes, Collection<C> candidates) { for (Iterator<D> i = datanodes.iterator(); i.hasNext();) { final D source = i.next(); for (; chooseOnSameNodeGroup(source, candidates.iterator()); ); if (!source.isMoveQuotaFull()) { i.remove(); } } }
          Hide
          djp Junping Du added a comment -

          Hi Nicholas,
          Thanks for your review and your great comments! Please see my reply below:
          1.It seems that we are not going to subclass Balancer. If it is the case, we should not change the fields/methods to protected.
          [JP] Sure. I remove all protected method in Balancer. The exception could be on unit test as it needs a subclass of MiniDFSCluster (MiniDFSClusterWithNodeGroup) which have different configuration in initialization.
          2. There are a few places initializing a NetworkTopology instance from conf. How about adding a new getInstance(conf) method to NetworkTopology so that Balancer, DatanodeManager and some tests could call it?
          [JP] Sure. Updated.
          3. The one line method areDataNodesOnSameNodeGroup(..) seems unnecessary. How about using cluster.isOnSameNodeGroup(..) directly?
          [JP] Agree. it could be better. Updated.
          4.If we have an addTo(..) method in PendingBlockMove, chooseProxySource() can be simplified as below:
          [JP] It is awesome! It makes code much simplified and clear. Thanks!
          5. The addTo method could also be used in Source.chooseNextBlockToMove().
          [JP] It looks slight different case there. If we use addTo() to replace original code, then we will lose "pendingBlock.source = this;" and add " proxySource = target" which is different with original logic?
          6.Let's combine chooseNodes() and chooseNodesForCustomFaultDomains()?
          [JP] Sure. Two methods are combined.
          7. chooseTargetOnSameNodeGroup and chooseSourceOnSameNodeGroup look the same. Similarly, chooseTargetsOnSameNodeGroup and chooseSourcesOnSameNodeGroup looks the same. I spent sometime to combine them. See if you think it is good.
          [JP] It is super! I put it in new changes and the method of selectCandidateOnSameNodeGroup can be used by other code in Balancer (I rename to matchSourceWithTargetToMove as it is more general).
          I will update patch soon.

          Show
          djp Junping Du added a comment - Hi Nicholas, Thanks for your review and your great comments! Please see my reply below: 1.It seems that we are not going to subclass Balancer. If it is the case, we should not change the fields/methods to protected. [JP] Sure. I remove all protected method in Balancer. The exception could be on unit test as it needs a subclass of MiniDFSCluster (MiniDFSClusterWithNodeGroup) which have different configuration in initialization. 2. There are a few places initializing a NetworkTopology instance from conf. How about adding a new getInstance(conf) method to NetworkTopology so that Balancer, DatanodeManager and some tests could call it? [JP] Sure. Updated. 3. The one line method areDataNodesOnSameNodeGroup(..) seems unnecessary. How about using cluster.isOnSameNodeGroup(..) directly? [JP] Agree. it could be better. Updated. 4.If we have an addTo(..) method in PendingBlockMove, chooseProxySource() can be simplified as below: [JP] It is awesome! It makes code much simplified and clear. Thanks! 5. The addTo method could also be used in Source.chooseNextBlockToMove(). [JP] It looks slight different case there. If we use addTo() to replace original code, then we will lose "pendingBlock.source = this;" and add " proxySource = target" which is different with original logic? 6.Let's combine chooseNodes() and chooseNodesForCustomFaultDomains()? [JP] Sure. Two methods are combined. 7. chooseTargetOnSameNodeGroup and chooseSourceOnSameNodeGroup look the same. Similarly, chooseTargetsOnSameNodeGroup and chooseSourcesOnSameNodeGroup looks the same. I spent sometime to combine them. See if you think it is good. [JP] It is super! I put it in new changes and the method of selectCandidateOnSameNodeGroup can be used by other code in Balancer (I rename to matchSourceWithTargetToMove as it is more general). I will update patch soon.
          Hide
          djp Junping Du added a comment -

          Nicholas, I address all your comments in v4 patch. Great thanks for your review!

          Show
          djp Junping Du added a comment - Nicholas, I address all your comments in v4 patch. Great thanks for your review!
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12554661/HADOOP-9045-v4.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 1 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.

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3556//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3556//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12554661/HADOOP-9045-v4.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 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. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3556//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3556//console This message is automatically generated.
          Hide
          djp Junping Du added a comment -

          Sorry. Above is a wrong patch, update a correct one of HDFS-3495-v4 patch.

          Show
          djp Junping Du added a comment - Sorry. Above is a wrong patch, update a correct one of HDFS-3495 -v4 patch.
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12554678/HDFS-3495-v4.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 3 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-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3557//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3557//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12554678/HDFS-3495-v4.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 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-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3557//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3557//console This message is automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Patch looks good. Just some minor comments:

          • In Balancer.chooseProxySource(), "proxySource = loc" is not needed since addTo(..) already has set it. Also, the indentation for the first if-statement has 4 spaces.
          • TestBalancerWithNodeGroup has some javac warnings
            • Random r is not used.
            • unused imports (your may use [ctrl/cmd]-shift-o to fix it in eclipse)
          • There are also unused imports in Balancer.
          • Typo: "paramater" => "parameter" in NetworkTopology.getInstance.
          • Change the new comments to javadoc whenever possible, e.g. use /** insteadof /* for method header comments.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Patch looks good. Just some minor comments: In Balancer.chooseProxySource(), "proxySource = loc" is not needed since addTo(..) already has set it. Also, the indentation for the first if-statement has 4 spaces. TestBalancerWithNodeGroup has some javac warnings Random r is not used. unused imports (your may use [ctrl/cmd] -shift-o to fix it in eclipse) There are also unused imports in Balancer. Typo: "paramater" => "parameter" in NetworkTopology.getInstance. Change the new comments to javadoc whenever possible, e.g. use /** insteadof /* for method header comments.
          Hide
          djp Junping Du added a comment -

          Thanks Nicholas for your careful review. I address all your comments in v5 patch. Thanks!

          Show
          djp Junping Du added a comment - Thanks Nicholas for your careful review. I address all your comments in v5 patch. Thanks!
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12555113/HDFS-3495-v5.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 3 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-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3569//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3569//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12555113/HDFS-3495-v5.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 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-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3569//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3569//console This message is automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          +1 patch looks good.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - +1 patch looks good.
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3070 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3070/)
          HDFS-3495. Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414874)

          Result = FAILURE
          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414874
          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/balancer/Balancer.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSClusterWithNodeGroup.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancerWithNodeGroup.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-trunk-Commit #3070 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3070/ ) HDFS-3495 . Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414874) Result = FAILURE szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414874 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/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSClusterWithNodeGroup.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancerWithNodeGroup.java
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thank you very much for the great work, Junping!

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - I have committed this. Thank you very much for the great work, Junping!
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3071 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3071/)
          HDFS-3495. Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414878)

          Result = SUCCESS
          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414878
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-trunk-Commit #3071 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3071/ ) HDFS-3495 . Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414878) Result = SUCCESS szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414878 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #51 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/51/)
          HDFS-3495. Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414878)
          HDFS-3495. Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414874)

          Result = SUCCESS
          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414878
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java

          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414874
          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/balancer/Balancer.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSClusterWithNodeGroup.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancerWithNodeGroup.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Yarn-trunk #51 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/51/ ) HDFS-3495 . Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414878) HDFS-3495 . Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414874) Result = SUCCESS szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414878 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414874 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/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSClusterWithNodeGroup.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancerWithNodeGroup.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1241 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1241/)
          HDFS-3495. Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414878)
          HDFS-3495. Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414874)

          Result = FAILURE
          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414878
          Files :

          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java

          szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414874
          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/balancer/Balancer.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSClusterWithNodeGroup.java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancerWithNodeGroup.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1241 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1241/ ) HDFS-3495 . Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414878) HDFS-3495 . Update Balancer to support new NetworkTopology with NodeGroup. Contributed by Junping Du (Revision 1414874) Result = FAILURE szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414878 Files : /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1414874 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/balancer/Balancer.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/blockmanagement/DatanodeManager.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSCluster.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/MiniDFSClusterWithNodeGroup.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/balancer/TestBalancerWithNodeGroup.java
          Hide
          djp Junping Du added a comment -

          reopen this JIRA to backport to branch-2.

          Show
          djp Junping Du added a comment - reopen this JIRA to backport to branch-2.
          Hide
          djp Junping Du added a comment -

          Attach a patch for branch-2.

          Show
          djp Junping Du added a comment - Attach a patch for branch-2.
          Hide
          hadoopqa Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12585722/HDFS-3495-branch-2.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4466//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12585722/HDFS-3495-branch-2.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/4466//console This message is automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Merged this to branch-2.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Merged this to branch-2.

            People

            • Assignee:
              djp Junping Du
              Reporter:
              djp Junping Du
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development