In doing some profiling, it looks as though the bulk of the runtime occurs copying the YARN counters into the TaskReport. Since this value isn't used in the web display for the tasks page, turn it off for webapp rendering.

For 404k map tasks, this reduces the loop from about 50 seconds down to about 5 seconds.

Initial version to remove copying YARN counters to TaskReports.

Submit for testing.

-1 overall

This message was automatically generated.

-1 overall

This message was automatically generated.

• Fix unit test issues

-1 overall

This message was automatically generated.

• Missed one accidentally deleted line. Restore it.

-1 overall

This message was automatically generated.

RE: checkstyle

File was previously over 2000 lines.

Found task counters section in UI. Need to figure out alternative to speedup.

So, from everything I can tell, the HTML/Javascript Table generated in the for loop of HsTasksBlock#render() ends up being slow for really large numbers of Tasks (e.g. taking up 102 out of 109 seconds for my 751k tasks .jhist file).

The bulk of that time taken up in the method with the signature:

TypeConverter#toYarn(org.apache.hadoop.mapreduce.Counters counters)

Given the CounterGroup/Counters are pretty simple and from what I can figure out through crude profiling is that the outer loop gets called some 4.5 million times and the inner loop nets out around 37 million times. Even as fast as it may be, the sheer number of counters adds up over so many tasks.

The only solution I can see offhand would be to defer this conversion to when the user clicks on the Counters link in the Tasks page. This would mean substituting the calls like:

report.setCounters(TypeConverter.toYarn(getCounters()));

in CompletedTaskAttempt (and similar places if any) and adjusting the JHS method that accesses the counters to do the parsing at link click time.

Robert Kanter, Jason Lowe any thoughts on the above suggested change? I don't think there will be any side effects outside of JHS, but this just occurred to me today so I thought I'd get feedback early.

• Change direct setCounters()/getCounters() to use late-binding combo setRawCounters()/getCounters() for conversion.
• Profiling-wise for 751k tasks, the loop in HsTasksBlock#render() takes ~100-110 seconds without this fix and around 20-25 seconds with this fix.

Uploading identical patch to get Jenkins run.

Allen Wittenauer any idea why my last two patches haven't gotten a Jenkins run?

any idea why my last two patches haven't gotten a Jenkins run?

Yes.

-1 overall

This message was automatically generated.

Thanks! That's what I get for staring at it for too long.

RE: checkstyle

• Redundant 'public' modified matches previous declarations
• For long lines in method names, those aren't fixed by breaking into two lines. Keeping a single line for readability.

Jason Lowe, this is the second JHS fix. I believe this one is also no-impact other than the speedup for jobs with large tasks. Please review at your leisure. Thanks.

• Fix some checkstyle issues
• Once counters is converted, free up rawCounters

-1 overall

This message was automatically generated.

Exact same checkstyle stuff as before:

RE: checkstyle

• Redundant 'public' modified matches previous declarations
• For long lines in method names, those aren't fixed by breaking into two lines. Keeping a single line for readability.

RE: No new unit tests

Should be functionally identical. Not sure if a test would be useful here.

Thanks for the analysis and patch, Ray! Sorry for the delay in reviewing.

General comment: If the file is going to import a particular version of Counters then I don't see the need to go changing it in many but not all places in the file. It should either remove the import and explicitly list the fully qualified classname in each instance or only list it when it's not using the one that was imported. My preference is for the latter, as this is already done for mapred vs. mapreduce and similar situations like this. It also makes the patch smaller.

In the PBImpl files, I don't see why we need a separate boolean to track whether we've converted rawCounters into counters. If counters is null and rawCounters isn't, we should convert and null out rawCounters otherwise we shouldn't. Am I missing the case where the separate boolean is really needed to distinguish?

Also in the PBImpl files, it looks like building the protobuf is incorrect if someone sets the raw counters and then tries to build. The build method simply checks if counters is not null to see if they should be included, but they will be null if only rawCounters was set and nobody called getCounters before calling getProto. That's an example where unit tests would have been useful, since the patch adds a new interface to those PBImpls that isn't currently covered by tests.

Thanks! I'll work on those items.

• Add two unit tests
• Move rawCounter to Counter conversion into single method. Call method in appropriate places
• Remove unneeded boolean variable

-1 overall

This message was automatically generated.

RE: checkstyle

Matches surrounding method definitions.

Thanks for updating the patch, Ray!

In the unit tests, the implicit raw counter test should be checking if the proto has counters rather than calling getCounters on the report. I think that test would have passed even with the original code, since getCounters() checks for converting raw counters. The bug I mentioned above was that setting the raw counters and then building the proto would cause the counters to be missing from the resulting protobuf. The test should verify that setting the raw counters causes the counters to appear in the generated protobuf.

Another thing occurred to me: what should the behavior be when someone sets the counters and the raw counters? Arguably it should take the last setting, but I believe currently if one sets counters and then the raw counters it will ignore the raw counters. We probably should be calling setCounters(null) in setRawCounters and vice-versa. Would be nice to have a test to cover these behaviors as well.

Thanks again. I'll update the tests.

And I was assuming for now that doing setCounters would always override any setting by setRawCounters--preserving legacy behavior. I'll update the tests to cover that as well.

