Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-3024

LocalizerRunner should give DIE action when all resources are localized

    Details

      Description

      We have observed that LocalizerRunner always gives a LIVE action at the end of localization process.

      The problem is findNextResource() can return null even when pending was not empty prior to the call. This method removes localized resources from pending, therefore we should check the return value, and gives DIE action when it returns null.

      1. YARN-3024.01.patch
        3 kB
        Chengbing Liu
      2. YARN-3024.02.patch
        4 kB
        Chengbing Liu
      3. YARN-3024.03.patch
        8 kB
        Chengbing Liu
      4. YARN-3024.04.patch
        15 kB
        Chengbing Liu

        Issue Links

          Activity

          Hide
          chengbing.liu Chengbing Liu added a comment -

          Upload patch.

          Show
          chengbing.liu Chengbing Liu added a comment - Upload patch.
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Fixed tests accordingly.

          Show
          chengbing.liu Chengbing Liu added a comment - Fixed tests accordingly.
          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/12691069/YARN-3024.01.patch
          against trunk revision ae91b13.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:

          org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6290//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6290//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/12691069/YARN-3024.01.patch against trunk revision ae91b13. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.TestResourceLocalizationService Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6290//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6290//console This message is automatically generated.
          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/12691073/YARN-3024.02.patch
          against trunk revision ae91b13.

          +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. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6291//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6291//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/12691073/YARN-3024.02.patch against trunk revision ae91b13. +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 . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6291//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6291//console This message is automatically generated.
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Updated patch.

          I refactored LocalizerRunner#update(), separating the following two phase:

          • first read and process resource statuses from the localizer through heartbeat
          • then find the next resource to be localized and send it through response

          Also, in the original code base, there is a small problem about the response action. Now if one of the following conditions is met, the response action will be DIE:

          • Got at least one FETCH_FAILURE
          • findNextResource() returns null, and pending is empty
          Show
          chengbing.liu Chengbing Liu added a comment - Updated patch. I refactored LocalizerRunner#update() , separating the following two phase: first read and process resource statuses from the localizer through heartbeat then find the next resource to be localized and send it through response Also, in the original code base, there is a small problem about the response action. Now if one of the following conditions is met, the response action will be DIE: Got at least one FETCH_FAILURE findNextResource() returns null, and pending is empty
          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/12691878/YARN-3024.03.patch
          against trunk revision c4cba61.

          +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. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager:

          org.apache.hadoop.yarn.server.nodemanager.containermanager.logaggregation.TestLogAggregationService

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6318//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6318//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/12691878/YARN-3024.03.patch against trunk revision c4cba61. +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 . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: org.apache.hadoop.yarn.server.nodemanager.containermanager.logaggregation.TestLogAggregationService Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6318//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6318//console This message is automatically generated.
          Hide
          xgong Xuan Gong added a comment -

          Chengbing Liu Thanks for working on this ticket. I am starting to look at the patch. Overall looks good, but

          • on the latest patch, looks like you change the logic for
                      case FETCH_PENDING:
                        break;
            

          Originally, we will directly return the response with LocalizerAction.LIVE
          But now we have to do:

                LocalResource next = findNextResource();
                if (next != null) {
                  try {
                    ResourceLocalizationSpec resource =
                        NodeManagerBuilderUtils.newResourceLocalizationSpec(next,
                          getPathForLocalization(next));
                    rsrcs.add(resource);
                  } catch (IOException e) {
                    LOG.error("local path for PRIVATE localization could not be " +
                      "found. Disks might have failed.", e);
                  } catch (URISyntaxException e) {
                      //TODO fail? Already translated several times...
                  }
                } else if (pending.isEmpty()) {
                  // TODO: Synchronization
                  action = LocalizerAction.DIE;
                }
          
                response.setLocalizerAction(action);
                response.setResourceSpecs(rsrcs);
                return response;
          
          • Could you fix this format
            +      if (action == LocalizerAction.DIE) {
            +	response.setLocalizerAction(action);
            +	return response;
            +      }
            
          Show
          xgong Xuan Gong added a comment - Chengbing Liu Thanks for working on this ticket. I am starting to look at the patch. Overall looks good, but on the latest patch, looks like you change the logic for case FETCH_PENDING: break ; Originally, we will directly return the response with LocalizerAction.LIVE But now we have to do: LocalResource next = findNextResource(); if (next != null ) { try { ResourceLocalizationSpec resource = NodeManagerBuilderUtils.newResourceLocalizationSpec(next, getPathForLocalization(next)); rsrcs.add(resource); } catch (IOException e) { LOG.error( "local path for PRIVATE localization could not be " + "found. Disks might have failed." , e); } catch (URISyntaxException e) { //TODO fail? Already translated several times... } } else if (pending.isEmpty()) { // TODO: Synchronization action = LocalizerAction.DIE; } response.setLocalizerAction(action); response.setResourceSpecs(rsrcs); return response; Could you fix this format + if (action == LocalizerAction.DIE) { + response.setLocalizerAction(action); + return response; + }
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Xuan Gong Thanks for reviewing.

          on the latest patch, looks like you change the logic for

          The logic of giving resources to be localized is actually changed.

          Previously, LocalizedRunner does not give the next resource to ContainerLocalizer until the previous has been downloaded.

          In this patch, LocalizedRunner will not wait for the previous resource to be downloaded. ContainerLocalizer can handle that by submitting the download task to its CompletionService, which is able to queue those tasks, before executing them. The download thread pool of the CompletionService remains a single thread executor.

          Therefore, it is possible that ContainerLocalizer sends multiple LocalResourceStatus to LocalizerRunner through heartbeat. In this case, I think we should try to find the next resources to be localized even when getting FETCH_PENDING.

          I have tested it on a real cluster. I specified a large archive which should take a long time to be localized. The result shows they were getting localized serially, and one heartbeat contained multiple statuses of small files (thus reducing the number of heartbeat).

          Could you fix this format

          My bad, I will fix this.

          Show
          chengbing.liu Chengbing Liu added a comment - Xuan Gong Thanks for reviewing. on the latest patch, looks like you change the logic for The logic of giving resources to be localized is actually changed. Previously, LocalizedRunner does not give the next resource to ContainerLocalizer until the previous has been downloaded. In this patch, LocalizedRunner will not wait for the previous resource to be downloaded. ContainerLocalizer can handle that by submitting the download task to its CompletionService, which is able to queue those tasks, before executing them. The download thread pool of the CompletionService remains a single thread executor. Therefore, it is possible that ContainerLocalizer sends multiple LocalResourceStatus to LocalizerRunner through heartbeat. In this case, I think we should try to find the next resources to be localized even when getting FETCH_PENDING. I have tested it on a real cluster. I specified a large archive which should take a long time to be localized. The result shows they were getting localized serially, and one heartbeat contained multiple statuses of small files (thus reducing the number of heartbeat). Could you fix this format My bad, I will fix this.
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Xuan Gong Could you please review the changed logic described in my last comment? This patch will save at least one second for each localization process.

          Show
          chengbing.liu Chengbing Liu added a comment - Xuan Gong Could you please review the changed logic described in my last comment? This patch will save at least one second for each localization process.
          Hide
          xgong Xuan Gong added a comment -

          Yes, I will take a look shortly.

          Show
          xgong Xuan Gong added a comment - Yes, I will take a look shortly.
          Hide
          xgong Xuan Gong added a comment -

          Chengbing Liu

          Could you please review the changed logic described in my last comment?

          The changed logic looks fine for me. Even if we submit multiple requests before the previous request is finished, those requests will be queued to process.

          So, can we add the test cases to verify this changed behavior ?

          Others are looks good to me.

          Show
          xgong Xuan Gong added a comment - Chengbing Liu Could you please review the changed logic described in my last comment? The changed logic looks fine for me. Even if we submit multiple requests before the previous request is finished, those requests will be queued to process. So, can we add the test cases to verify this changed behavior ? Others are looks good to me.
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Modified the test to verify the changed logic.

          Show
          chengbing.liu Chengbing Liu added a comment - Modified the test to verify the changed logic.
          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/12693845/YARN-3024.04.patch
          against trunk revision 786dbdf.

          +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. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager.

          Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6388//testReport/
          Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6388//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/12693845/YARN-3024.04.patch against trunk revision 786dbdf. +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 . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6388//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6388//console This message is automatically generated.
          Hide
          xgong Xuan Gong added a comment -

          +1 LGTM. Will commit it

          Show
          xgong Xuan Gong added a comment - +1 LGTM. Will commit it
          Hide
          xgong Xuan Gong added a comment -

          Committed to trunk/branch-2. Thanks, Chengbing !

          Show
          xgong Xuan Gong added a comment - Committed to trunk/branch-2. Thanks, Chengbing !
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #6928 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6928/)
          YARN-3024. LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          • hadoop-yarn-project/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #6928 (See https://builds.apache.org/job/Hadoop-trunk-Commit/6928/ ) YARN-3024 . LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java hadoop-yarn-project/CHANGES.txt
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Thanks Xuan Gong for the review!

          Show
          chengbing.liu Chengbing Liu added a comment - Thanks Xuan Gong for the review!
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #85 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/85/)
          YARN-3024. LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #85 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/85/ ) YARN-3024 . LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #819 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/819/)
          YARN-3024. LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          • hadoop-yarn-project/CHANGES.txt
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #819 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/819/ ) YARN-3024 . LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java hadoop-yarn-project/CHANGES.txt
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Hdfs-trunk #2017 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2017/)
          YARN-3024. LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2017 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2017/ ) YARN-3024 . LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk-Java8 #82 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/82/)
          YARN-3024. LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk-Java8 #82 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/82/ ) YARN-3024 . LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #86 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/86/)
          YARN-3024. LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #86 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/86/ ) YARN-3024 . LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #2036 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2036/)
          YARN-3024. LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127)

          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java
          • hadoop-yarn-project/CHANGES.txt
          • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2036 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2036/ ) YARN-3024 . LocalizerRunner should give DIE action when all resources are (xgong: rev 0d6bd62102f94c55d59f7a0a86a684e99d746127) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ResourceLocalizationService.java
          Hide
          kasha Karthik Kambatla added a comment -

          Chengbing Liu, Xuan Gong - is there a particular reason for changing .equals() calls to ==? Also, the patch seems to add some TODOs. Were there any follow-up JIRAs filed?

          Show
          kasha Karthik Kambatla added a comment - Chengbing Liu , Xuan Gong - is there a particular reason for changing .equals() calls to == ? Also, the patch seems to add some TODOs. Were there any follow-up JIRAs filed?
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Karthik Kambatla, I think we should use == for enum members, for it is both null safe and it saves a function call.
          The TODOs were there before this patch. Previously there were 5 TODOs. I did some refactoring to remove the duplicated code, and now there are 3. Would you like me to create JIRAs to follow the issue?

          Show
          chengbing.liu Chengbing Liu added a comment - Karthik Kambatla , I think we should use == for enum members, for it is both null safe and it saves a function call. The TODOs were there before this patch. Previously there were 5 TODOs. I did some refactoring to remove the duplicated code, and now there are 3. Would you like me to create JIRAs to follow the issue?
          Hide
          kasha Karthik Kambatla added a comment -

          Chengbing Liu - thanks for the clarifications. Makes sense.

          For the TODOs, it would be nice to have follow-up JIRAs. If it is not too much trouble, can you create them so interested contributors could follow up?

          Show
          kasha Karthik Kambatla added a comment - Chengbing Liu - thanks for the clarifications. Makes sense. For the TODOs, it would be nice to have follow-up JIRAs. If it is not too much trouble, can you create them so interested contributors could follow up?
          Hide
          chengbing.liu Chengbing Liu added a comment -

          Karthik Kambatla, I created YARN-3396 to track the URISyntaxException issue.
          For multiple downloads per ContainerLocalizer, I found YARN-665 already created.
          As for the other TODO, i.e. synchronization, I don't see any need for this. I think we can safely remove this one.

          Show
          chengbing.liu Chengbing Liu added a comment - Karthik Kambatla , I created YARN-3396 to track the URISyntaxException issue. For multiple downloads per ContainerLocalizer, I found YARN-665 already created. As for the other TODO, i.e. synchronization, I don't see any need for this. I think we can safely remove this one.
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Added this to 2.6.1 as a dependency for YARN-3464.

          Patch applied cleanly for the most part, except for one non-trivial merge conflict.

          Pulled this into 2.6.1. Ran compilation and TestResourceLocalizationService before the push.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Added this to 2.6.1 as a dependency for YARN-3464 . Patch applied cleanly for the most part, except for one non-trivial merge conflict. Pulled this into 2.6.1. Ran compilation and TestResourceLocalizationService before the push.

            People

            • Assignee:
              chengbing.liu Chengbing Liu
              Reporter:
              chengbing.liu Chengbing Liu
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development