Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3942

Backport HDFS-3495: Update balancer policy for Network Topology with additional 'NodeGroup' layer

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.0
    • Fix Version/s: 1.2.0, 1-win
    • Component/s: balancer & mover
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      This is the backport work for HDFS-3495 and HDFS-4234.

      1. HDFS-3942-v5.patch
        44 kB
        Junping Du
      2. HDFS-3942-v4.patch
        38 kB
        Junping Du
      3. HDFS-3942-v3.patch
        38 kB
        Junping Du
      4. HDFS-3942-v2.patch
        37 kB
        Junping Du
      5. HDFS-3942.patch
        29 kB
        Junping Du

        Issue Links

          Activity

          Junping Du created issue -
          Junping Du made changes -
          Field Original Value New Value
          Link This issue is related to HADOOP-8817 [ HADOOP-8817 ]
          Junping Du made changes -
          Assignee Junping Du [ djp ]
          Hide
          Suresh Srinivas added a comment -

          Target version cannot be 1.0.3. It is already released.

          Show
          Suresh Srinivas added a comment - Target version cannot be 1.0.3. It is already released.
          Suresh Srinivas made changes -
          Target Version/s 1.0.3 [ 12320249 ] 1.2.0 [ 12321657 ]
          Junping Du made changes -
          Link This issue is related to HADOOP-8817 [ HADOOP-8817 ]
          Junping Du made changes -
          Link This issue Is contained by HADOOP-8817 [ HADOOP-8817 ]
          Junping Du made changes -
          Link This issue is blocked by HADOOP-8820 [ HADOOP-8820 ]
          Junping Du made changes -
          Attachment HDFS-3942.patch [ 12545371 ]
          Tsz Wo Nicholas Sze made changes -
          Link This issue relates to HDFS-3495 [ HDFS-3495 ]
          Hide
          Junping Du added a comment -

          Update patch due to refactor work for balancer in trunk branch.

          Show
          Junping Du added a comment - Update patch due to refactor work for balancer in trunk branch.
          Junping Du made changes -
          Attachment HDFS-3942-v2.patch [ 12560533 ]
          Junping Du made changes -
          Description This is the backport work for HDFS-3495 and HDFS-4234.
          Junping Du made changes -
          Target Version/s 1.2.0 [ 12321657 ] 1.2.0, 1.1.2 [ 12321657, 12323593 ]
          Hide
          Junping Du added a comment -

          Thanks Suresh for reminder. I just update target version to 1.1.2.

          Show
          Junping Du added a comment - Thanks Suresh for reminder. I just update target version to 1.1.2.
          Junping Du made changes -
          Link This issue relates HDFS-4234 [ HDFS-4234 ]
          Hide
          Junping Du added a comment -

          This patch backport HDFS-4234 too.

          Show
          Junping Du added a comment - This patch backport HDFS-4234 too.
          Hide
          Tsz Wo Nicholas Sze added a comment -
          • Remove the commented code in chooseNodes().
          • Move getNetworkTopologyInstance to NetworkTopology.
          • Remove areDataNodesOnSameNodeGroup(..).

          BTW, should we also wait for HDFS-4261?

          Show
          Tsz Wo Nicholas Sze added a comment - Remove the commented code in chooseNodes(). Move getNetworkTopologyInstance to NetworkTopology. Remove areDataNodesOnSameNodeGroup(..). BTW, should we also wait for HDFS-4261 ?
          Hide
          Junping Du added a comment -

          Thanks Nicholas for review and comments. I will address these comments later. In branch-1, there is no multiple NameNodeConnector, so the issue for HDFS-4261 is not existing here. Thoughts?

          Show
          Junping Du added a comment - Thanks Nicholas for review and comments. I will address these comments later. In branch-1, there is no multiple NameNodeConnector, so the issue for HDFS-4261 is not existing here. Thoughts?
          Hide
          Junping Du added a comment -

          Address Nicholas' comments in v3 patch.

          Show
          Junping Du added a comment - Address Nicholas' comments in v3 patch.
          Junping Du made changes -
          Attachment HDFS-3942-v3.patch [ 12560725 ]
          Hide
          Junping Du added a comment -

          Remove unnecessary import and code in v4 patch.

          Show
          Junping Du added a comment - Remove unnecessary import and code in v4 patch.
          Junping Du made changes -
          Attachment HDFS-3942-v4.patch [ 12560777 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          > ... so the issue for HDFS-4261 is not existing here. Thoughts?

          I think you are right. How about also add the new test in HDFS-4261 to branch-1?

          Some more comments on the patch:

          • TestBalancerWithNodeGroup should use junit 4 and not extend TestCase.
          • Change the comment on top of isOnSameNodeGroupWithReplicas(..) to javadoc and copy the javadoc from trunk.
          • In TestBalancerWithNodeGroup, the waitForHeartBeat(..) and runBalancer(..) are at the end but they are put near the beginning in trunk. Could you make order them the same way? In general, please make the code the same for both trunk and branch-1 so that it is easier to maintain.
          Show
          Tsz Wo Nicholas Sze added a comment - > ... so the issue for HDFS-4261 is not existing here. Thoughts? I think you are right. How about also add the new test in HDFS-4261 to branch-1? Some more comments on the patch: TestBalancerWithNodeGroup should use junit 4 and not extend TestCase. Change the comment on top of isOnSameNodeGroupWithReplicas(..) to javadoc and copy the javadoc from trunk. In TestBalancerWithNodeGroup, the waitForHeartBeat(..) and runBalancer(..) are at the end but they are put near the beginning in trunk. Could you make order them the same way? In general, please make the code the same for both trunk and branch-1 so that it is easier to maintain.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          BTW, we have to generate the patch with "--no-prefix" for branch-1. Otherwise, "ant test-patch" won't work. Please also run ant test-patch and all the unit tests. Thanks a lot!

          Show
          Tsz Wo Nicholas Sze added a comment - BTW, we have to generate the patch with "--no-prefix" for branch-1. Otherwise, "ant test-patch" won't work. Please also run ant test-patch and all the unit tests. Thanks a lot!
          Hide
          Junping Du added a comment -

          Thanks Nicholas for the comments. Yes. I will try the new patch on my local branch-1 env (previous issue should be resolved by HADOOP-9051).

          Show
          Junping Du added a comment - Thanks Nicholas for the comments. Yes. I will try the new patch on my local branch-1 env (previous issue should be resolved by HADOOP-9051 ).
          Hide
          Junping Du added a comment -

          Update patch as last comments from Nicholas. Will update test-patch result later.

          Show
          Junping Du added a comment - Update patch as last comments from Nicholas. Will update test-patch result later.
          Junping Du made changes -
          Attachment HDFS-3942-v5.patch [ 12561108 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Thanks a lot. I just have run test-patch.

               [exec] -1 overall.  
               [exec] 
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec] 
               [exec]     +1 tests included.  The patch appears to include 7 new or modified tests.
               [exec] 
               [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
               [exec] 
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec] 
               [exec]     -1 findbugs.  The patch appears to introduce 11 new Findbugs (version 1.3.9) warnings.
          
          Show
          Tsz Wo Nicholas Sze added a comment - Thanks a lot. I just have run test-patch. [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 7 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] -1 findbugs. The patch appears to introduce 11 new Findbugs (version 1.3.9) warnings.
          Hide
          Junping Du added a comment -

          Thanks Nicholas to post test-patch result. I am curious on are all findbugs issues involved by this patch?
          I have this question because I tried a previous patch on my env (not sure if it is settled down well) to have 225 Findbugs warning. I saw the same Findbugs warning number in recent result from other guys (like Zhao Jing) to post for branch-1 work. Just a little confused.

              -1 findbugs.  The patch appears to introduce 225 new Findbugs (version 2.0.1) warnings.
          
          Show
          Junping Du added a comment - Thanks Nicholas to post test-patch result. I am curious on are all findbugs issues involved by this patch? I have this question because I tried a previous patch on my env (not sure if it is settled down well) to have 225 Findbugs warning. I saw the same Findbugs warning number in recent result from other guys (like Zhao Jing) to post for branch-1 work. Just a little confused. -1 findbugs. The patch appears to introduce 225 new Findbugs (version 2.0.1) warnings.
          Hide
          Jing Zhao added a comment -

          Seems the test-patch in branch-1 has some problem with findbug?

          By the way, I run ant test for the latest patch and it passed all the local unit tests.

          Show
          Jing Zhao added a comment - Seems the test-patch in branch-1 has some problem with findbug? By the way, I run ant test for the latest patch and it passed all the local unit tests.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          It seems that test-patch cannot determine the new findbugs warnings correctly. I usually compare the findbugs warnings with an empty patch which also generate 11 "new" Findbugs (version 1.3.9) warnings.

          Show
          Tsz Wo Nicholas Sze added a comment - It seems that test-patch cannot determine the new findbugs warnings correctly. I usually compare the findbugs warnings with an empty patch which also generate 11 "new" Findbugs (version 1.3.9) warnings.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 on the new patch.

          I have committed this. Thanks Junping!

          Also thanks Jing for testing the patch.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 on the new patch. I have committed this. Thanks Junping! Also thanks Jing for testing the patch.
          Tsz Wo Nicholas Sze made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Fix Version/s 1.2.0 [ 12321657 ]
          Fix Version/s 1-win [ 12320362 ]
          Resolution Fixed [ 1 ]
          Hide
          Junping Du added a comment -

          Thanks Nicholas and Jing for help in review and testing patch!

          Show
          Junping Du added a comment - Thanks Nicholas and Jing for help in review and testing patch!
          Hide
          Junping Du added a comment -

          BTW, I agree that test-patch in branch-1 has problem with findbugs as findbugs seems to accumulate warnings of previous code.
          As lacking of a central pre-commit test (like Trunk), each individual developers' env with different version of FindBugs may have different result which make some warnings get ignored. So I guess it could be useful to have a central Jenkins for test-patch on branch-1 (and branch-2 too)? I saw some effort initiated in HADOOP-7435 but no clear the history and how to follow up.

          Show
          Junping Du added a comment - BTW, I agree that test-patch in branch-1 has problem with findbugs as findbugs seems to accumulate warnings of previous code. As lacking of a central pre-commit test (like Trunk), each individual developers' env with different version of FindBugs may have different result which make some warnings get ignored. So I guess it could be useful to have a central Jenkins for test-patch on branch-1 (and branch-2 too)? I saw some effort initiated in HADOOP-7435 but no clear the history and how to follow up.
          Matt Foley made changes -
          Target Version/s 1.2.0, 1.1.2 [ 12321657, 12323593 ] 1.2.0 [ 12321657 ]
          Gavin made changes -
          Link This issue relates to HDFS-4234 [ HDFS-4234 ]
          Gavin made changes -
          Link This issue relates to HDFS-4234 [ HDFS-4234 ]
          Hide
          Matt Foley added a comment -

          Closed upon release of Hadoop 1.2.0.

          Show
          Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.
          Matt Foley made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          90d 13h 30m 1 Tsz Wo Nicholas Sze 16/Dec/12 08:41
          Resolved Resolved Closed Closed
          149d 20h 34m 1 Matt Foley 15/May/13 05:15

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development