• Update setCounters() and setRawCounters() to override each other. Thus, the "last set" will be used.
• Add several more unit tests to test various orders of setting counters and/or setting raw counters.

-1 overall

This message was automatically generated.

RE: checkstyle

Same errors as before. Keep matching style for now.

Thanks for updating the patch. I think we're almost there.

This should either call setCounters(null) instead of manipulating counters and the builder directly or unconditionally call builder.clearCounters, otherwise we will continue to hold onto the old counters in the builder for no good reason.

+  public void setRawCounters(org.apache.hadoop.mapreduce.Counters rCounters) {
+    maybeInitBuilder();
+    if (counters == null) {
+      builder.clearCounters();
+    }
+    this.counters = null;
+    this.rawCounters = rCounters;
+  }

The unit tests should be using the assert methods from junit rather than native Java asserts, as the latter implicitly relies on the JVM being run with assertions enabled for the unit tests to function properly. Failure to do so will have every test passing whether it should or not. Running with assertions enabled should happen when running under surefire and what-not, but it's better to not have that implicit dependency. My apologies for missing that in the last review.

Okay. I was trying to figure that out after the last comment, but I couldn't quite see it. Thanks.

Not a problem. That's what I get for relying on IntelliJ. It's very good for importing the right package for me, but not in this case where there's already a corresponding native Java class.

• Fix based on latest feedback (setCounters and junit assert* methods)

-1 overall

This message was automatically generated.

• Put up correctly generated patch this time.

-1 overall

This message was automatically generated.

RE: checkstyle

The same four public declarations as before.

+1 for the latest patch. Committing this.

Thanks, Ray! I committed this to trunk and branch-2.

Excellent. Thanks for all the help!

FAILURE: Integrated in Hadoop-trunk-Commit #8252 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8252/)
MAPREDUCE-6394. Speed up Task processing loop in HsTasksBlock#render(). Contributed by Ray Chiang (jlowe: rev d0e0ba8010b72f58ddede5303f2b88404263d4bf)

• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTask.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskAttemptReport.java
• hadoop-mapreduce-project/CHANGES.txt
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskAttemptReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTaskAttempt.java

FAILURE: Integrated in Hadoop-Yarn-trunk #1004 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1004/)
MAPREDUCE-6394. Speed up Task processing loop in HsTasksBlock#render(). Contributed by Ray Chiang (jlowe: rev d0e0ba8010b72f58ddede5303f2b88404263d4bf)

• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTask.java
• hadoop-mapreduce-project/CHANGES.txt
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTaskAttempt.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskAttemptReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskReportPBImpl.java

SUCCESS: Integrated in Hadoop-Yarn-trunk-Java8 #274 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/274/)
MAPREDUCE-6394. Speed up Task processing loop in HsTasksBlock#render(). Contributed by Ray Chiang (jlowe: rev d0e0ba8010b72f58ddede5303f2b88404263d4bf)

• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTaskAttempt.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskAttemptReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskAttemptReport.java
• hadoop-mapreduce-project/CHANGES.txt
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTask.java

FAILURE: Integrated in Hadoop-Mapreduce-trunk #2220 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2220/)
MAPREDUCE-6394. Speed up Task processing loop in HsTasksBlock#render(). Contributed by Ray Chiang (jlowe: rev d0e0ba8010b72f58ddede5303f2b88404263d4bf)

• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTask.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskAttemptReport.java
• hadoop-mapreduce-project/CHANGES.txt
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTaskAttempt.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskAttemptReportPBImpl.java

FAILURE: Integrated in Hadoop-Hdfs-trunk #2201 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2201/)
MAPREDUCE-6394. Speed up Task processing loop in HsTasksBlock#render(). Contributed by Ray Chiang (jlowe: rev d0e0ba8010b72f58ddede5303f2b88404263d4bf)

• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTaskAttempt.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTask.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskAttemptReport.java
• hadoop-mapreduce-project/CHANGES.txt
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskAttemptReportPBImpl.java

FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #263 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/263/)
MAPREDUCE-6394. Speed up Task processing loop in HsTasksBlock#render(). Contributed by Ray Chiang (jlowe: rev d0e0ba8010b72f58ddede5303f2b88404263d4bf)

• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTaskAttempt.java
• hadoop-mapreduce-project/CHANGES.txt
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTask.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskAttemptReportPBImpl.java

SUCCESS: Integrated in Hadoop-Mapreduce-trunk-Java8 #271 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/271/)
MAPREDUCE-6394. Speed up Task processing loop in HsTasksBlock#render(). Contributed by Ray Chiang (jlowe: rev d0e0ba8010b72f58ddede5303f2b88404263d4bf)

• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTaskAttempt.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-hs/src/main/java/org/apache/hadoop/mapreduce/v2/hs/CompletedTask.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskAttemptReportPBImpl.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/impl/pb/TaskReportPBImpl.java
• hadoop-mapreduce-project/CHANGES.txt
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskAttemptReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-common/src/main/java/org/apache/hadoop/mapreduce/v2/api/records/TaskReport.java
• hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/api/records/TestTaskReport.java