Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.23.3, 2.0.0-alpha
    • Fix Version/s: 0.23.3, 2.0.2-alpha
    • Component/s: mrv2
    • Labels:
      None

      Description

      container logs (stdout, stderr, syslog) in the nodemanager ui and jobhistory ui appear in unsorted order where the order displayed is based on what file was created first. This jira will have the results be displayed in a consistent order.

      1. MAPREDUCE-4169.patch
        3 kB
        Jonathan Eagles
      2. MAPREDUCE-4169.patch
        3 kB
        Jonathan Eagles
      3. MAPREDUCE-4169.patch
        4 kB
        Jonathan Eagles

        Activity

        Hide
        Harsh J added a comment -

        Patch looks good to me (Did you verify the results manually too? Looks about right but I've not verified it manually yet). +1 on this - just one nit: Can you also add in a very short 'purpose' comment for each place you've done the sorting at? Having tests seems difficult for these particular classes so we can at least do with comments to help prevent removal/regress.

        Show
        Harsh J added a comment - Patch looks good to me (Did you verify the results manually too? Looks about right but I've not verified it manually yet). +1 on this - just one nit: Can you also add in a very short 'purpose' comment for each place you've done the sorting at? Having tests seems difficult for these particular classes so we can at least do with comments to help prevent removal/regress.
        Hide
        Jonathan Eagles added a comment -

        Thanks, Harsh. Canceling the patch to add comments. I did verify both container logs UI and log aggregation logs UI in the history server to verify logs appear in sorted order.

        Show
        Jonathan Eagles added a comment - Thanks, Harsh. Canceling the patch to add comments. I did verify both container logs UI and log aggregation logs UI in the history server to verify logs appear in sorted order.
        Hide
        Robert Joseph Evans added a comment -

        I also like the patch for the most part, but while reviewing it I saw one disturbing preexisting problem.

        The LogValue constructor is called from AppLogAggregatorImpl.uploadLogsFromContainer(), which gets its value from LocalDirsHandlerService.getLogDirs() which calls DirectoryCollection.getGoodDirs(), which returns an internal ArrayList. When we sort this we are sorting an internal data structre of DirectoryCollection, that appears to be something that can be called from multiple threads and that it too modifies internally, possibly on a separate thread.

        I think the correct thing to do is to modify DirectoryCollection to not return internal values, as we could get concurrent modification exceptions as it is. We either need to clone the Lists before returning them, or we need to make them concurrent in some way.

        I am fine if we do this on a different JIRA, because it is a preexisting problem. It is just that this JIRA will potentially exacerbate the problem.

        Show
        Robert Joseph Evans added a comment - I also like the patch for the most part, but while reviewing it I saw one disturbing preexisting problem. The LogValue constructor is called from AppLogAggregatorImpl.uploadLogsFromContainer(), which gets its value from LocalDirsHandlerService.getLogDirs() which calls DirectoryCollection.getGoodDirs(), which returns an internal ArrayList. When we sort this we are sorting an internal data structre of DirectoryCollection, that appears to be something that can be called from multiple threads and that it too modifies internally, possibly on a separate thread. I think the correct thing to do is to modify DirectoryCollection to not return internal values, as we could get concurrent modification exceptions as it is. We either need to clone the Lists before returning them, or we need to make them concurrent in some way. I am fine if we do this on a different JIRA, because it is a preexisting problem. It is just that this JIRA will potentially exacerbate the problem.
        Hide
        Jonathan Eagles added a comment -

        Good catch, Bobby. Canceling this patch to address to concurrency issue.

        Show
        Jonathan Eagles added a comment - Good catch, Bobby. Canceling this patch to address to concurrency issue.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12523797/MAPREDUCE-4169.patch
        against trunk revision .

        +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 javadoc. The javadoc tool did not generate any warning messages.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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:
        org.apache.hadoop.yarn.logaggregation.TestAggregatedLogFormat
        org.apache.hadoop.yarn.server.TestContainerManagerSecurity
        org.apache.hadoop.yarn.server.resourcemanager.security.TestApplicationTokens
        org.apache.hadoop.yarn.server.resourcemanager.TestClientRMService
        org.apache.hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry
        org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization
        org.apache.hadoop.yarn.server.resourcemanager.TestApplicationACLs
        org.apache.hadoop.mapred.TestClientRedirect
        org.apache.hadoop.mapreduce.security.TestJHSSecurity

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2295//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2295//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12523797/MAPREDUCE-4169.patch against trunk revision . +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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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: org.apache.hadoop.yarn.logaggregation.TestAggregatedLogFormat org.apache.hadoop.yarn.server.TestContainerManagerSecurity org.apache.hadoop.yarn.server.resourcemanager.security.TestApplicationTokens org.apache.hadoop.yarn.server.resourcemanager.TestClientRMService org.apache.hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization org.apache.hadoop.yarn.server.resourcemanager.TestApplicationACLs org.apache.hadoop.mapred.TestClientRedirect org.apache.hadoop.mapreduce.security.TestJHSSecurity +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2295//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2295//console This message is automatically generated.
        Hide
        Jonathan Eagles added a comment -

        Addressed Bobby's comments as part of MAPREDUCE-4194. Posting new patch.

        Show
        Jonathan Eagles added a comment - Addressed Bobby's comments as part of MAPREDUCE-4194 . Posting new patch.
        Hide
        Jonathan Eagles added a comment -

        Canceling to address test failures

        Show
        Jonathan Eagles added a comment - Canceling to address test failures
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12524357/MAPREDUCE-4169.patch
        against trunk revision .

        +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 javadoc. The javadoc tool did not generate any warning messages.

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

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

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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:
        org.apache.hadoop.yarn.server.TestContainerManagerSecurity
        org.apache.hadoop.yarn.server.resourcemanager.security.TestApplicationTokens
        org.apache.hadoop.yarn.server.resourcemanager.TestClientRMService
        org.apache.hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry
        org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization
        org.apache.hadoop.yarn.server.resourcemanager.TestApplicationACLs
        org.apache.hadoop.mapred.TestClientRedirect
        org.apache.hadoop.mapreduce.TestYarnClientProtocolProvider
        org.apache.hadoop.mapreduce.security.TestJHSSecurity

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2310//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2310//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12524357/MAPREDUCE-4169.patch against trunk revision . +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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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: org.apache.hadoop.yarn.server.TestContainerManagerSecurity org.apache.hadoop.yarn.server.resourcemanager.security.TestApplicationTokens org.apache.hadoop.yarn.server.resourcemanager.TestClientRMService org.apache.hadoop.yarn.server.resourcemanager.resourcetracker.TestNMExpiry org.apache.hadoop.yarn.server.resourcemanager.TestAMAuthorization org.apache.hadoop.yarn.server.resourcemanager.TestApplicationACLs org.apache.hadoop.mapred.TestClientRedirect org.apache.hadoop.mapreduce.TestYarnClientProtocolProvider org.apache.hadoop.mapreduce.security.TestJHSSecurity +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2310//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2310//console This message is automatically generated.
        Hide
        Jonathan Eagles added a comment -

        Apparently, I was too eager and looked at the old test results.

        Show
        Jonathan Eagles added a comment - Apparently, I was too eager and looked at the old test results.
        Hide
        Jonathan Eagles added a comment -

        Current test failures are preexisting.

        Show
        Jonathan Eagles added a comment - Current test failures are preexisting.
        Hide
        Robert Joseph Evans added a comment -

        Thanks John,

        I put this into trunk, branch-2, and branch-0.23

        Show
        Robert Joseph Evans added a comment - Thanks John, I put this into trunk, branch-2, and branch-0.23
        Hide
        Jonathan Eagles added a comment -

        Awesome, thanks so much for the thorough reviews Bobby and Harsh.

        Jon

        Show
        Jonathan Eagles added a comment - Awesome, thanks so much for the thorough reviews Bobby and Harsh. Jon
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #240 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/240/)
        svn merge -c 1331012. FIXES: MAPREDUCE-4169. Container Logs appear in unsorted order (Jonathan Eagles via bobby) (Revision 1331016)

        Result = SUCCESS
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1331016
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java
        • /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/ContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #240 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/240/ ) svn merge -c 1331012. FIXES: MAPREDUCE-4169 . Container Logs appear in unsorted order (Jonathan Eagles via bobby) (Revision 1331016) Result = SUCCESS bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1331016 Files : /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java /hadoop/common/branches/branch-0.23/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/ContainerLogsPage.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1027 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1027/)
        MAPREDUCE-4169. Container Logs appear in unsorted order (Jonathan Eagles via bobby) (Revision 1331012)

        Result = FAILURE
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1331012
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/ContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1027 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1027/ ) MAPREDUCE-4169 . Container Logs appear in unsorted order (Jonathan Eagles via bobby) (Revision 1331012) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1331012 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/ContainerLogsPage.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1062 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1062/)
        MAPREDUCE-4169. Container Logs appear in unsorted order (Jonathan Eagles via bobby) (Revision 1331012)

        Result = FAILURE
        bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1331012
        Files :

        • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java
        • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/ContainerLogsPage.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1062 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1062/ ) MAPREDUCE-4169 . Container Logs appear in unsorted order (Jonathan Eagles via bobby) (Revision 1331012) Result = FAILURE bobby : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1331012 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/AggregatedLogFormat.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/ContainerLogsPage.java

          People

          • Assignee:
            Jonathan Eagles
            Reporter:
            Jonathan Eagles
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development