Uploaded image for project: 'Hadoop Map/Reduce'
  1. Hadoop Map/Reduce
  2. MAPREDUCE-6871

Allow users to specify racks and nodes for strict locality for AMs

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-alpha4
    • Component/s: client
    • Labels:
      None

      Description

      YARN-6050 fixed the YARN API to allow multiple ResourceRequest's when submitting an AM so that you can actually do rack or node locality. We should allow MapReduce users to take advantage of this by exposing this functionality in some way. The raw YARN API allows for a lot of flexibility (e.g. different resources per request, etc), but we don't necessarily want to allow the user to do too much here so they don't shoot themselves in the foot and we don't make this overly complicated.

      I propose we allow users to specify racks and nodes for strict locality. This would allow users to restrict an MR AM to specific racks and/or nodes. We could add a new property, mapreduce.job.am.resource-request.strict.locality, which takes a comma-separated list of entries like:

      • /<rack>
      • /<rack>/<node>
      • <node> (assumes /default-rack)

      MapReduce would then use this information to create the corresponding ResourceRequest's.

      For example, mapreduce.job.am.resource-request.strict.locality=/rack1/node1 would create the following ResourceRequest's:

      • resourceName=ANY, relaxLocality=false, capability=<X,Y>
      • resourceName=/rack1, relaxLocality=false, capability=<X,Y>
      • resourceName=node1, relaxLocality=true, capability=<X,Y>

      By default, the property would be unset, and you'd get the normal ANY ResourceRequest.

      1. MAPREDUCE-6871.001.patch
        14 kB
        Robert Kanter
      2. MAPREDUCE-6871.002.patch
        16 kB
        Robert Kanter
      3. MAPREDUCE-6871.003.patch
        18 kB
        Robert Kanter
      4. MAPREDUCE-6871.004.patch
        17 kB
        Robert Kanter
      5. MAPREDUCE-6871.005.patch
        17 kB
        Robert Kanter
      6. MAPREDUCE-6871.005.patch
        17 kB
        Robert Kanter

        Issue Links

          Activity

          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11622 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11622/)
          MAPREDUCE-6871. Allow users to specify racks and nodes for strict (rkanter: rev 3721cfe1fbd98c5b6aa46aefdfcf62276c28c4a4)

          • (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java
          • (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestYARNRunner.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11622 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11622/ ) MAPREDUCE-6871 . Allow users to specify racks and nodes for strict (rkanter: rev 3721cfe1fbd98c5b6aa46aefdfcf62276c28c4a4) (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapred/TestYARNRunner.java
          Hide
          rkanter Robert Kanter added a comment -

          Thanks Haibo Chen and Karthik Kambatla for reviews. Committed to branch-2 and trunk!

          Show
          rkanter Robert Kanter added a comment - Thanks Haibo Chen and Karthik Kambatla for reviews. Committed to branch-2 and trunk!
          Hide
          kasha Karthik Kambatla added a comment -

          it does need to be a map. If someone specifies /rack1,/rack1/node1, we'd need to update the already created request for rack1 to have the strict locality set to false.

          Gotcha. That makes sense.

          String.format does make it more readable.

          +1

          Show
          kasha Karthik Kambatla added a comment - it does need to be a map. If someone specifies /rack1,/rack1/node1, we'd need to update the already created request for rack1 to have the strict locality set to false. Gotcha. That makes sense. String.format does make it more readable. +1
          Hide
          rkanter Robert Kanter added a comment -

          Test failure unrelated

          Show
          rkanter Robert Kanter added a comment - Test failure unrelated
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 45s Maven dependency ordering for branch
          +1 mvninstall 12m 58s trunk passed
          +1 compile 1m 34s trunk passed
          +1 checkstyle 0m 28s trunk passed
          +1 mvnsite 0m 53s trunk passed
          +1 mvneclipse 0m 34s trunk passed
          -1 findbugs 0m 48s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core in trunk has 3 extant Findbugs warnings.
          +1 javadoc 0m 35s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 0m 40s the patch passed
          +1 compile 1m 31s the patch passed
          +1 javac 1m 31s the patch passed
          +1 checkstyle 0m 25s the patch passed
          +1 mvnsite 0m 46s the patch passed
          +1 mvneclipse 0m 28s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 20s the patch passed
          +1 javadoc 0m 29s the patch passed
          +1 unit 2m 47s hadoop-mapreduce-client-core in the patch passed.
          -1 unit 110m 9s hadoop-mapreduce-client-jobclient in the patch failed.
          +1 asflicense 0m 27s The patch does not generate ASF License warnings.
          139m 18s



          Reason Tests
          Failed junit tests hadoop.mapred.TestLocalMRNotification



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864311/MAPREDUCE-6871.005.patch
          JIRA Issue MAPREDUCE-6871
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 797fc76746c3 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c0cf11e
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/artifact/patchprocess/branch-findbugs-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core-warnings.html
          unit https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-jobclient.txt
          unit test logs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-jobclient.txt
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 45s Maven dependency ordering for branch +1 mvninstall 12m 58s trunk passed +1 compile 1m 34s trunk passed +1 checkstyle 0m 28s trunk passed +1 mvnsite 0m 53s trunk passed +1 mvneclipse 0m 34s trunk passed -1 findbugs 0m 48s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core in trunk has 3 extant Findbugs warnings. +1 javadoc 0m 35s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 0m 40s the patch passed +1 compile 1m 31s the patch passed +1 javac 1m 31s the patch passed +1 checkstyle 0m 25s the patch passed +1 mvnsite 0m 46s the patch passed +1 mvneclipse 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 20s the patch passed +1 javadoc 0m 29s the patch passed +1 unit 2m 47s hadoop-mapreduce-client-core in the patch passed. -1 unit 110m 9s hadoop-mapreduce-client-jobclient in the patch failed. +1 asflicense 0m 27s The patch does not generate ASF License warnings. 139m 18s Reason Tests Failed junit tests hadoop.mapred.TestLocalMRNotification Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864311/MAPREDUCE-6871.005.patch JIRA Issue MAPREDUCE-6871 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 797fc76746c3 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c0cf11e Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/artifact/patchprocess/branch-findbugs-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core-warnings.html unit https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-jobclient.txt unit test logs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/artifact/patchprocess/patch-unit-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-jobclient.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6962/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          I was talking to Daniel Templeton about regexes and he suggested using String.format to make the regex as one String, which makes it easier to read. The 005 patch does that.

          Show
          rkanter Robert Kanter added a comment - I was talking to Daniel Templeton about regexes and he suggested using String.format to make the regex as one String, which makes it easier to read. The 005 patch does that.
          Hide
          rkanter Robert Kanter added a comment -

          I was talking to Daniel Templeton about regexes and he suggested using String.format to make the regex as one String, which makes it easier to read. The 005 patch does that.

          Show
          rkanter Robert Kanter added a comment - I was talking to Daniel Templeton about regexes and he suggested using String.format to make the regex as one String, which makes it easier to read. The 005 patch does that.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 2m 1s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 54s Maven dependency ordering for branch
          +1 mvninstall 13m 0s trunk passed
          +1 compile 1m 33s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 48s trunk passed
          +1 mvneclipse 0m 34s trunk passed
          -1 findbugs 0m 54s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core in trunk has 3 extant Findbugs warnings.
          +1 javadoc 0m 32s trunk passed
          0 mvndep 0m 6s Maven dependency ordering for patch
          +1 mvninstall 0m 45s the patch passed
          +1 compile 1m 41s the patch passed
          +1 javac 1m 41s the patch passed
          +1 checkstyle 0m 25s the patch passed
          +1 mvnsite 0m 49s the patch passed
          +1 mvneclipse 0m 32s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 20s the patch passed
          +1 javadoc 0m 27s the patch passed
          +1 unit 2m 42s hadoop-mapreduce-client-core in the patch passed.
          +1 unit 101m 40s hadoop-mapreduce-client-jobclient in the patch passed.
          +1 asflicense 0m 40s The patch does not generate ASF License warnings.
          132m 59s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ac17dc
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864170/MAPREDUCE-6871.004.patch
          JIRA Issue MAPREDUCE-6871
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 8fdccacbb863 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c154935
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6961/artifact/patchprocess/branch-findbugs-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core-warnings.html
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6961/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6961/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 2m 1s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 54s Maven dependency ordering for branch +1 mvninstall 13m 0s trunk passed +1 compile 1m 33s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 48s trunk passed +1 mvneclipse 0m 34s trunk passed -1 findbugs 0m 54s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core in trunk has 3 extant Findbugs warnings. +1 javadoc 0m 32s trunk passed 0 mvndep 0m 6s Maven dependency ordering for patch +1 mvninstall 0m 45s the patch passed +1 compile 1m 41s the patch passed +1 javac 1m 41s the patch passed +1 checkstyle 0m 25s the patch passed +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 32s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 20s the patch passed +1 javadoc 0m 27s the patch passed +1 unit 2m 42s hadoop-mapreduce-client-core in the patch passed. +1 unit 101m 40s hadoop-mapreduce-client-jobclient in the patch passed. +1 asflicense 0m 40s The patch does not generate ASF License warnings. 132m 59s Subsystem Report/Notes Docker Image:yetus/hadoop:0ac17dc JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864170/MAPREDUCE-6871.004.patch JIRA Issue MAPREDUCE-6871 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 8fdccacbb863 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c154935 Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6961/artifact/patchprocess/branch-findbugs-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core-warnings.html Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6961/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6961/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          The 004 patch:

          • Renames the config to mapreduce.job.am.strict-locality
          • Adds a debug logging check
          • Removes the redundant label setting code in a test (I think that's a holdover from an earlier version)

          Karthik Kambatla, for your feedback about the rackRequests: it does need to be a map. If someone specifies /rack1,/rack1/node1, we'd need to update the already created request for rack1 to have the strict locality set to false.

          Show
          rkanter Robert Kanter added a comment - The 004 patch: Renames the config to mapreduce.job.am.strict-locality Adds a debug logging check Removes the redundant label setting code in a test (I think that's a holdover from an earlier version) Karthik Kambatla , for your feedback about the rackRequests : it does need to be a map. If someone specifies /rack1,/rack1/node1 , we'd need to update the already created request for rack1 to have the strict locality set to false.
          Hide
          kasha Karthik Kambatla added a comment -

          Thanks for updating the patch, Robert. Few minor comments:

          1. For the config, the resource-request part might not be adding much information. How about we call it just mapreduce.job.am.strict-locality and update the corresponding field in MRJobConfig as well? Sorry for not pointing it out in my previous review.
          2. YARNRunner.generateResourceRequests:
            1. Let us wrap the debug log in a isDebugEnabled guard.
              LOG.debug("AppMaster capability = " + capability);
            2. rackRequests need not be a map. It could be a set as well.
          3. In the tests, verifyResourceRequestLocality sets node label expression multiple times. Is that required?
          Show
          kasha Karthik Kambatla added a comment - Thanks for updating the patch, Robert. Few minor comments: For the config, the resource-request part might not be adding much information. How about we call it just mapreduce.job.am.strict-locality and update the corresponding field in MRJobConfig as well? Sorry for not pointing it out in my previous review. YARNRunner.generateResourceRequests: Let us wrap the debug log in a isDebugEnabled guard. LOG.debug( "AppMaster capability = " + capability); rackRequests need not be a map. It could be a set as well. In the tests, verifyResourceRequestLocality sets node label expression multiple times. Is that required?
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 45s Maven dependency ordering for branch
          +1 mvninstall 15m 54s trunk passed
          +1 compile 2m 9s trunk passed
          +1 checkstyle 0m 35s trunk passed
          +1 mvnsite 1m 8s trunk passed
          +1 mvneclipse 0m 40s trunk passed
          +1 findbugs 1m 46s trunk passed
          +1 javadoc 0m 45s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 7s the patch passed
          +1 compile 2m 23s the patch passed
          +1 javac 2m 23s the patch passed
          +1 checkstyle 0m 36s the patch passed
          +1 mvnsite 1m 10s the patch passed
          +1 mvneclipse 0m 38s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 8s the patch passed
          +1 javadoc 0m 41s the patch passed
          +1 unit 3m 1s hadoop-mapreduce-client-core in the patch passed.
          +1 unit 104m 1s hadoop-mapreduce-client-jobclient in the patch passed.
          +1 asflicense 0m 29s The patch does not generate ASF License warnings.
          141m 24s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:612578f
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863188/MAPREDUCE-6871.003.patch
          JIRA Issue MAPREDUCE-6871
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 5271f9ca5c19 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0cab572
          Default Java 1.8.0_121
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6954/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6954/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 45s Maven dependency ordering for branch +1 mvninstall 15m 54s trunk passed +1 compile 2m 9s trunk passed +1 checkstyle 0m 35s trunk passed +1 mvnsite 1m 8s trunk passed +1 mvneclipse 0m 40s trunk passed +1 findbugs 1m 46s trunk passed +1 javadoc 0m 45s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 7s the patch passed +1 compile 2m 23s the patch passed +1 javac 2m 23s the patch passed +1 checkstyle 0m 36s the patch passed +1 mvnsite 1m 10s the patch passed +1 mvneclipse 0m 38s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 8s the patch passed +1 javadoc 0m 41s the patch passed +1 unit 3m 1s hadoop-mapreduce-client-core in the patch passed. +1 unit 104m 1s hadoop-mapreduce-client-jobclient in the patch passed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 141m 24s Subsystem Report/Notes Docker Image:yetus/hadoop:612578f JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12863188/MAPREDUCE-6871.003.patch JIRA Issue MAPREDUCE-6871 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 5271f9ca5c19 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0cab572 Default Java 1.8.0_121 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6954/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6954/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          Thanks for taking a look Karthik Kambatla. I've addressed your feedback.

          The 003 patch:

          • Moves the code to it's own method
          • Replaces the iterating lookup with a hashmap
          Show
          rkanter Robert Kanter added a comment - Thanks for taking a look Karthik Kambatla . I've addressed your feedback. The 003 patch: Moves the code to it's own method Replaces the iterating lookup with a hashmap
          Hide
          kasha Karthik Kambatla added a comment -

          Thanks for working on this, Robert. The patch looks mostly good. The tests in particular are quite comprehensive. Few minor comments:

          1. findResourceRequest, in its current form, iterates through all ResourceRequests. Instead, would it be more efficient to track the set of racks that have been included so far?
          2. Can we move the AM container's ResourceRequest construction into its own method for better readability?
          Show
          kasha Karthik Kambatla added a comment - Thanks for working on this, Robert. The patch looks mostly good. The tests in particular are quite comprehensive. Few minor comments: findResourceRequest, in its current form, iterates through all ResourceRequests. Instead, would it be more efficient to track the set of racks that have been included so far? Can we move the AM container's ResourceRequest construction into its own method for better readability?
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 28s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 59s Maven dependency ordering for branch
          +1 mvninstall 18m 3s trunk passed
          +1 compile 2m 28s trunk passed
          +1 checkstyle 0m 39s trunk passed
          +1 mvnsite 1m 11s trunk passed
          +1 mvneclipse 0m 37s trunk passed
          +1 findbugs 1m 39s trunk passed
          +1 javadoc 0m 48s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 0m 59s the patch passed
          +1 compile 2m 18s the patch passed
          +1 javac 2m 18s the patch passed
          +1 checkstyle 0m 34s the patch passed
          +1 mvnsite 1m 7s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 54s the patch passed
          +1 javadoc 0m 42s the patch passed
          +1 unit 3m 22s hadoop-mapreduce-client-core in the patch passed.
          +1 unit 108m 19s hadoop-mapreduce-client-jobclient in the patch passed.
          +1 asflicense 0m 31s The patch does not generate ASF License warnings.
          148m 29s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12862389/MAPREDUCE-6871.002.patch
          JIRA Issue MAPREDUCE-6871
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a9c964f291ee 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / a49fac5
          Default Java 1.8.0_121
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6949/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6949/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 28s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 59s Maven dependency ordering for branch +1 mvninstall 18m 3s trunk passed +1 compile 2m 28s trunk passed +1 checkstyle 0m 39s trunk passed +1 mvnsite 1m 11s trunk passed +1 mvneclipse 0m 37s trunk passed +1 findbugs 1m 39s trunk passed +1 javadoc 0m 48s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 0m 59s the patch passed +1 compile 2m 18s the patch passed +1 javac 2m 18s the patch passed +1 checkstyle 0m 34s the patch passed +1 mvnsite 1m 7s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 54s the patch passed +1 javadoc 0m 42s the patch passed +1 unit 3m 22s hadoop-mapreduce-client-core in the patch passed. +1 unit 108m 19s hadoop-mapreduce-client-jobclient in the patch passed. +1 asflicense 0m 31s The patch does not generate ASF License warnings. 148m 29s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12862389/MAPREDUCE-6871.002.patch JIRA Issue MAPREDUCE-6871 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a9c964f291ee 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a49fac5 Default Java 1.8.0_121 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6949/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6949/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          Thanks Haibo Chen for taking a look. I've addressed your feedback.

          The 002 patch:

          • Uses a regex for parsing the config. I think this makes the code much clearer and more readable because I'm also using named captured groups.
          • Renames the config property as Haibo Chen suggested.
          • Refactored the test code to split it up into different tests and added a few more cases.
          Show
          rkanter Robert Kanter added a comment - Thanks Haibo Chen for taking a look. I've addressed your feedback. The 002 patch: Uses a regex for parsing the config. I think this makes the code much clearer and more readable because I'm also using named captured groups. Renames the config property as Haibo Chen suggested. Refactored the test code to split it up into different tests and added a few more cases.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Robert Kanter for the patch! A few comments
          1) Do you think having regular expression for the configuration is more robust? rack1/node2 will be treated as a node name with current implementation. If this will cause application failure later, better we catch it early before submission.
          2) Maybe rename mapreduce.job.am.resource-request.strict.locality to mapreduce.job.am.resource-request-strict-locality ?
          3) The different test cases can be broken into different test methods individually, so that one test case failure won't disguise failures in following test cases.

          Show
          haibochen Haibo Chen added a comment - Thanks Robert Kanter for the patch! A few comments 1) Do you think having regular expression for the configuration is more robust? rack1/node2 will be treated as a node name with current implementation. If this will cause application failure later, better we catch it early before submission. 2) Maybe rename mapreduce.job.am.resource-request.strict.locality to mapreduce.job.am.resource-request-strict-locality ? 3) The different test cases can be broken into different test methods individually, so that one test case failure won't disguise failures in following test cases.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 51s Maven dependency ordering for branch
          +1 mvninstall 16m 25s trunk passed
          +1 compile 2m 20s trunk passed
          +1 checkstyle 0m 40s trunk passed
          +1 mvnsite 1m 18s trunk passed
          +1 mvneclipse 0m 35s trunk passed
          +1 findbugs 1m 34s trunk passed
          +1 javadoc 0m 45s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 12s the patch passed
          +1 compile 2m 27s the patch passed
          +1 javac 2m 27s the patch passed
          +1 checkstyle 0m 38s the patch passed
          +1 mvnsite 1m 7s the patch passed
          +1 mvneclipse 0m 32s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 59s the patch passed
          -1 javadoc 0m 33s hadoop-mapreduce-client-core in the patch failed.
          +1 unit 3m 21s hadoop-mapreduce-client-core in the patch passed.
          +1 unit 114m 45s hadoop-mapreduce-client-jobclient in the patch passed.
          +1 asflicense 0m 25s The patch does not generate ASF License warnings.
          153m 28s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861073/MAPREDUCE-6871.001.patch
          JIRA Issue MAPREDUCE-6871
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 64f5fe4d508b 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 15e3873
          Default Java 1.8.0_121
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6934/artifact/patchprocess/patch-javadoc-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt
          Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6934/testReport/
          modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client
          Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6934/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. 0 mvndep 0m 51s Maven dependency ordering for branch +1 mvninstall 16m 25s trunk passed +1 compile 2m 20s trunk passed +1 checkstyle 0m 40s trunk passed +1 mvnsite 1m 18s trunk passed +1 mvneclipse 0m 35s trunk passed +1 findbugs 1m 34s trunk passed +1 javadoc 0m 45s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 12s the patch passed +1 compile 2m 27s the patch passed +1 javac 2m 27s the patch passed +1 checkstyle 0m 38s the patch passed +1 mvnsite 1m 7s the patch passed +1 mvneclipse 0m 32s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 59s the patch passed -1 javadoc 0m 33s hadoop-mapreduce-client-core in the patch failed. +1 unit 3m 21s hadoop-mapreduce-client-core in the patch passed. +1 unit 114m 45s hadoop-mapreduce-client-jobclient in the patch passed. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 153m 28s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12861073/MAPREDUCE-6871.001.patch JIRA Issue MAPREDUCE-6871 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 64f5fe4d508b 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 15e3873 Default Java 1.8.0_121 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6934/artifact/patchprocess/patch-javadoc-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-core.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6934/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/6934/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          rkanter Robert Kanter added a comment -

          The 001 patch:

          • Adds the new property with the behavior as described above
          • Adds unit tests
          • I also verified that it behaves correctly in a cluster
          Show
          rkanter Robert Kanter added a comment - The 001 patch: Adds the new property with the behavior as described above Adds unit tests I also verified that it behaves correctly in a cluster

            People

            • Assignee:
              rkanter Robert Kanter
              Reporter:
              rkanter Robert Kanter
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development