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

Shuffle audit logger should log size of shuffle transfer

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-beta1, 2.8.3
    • Component/s: None
    • Labels:
      None

      Description

      The shuffle audit logger currently logs the job ID and reducer ID but nothing about the size of the requested transfer. It calculates this as part of the HTTP response headers, so it would be trivial to log the response size. This would be very valuable for debugging network traffic storms from the shuffle handler.

      1. MAPREDUCE-6958.001.patch
        2 kB
        Jason Lowe
      2. MAPREDUCE-6958.002.patch
        2 kB
        Jason Lowe
      3. MAPREDUCE-6958.003.patch
        2 kB
        Jason Lowe
      4. MAPREDUCE-6958-branch-2.002.patch
        2 kB
        Jason Lowe
      5. MAPREDUCE-6958-branch-2.8.002.patch
        6 kB
        Jason Lowe

        Activity

        Hide
        jlowe Jason Lowe added a comment -

        Thanks Eric for the review! I committed this to branch-2.8 as well.

        Show
        jlowe Jason Lowe added a comment - Thanks Eric for the review! I committed this to branch-2.8 as well.
        Hide
        eepayne Eric Payne added a comment -

        Thanks Jason Lowe
        The branch-2.8 patch LGTM.
        +1

        Show
        eepayne Eric Payne added a comment - Thanks Jason Lowe The branch-2.8 patch LGTM. +1
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
              Prechecks
        +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.
              branch-2.8 Compile Tests
        +1 mvninstall 9m 7s branch-2.8 passed
        +1 compile 0m 18s branch-2.8 passed
        +1 checkstyle 0m 16s branch-2.8 passed
        +1 mvnsite 0m 24s branch-2.8 passed
        +1 findbugs 0m 30s branch-2.8 passed
        +1 javadoc 0m 16s branch-2.8 passed
              Patch Compile Tests
        +1 mvninstall 0m 15s the patch passed
        +1 compile 0m 15s the patch passed
        +1 javac 0m 15s the patch passed
        -1 checkstyle 0m 11s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle: The patch generated 1 new + 58 unchanged - 2 fixed = 59 total (was 60)
        +1 mvnsite 0m 17s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 38s the patch passed
        +1 javadoc 0m 12s the patch passed
              Other Tests
        +1 unit 0m 21s hadoop-mapreduce-client-shuffle in the patch passed.
        +1 asflicense 0m 17s The patch does not generate ASF License warnings.
        14m 23s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:c2d96dd
        JIRA Issue MAPREDUCE-6958
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887902/MAPREDUCE-6958-branch-2.8.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 6ad070db71e2 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 branch-2.8 / b1e1f75
        Default Java 1.7.0_151
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7144/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-shuffle.txt
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7144/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7144/console
        Powered by Apache Yetus 0.5.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 17s Docker mode activated.       Prechecks +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.       branch-2.8 Compile Tests +1 mvninstall 9m 7s branch-2.8 passed +1 compile 0m 18s branch-2.8 passed +1 checkstyle 0m 16s branch-2.8 passed +1 mvnsite 0m 24s branch-2.8 passed +1 findbugs 0m 30s branch-2.8 passed +1 javadoc 0m 16s branch-2.8 passed       Patch Compile Tests +1 mvninstall 0m 15s the patch passed +1 compile 0m 15s the patch passed +1 javac 0m 15s the patch passed -1 checkstyle 0m 11s hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle: The patch generated 1 new + 58 unchanged - 2 fixed = 59 total (was 60) +1 mvnsite 0m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 38s the patch passed +1 javadoc 0m 12s the patch passed       Other Tests +1 unit 0m 21s hadoop-mapreduce-client-shuffle in the patch passed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 14m 23s Subsystem Report/Notes Docker Image:yetus/hadoop:c2d96dd JIRA Issue MAPREDUCE-6958 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887902/MAPREDUCE-6958-branch-2.8.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 6ad070db71e2 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 branch-2.8 / b1e1f75 Default Java 1.7.0_151 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7144/artifact/patchprocess/diff-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client_hadoop-mapreduce-client-shuffle.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7144/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7144/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12912 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12912/)
        MAPREDUCE-6958. Shuffle audit logger should log size of shuffle (jlowe: rev 3a20debddeac69596ceb5b36f8413529ea8570e6)

        • (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleHandler.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12912 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12912/ ) MAPREDUCE-6958 . Shuffle audit logger should log size of shuffle (jlowe: rev 3a20debddeac69596ceb5b36f8413529ea8570e6) (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleHandler.java
        Hide
        jlowe Jason Lowe added a comment -

        Attaching the patch for branch-2.8

        Show
        jlowe Jason Lowe added a comment - Attaching the patch for branch-2.8
        Hide
        jlowe Jason Lowe added a comment -

        I reverted the 003 patch from trunk and branch-3.0 and committed the compatible version to trunk, branch-3.0, and branch-2. The patch doesn't work for branch-2.8 since that branch is missing MAPREDUCE-6197. I'll post a branch-2.8 patch shortly.

        Show
        jlowe Jason Lowe added a comment - I reverted the 003 patch from trunk and branch-3.0 and committed the compatible version to trunk, branch-3.0, and branch-2. The patch doesn't work for branch-2.8 since that branch is missing MAPREDUCE-6197 . I'll post a branch-2.8 patch shortly.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12910 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12910/)
        Revert "MAPREDUCE-6958. Shuffle audit logger should log size of shuffle (jlowe: rev ea845ba58c585647c4be8d30d9b814f098e34a12)

        • (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleHandler.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #12910 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12910/ ) Revert " MAPREDUCE-6958 . Shuffle audit logger should log size of shuffle (jlowe: rev ea845ba58c585647c4be8d30d9b814f098e34a12) (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleHandler.java
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 16m 45s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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.
              branch-2 Compile Tests
        +1 mvninstall 6m 34s branch-2 passed
        +1 compile 0m 15s branch-2 passed
        +1 checkstyle 0m 11s branch-2 passed
        +1 mvnsite 0m 18s branch-2 passed
        +1 findbugs 0m 28s branch-2 passed
        +1 javadoc 0m 12s branch-2 passed
              Patch Compile Tests
        +1 mvninstall 0m 15s the patch passed
        +1 compile 0m 13s the patch passed
        +1 javac 0m 13s the patch passed
        +1 checkstyle 0m 9s the patch passed
        +1 mvnsite 0m 17s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 35s the patch passed
        +1 javadoc 0m 11s the patch passed
              Other Tests
        +1 unit 0m 17s hadoop-mapreduce-client-shuffle in the patch passed.
        +1 asflicense 0m 14s The patch does not generate ASF License warnings.
        27m 34s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:eaf5c66
        JIRA Issue MAPREDUCE-6958
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887770/MAPREDUCE-6958-branch-2.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 92227d8c29f4 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 branch-2 / 6aaca3d
        Default Java 1.7.0_151
        findbugs v3.0.0
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7143/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7143/console
        Powered by Apache Yetus 0.5.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 16m 45s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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.       branch-2 Compile Tests +1 mvninstall 6m 34s branch-2 passed +1 compile 0m 15s branch-2 passed +1 checkstyle 0m 11s branch-2 passed +1 mvnsite 0m 18s branch-2 passed +1 findbugs 0m 28s branch-2 passed +1 javadoc 0m 12s branch-2 passed       Patch Compile Tests +1 mvninstall 0m 15s the patch passed +1 compile 0m 13s the patch passed +1 javac 0m 13s the patch passed +1 checkstyle 0m 9s the patch passed +1 mvnsite 0m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 35s the patch passed +1 javadoc 0m 11s the patch passed       Other Tests +1 unit 0m 17s hadoop-mapreduce-client-shuffle in the patch passed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 27m 34s Subsystem Report/Notes Docker Image:yetus/hadoop:eaf5c66 JIRA Issue MAPREDUCE-6958 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887770/MAPREDUCE-6958-branch-2.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 92227d8c29f4 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 branch-2 / 6aaca3d Default Java 1.7.0_151 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7143/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7143/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        chris.douglas Chris Douglas added a comment -

        Since 3.0 hasn't officially shipped yet, I propose to revert the 003 patch I committed to trunk and branch-3.0 and instead commit patch version 002 which preserves the job-then-reducer ordering already established in the 2.x line. Objections?

        Nope, that sounds reasonable. Thanks for the extra audit

        Show
        chris.douglas Chris Douglas added a comment - Since 3.0 hasn't officially shipped yet, I propose to revert the 003 patch I committed to trunk and branch-3.0 and instead commit patch version 002 which preserves the job-then-reducer ordering already established in the 2.x line. Objections? Nope, that sounds reasonable. Thanks for the extra audit
        Hide
        jlowe Jason Lowe added a comment -

        Attaching a branch-2 version of the 002 patch.

        Show
        jlowe Jason Lowe added a comment - Attaching a branch-2 version of the 002 patch.
        Hide
        jlowe Jason Lowe added a comment -

        Hmm, so we have a bit of a quandry. I committed this through branch-3.0, but when I tried picking this to branch-2 and 2.8, but the patch doesn't apply because the mappers field was not there. I thought it had been there awhile, but it turns out it was added in MAPREDUCE-6808 in a backwards-incompatible way. The original format was job then reducer, and that JIRA made it job, maps, reducer.

        Since 3.0 hasn't officially shipped yet, I propose to revert the 003 patch I committed to trunk and branch-3.0 and instead commit patch version 002 which preserves the job-then-reducer ordering already established in the 2.x line. Objections?

        Show
        jlowe Jason Lowe added a comment - Hmm, so we have a bit of a quandry. I committed this through branch-3.0, but when I tried picking this to branch-2 and 2.8, but the patch doesn't apply because the mappers field was not there. I thought it had been there awhile, but it turns out it was added in MAPREDUCE-6808 in a backwards-incompatible way. The original format was job then reducer, and that JIRA made it job, maps, reducer. Since 3.0 hasn't officially shipped yet, I propose to revert the 003 patch I committed to trunk and branch-3.0 and instead commit patch version 002 which preserves the job-then-reducer ordering already established in the 2.x line. Objections?
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #12903 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12903/)
        MAPREDUCE-6958. Shuffle audit logger should log size of shuffle (jlowe: rev b3d61304f2fa4a99526f7a60ccaac9f262083079)

        • (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleHandler.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #12903 (See https://builds.apache.org/job/Hadoop-trunk-Commit/12903/ ) MAPREDUCE-6958 . Shuffle audit logger should log size of shuffle (jlowe: rev b3d61304f2fa4a99526f7a60ccaac9f262083079) (edit) hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle/src/main/java/org/apache/hadoop/mapred/ShuffleHandler.java
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for the review! Committing this.

        Show
        jlowe Jason Lowe added a comment - Thanks for the review! Committing this.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 19s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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.
              trunk Compile Tests
        +1 mvninstall 14m 10s trunk passed
        +1 compile 0m 18s trunk passed
        +1 checkstyle 0m 13s trunk passed
        +1 mvnsite 0m 20s trunk passed
        +1 findbugs 0m 25s trunk passed
        +1 javadoc 0m 14s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 16s the patch passed
        +1 compile 0m 15s the patch passed
        +1 javac 0m 15s the patch passed
        +1 checkstyle 0m 9s the patch passed
        +1 mvnsite 0m 17s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 29s the patch passed
        +1 javadoc 0m 11s the patch passed
              Other Tests
        +1 unit 0m 21s hadoop-mapreduce-client-shuffle in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        18m 53s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:71bbb86
        JIRA Issue MAPREDUCE-6958
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887718/MAPREDUCE-6958.003.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux a423f3a9b770 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 29dd551
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7141/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7141/console
        Powered by Apache Yetus 0.5.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 19s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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.       trunk Compile Tests +1 mvninstall 14m 10s trunk passed +1 compile 0m 18s trunk passed +1 checkstyle 0m 13s trunk passed +1 mvnsite 0m 20s trunk passed +1 findbugs 0m 25s trunk passed +1 javadoc 0m 14s trunk passed       Patch Compile Tests +1 mvninstall 0m 16s the patch passed +1 compile 0m 15s the patch passed +1 javac 0m 15s the patch passed +1 checkstyle 0m 9s the patch passed +1 mvnsite 0m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 29s the patch passed +1 javadoc 0m 11s the patch passed       Other Tests +1 unit 0m 21s hadoop-mapreduce-client-shuffle in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 18m 53s Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue MAPREDUCE-6958 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887718/MAPREDUCE-6958.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a423f3a9b770 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 29dd551 Default Java 1.8.0_144 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7141/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7141/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        chris.douglas Chris Douglas added a comment -

        Thanks for updating the patch, Jason Lowe. +1

        Show
        chris.douglas Chris Douglas added a comment - Thanks for updating the patch, Jason Lowe . +1
        Hide
        jlowe Jason Lowe added a comment -

        Updated the patch to reorder the log message for better backwards compatibility.

        Show
        jlowe Jason Lowe added a comment - Updated the patch to reorder the log message for better backwards compatibility.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for the review, Chris! I'll update the patch to preserve the existing format as best as I can.

        The shuffle sizes used to be available in the clienttrace log. Was that removed from the ShuffleHandler at some point?

        It does log the content length in the normal logger, but it's on a separate log line which isn't very friendly given the multithreaded nature of the netty processing and also not available in the audit log itself.

        Show
        jlowe Jason Lowe added a comment - Thanks for the review, Chris! I'll update the patch to preserve the existing format as best as I can. The shuffle sizes used to be available in the clienttrace log. Was that removed from the ShuffleHandler at some point? It does log the content length in the normal logger, but it's on a separate log line which isn't very friendly given the multithreaded nature of the netty processing and also not available in the audit log itself.
        Hide
        chris.douglas Chris Douglas added a comment -

        Sorry to ask for revs on this kind of patch, but this changes the format of the audit log in a way that might break downstream consumers. The mapIds are printed after the reducer in the revised version. Could this keep the format as-is, with the length appended?

        The shuffle sizes used to be available in the clienttrace log. Was that removed from the ShuffleHandler at some point?

        Show
        chris.douglas Chris Douglas added a comment - Sorry to ask for revs on this kind of patch, but this changes the format of the audit log in a way that might break downstream consumers. The mapIds are printed after the reducer in the revised version. Could this keep the format as-is, with the length appended? The shuffle sizes used to be available in the clienttrace log. Was that removed from the ShuffleHandler at some point?
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 28s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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.
              trunk Compile Tests
        +1 mvninstall 13m 35s trunk passed
        +1 compile 0m 18s trunk passed
        +1 checkstyle 0m 15s trunk passed
        +1 mvnsite 0m 21s trunk passed
        +1 findbugs 0m 25s trunk passed
        +1 javadoc 0m 14s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 16s the patch passed
        +1 compile 0m 15s the patch passed
        +1 javac 0m 15s the patch passed
        +1 checkstyle 0m 11s the patch passed
        +1 mvnsite 0m 16s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 28s the patch passed
        +1 javadoc 0m 10s the patch passed
              Other Tests
        +1 unit 0m 20s hadoop-mapreduce-client-shuffle in the patch passed.
        +1 asflicense 0m 14s The patch does not generate ASF License warnings.
        18m 28s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:71bbb86
        JIRA Issue MAPREDUCE-6958
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887420/MAPREDUCE-6958.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux cfd259bb0f16 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 / fbe06b5
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7140/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7140/console
        Powered by Apache Yetus 0.5.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.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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.       trunk Compile Tests +1 mvninstall 13m 35s trunk passed +1 compile 0m 18s trunk passed +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 21s trunk passed +1 findbugs 0m 25s trunk passed +1 javadoc 0m 14s trunk passed       Patch Compile Tests +1 mvninstall 0m 16s the patch passed +1 compile 0m 15s the patch passed +1 javac 0m 15s the patch passed +1 checkstyle 0m 11s the patch passed +1 mvnsite 0m 16s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 28s the patch passed +1 javadoc 0m 10s the patch passed       Other Tests +1 unit 0m 20s hadoop-mapreduce-client-shuffle in the patch passed. +1 asflicense 0m 14s The patch does not generate ASF License warnings. 18m 28s Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue MAPREDUCE-6958 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887420/MAPREDUCE-6958.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cfd259bb0f16 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 / fbe06b5 Default Java 1.8.0_144 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7140/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7140/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        shahrs87 Rushabh S Shah added a comment -

        +1 ltgm non-binding.

        Show
        shahrs87 Rushabh S Shah added a comment - +1 ltgm non-binding.
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for the reviews! I updated the patch to print the ArrayList directly.

        Show
        jlowe Jason Lowe added a comment - Thanks for the reviews! I updated the patch to print the ArrayList directly.
        Hide
        shahrs87 Rushabh S Shah added a comment - - edited

        Overall the patch looks good.
        Just one very minor nit.

                for (String mapId : mapIds) {
                   sb.append(" ");
                  sb.append(mapId);
                }
        

        Instead of going through mapId, we can just print mapIds. It will be just less code.

        Show
        shahrs87 Rushabh S Shah added a comment - - edited Overall the patch looks good. Just one very minor nit. for (String mapId : mapIds) { sb.append(" "); sb.append(mapId); } Instead of going through mapId , we can just print mapIds . It will be just less code.
        Hide
        kshukla Kuhu Shukla added a comment -

        Thank you Jason Lowe for the patch!
        +1 (non-binding) LGTM.

        Show
        kshukla Kuhu Shukla added a comment - Thank you Jason Lowe for the patch! +1 (non-binding) LGTM.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 27m 51s Docker mode activated.
              Prechecks
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s 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.
              trunk Compile Tests
        +1 mvninstall 19m 51s trunk passed
        +1 compile 0m 38s trunk passed
        +1 checkstyle 0m 19s trunk passed
        +1 mvnsite 0m 28s trunk passed
        +1 findbugs 0m 37s trunk passed
        +1 javadoc 0m 19s trunk passed
              Patch Compile Tests
        +1 mvninstall 0m 21s the patch passed
        +1 compile 0m 23s the patch passed
        +1 javac 0m 23s the patch passed
        +1 checkstyle 0m 13s the patch passed
        +1 mvnsite 0m 21s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 39s the patch passed
        +1 javadoc 0m 14s the patch passed
              Other Tests
        +1 unit 0m 27s hadoop-mapreduce-client-shuffle in the patch passed.
        +1 asflicense 0m 42s The patch does not generate ASF License warnings.
        54m 16s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:71bbb86
        JIRA Issue MAPREDUCE-6958
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887206/MAPREDUCE-6958.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux fddfdc03bc32 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 390c2b5
        Default Java 1.8.0_144
        findbugs v3.1.0-RC1
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7135/testReport/
        modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7135/console
        Powered by Apache Yetus 0.5.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 27m 51s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s 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.       trunk Compile Tests +1 mvninstall 19m 51s trunk passed +1 compile 0m 38s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 28s trunk passed +1 findbugs 0m 37s trunk passed +1 javadoc 0m 19s trunk passed       Patch Compile Tests +1 mvninstall 0m 21s the patch passed +1 compile 0m 23s the patch passed +1 javac 0m 23s the patch passed +1 checkstyle 0m 13s the patch passed +1 mvnsite 0m 21s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 39s the patch passed +1 javadoc 0m 14s the patch passed       Other Tests +1 unit 0m 27s hadoop-mapreduce-client-shuffle in the patch passed. +1 asflicense 0m 42s The patch does not generate ASF License warnings. 54m 16s Subsystem Report/Notes Docker Image:yetus/hadoop:71bbb86 JIRA Issue MAPREDUCE-6958 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12887206/MAPREDUCE-6958.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fddfdc03bc32 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 390c2b5 Default Java 1.8.0_144 findbugs v3.1.0-RC1 Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7135/testReport/ modules C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle U: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/7135/console Powered by Apache Yetus 0.5.0 http://yetus.apache.org This message was automatically generated.
        Hide
        jlowe Jason Lowe added a comment -

        Patch that adds the content length to the audit log.

        Show
        jlowe Jason Lowe added a comment - Patch that adds the content length to the audit log.

          People

          • Assignee:
            jlowe Jason Lowe
            Reporter:
            jlowe Jason Lowe
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development