Details

    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      Enhance locks in SchedulerApplicationAttempt/FSAppAttempt/FiCaSchedulerApp, as mentioned in YARN-3091, a possible solution is using read/write lock. Other fine-graind locks for specific purposes / bugs should be addressed in separated tickets.

      1. YARN-3141.1.patch
        88 kB
        Wangda Tan
      2. YARN-3141.2.patch
        89 kB
        Wangda Tan
      3. YARN-3141.3.patch
        88 kB
        Wangda Tan
      4. YARN-3141.4.patch
        88 kB
        Wangda Tan
      5. YARN-3141.5.patch
        88 kB
        Wangda Tan
      6. YARN-3141.6.patch
        88 kB
        Wangda Tan
      7. YARN-3141.addendum-0.patch
        1 kB
        Wangda Tan

        Activity

        Hide
        yanghaogn Yang Hao added a comment -

        Can you give a description

        Show
        yanghaogn Yang Hao added a comment - Can you give a description
        Hide
        leftnoteasy Wangda Tan added a comment -

        Just updated description, thanks.

        Show
        leftnoteasy Wangda Tan added a comment - Just updated description, thanks.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Attached ver.1 patch for review.

        Show
        leftnoteasy Wangda Tan added a comment - Attached ver.1 patch for review.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 10s Docker mode activated.
        +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.
        +1 mvninstall 6m 36s trunk passed
        +1 compile 0m 31s trunk passed
        +1 checkstyle 0m 21s trunk passed
        +1 mvnsite 0m 37s trunk passed
        +1 mvneclipse 0m 16s trunk passed
        +1 findbugs 0m 54s trunk passed
        +1 javadoc 0m 20s trunk passed
        +1 mvninstall 0m 30s the patch passed
        +1 compile 0m 29s the patch passed
        +1 javac 0m 29s the patch passed
        -1 checkstyle 0m 18s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 21 new + 49 unchanged - 23 fixed = 70 total (was 72)
        +1 mvnsite 0m 33s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 59s the patch passed
        +1 javadoc 0m 18s the patch passed
        -1 unit 586m 35s hadoop-yarn-server-resourcemanager in the patch failed.
        -1 asflicense 0m 16s The patch generated 1 ASF License warnings.
        600m 36s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMService
          hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerLazyPreemption
          hadoop.yarn.server.resourcemanager.TestAMAuthorization
          hadoop.yarn.server.resourcemanager.rmapp.TestNodesListManager
          hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer
          hadoop.yarn.server.resourcemanager.TestContainerResourceUsage
          hadoop.yarn.server.resourcemanager.security.TestClientToAMTokens
          hadoop.yarn.server.resourcemanager.TestNodeBlacklistingOnAMFailures
          hadoop.yarn.server.resourcemanager.TestApplicationACLs
          hadoop.yarn.server.resourcemanager.TestRM
          hadoop.yarn.server.resourcemanager.scheduler.fifo.TestFifoScheduler
          hadoop.yarn.server.resourcemanager.security.TestAMRMTokens
          hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerSurgicalPreemption
          hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart
          hadoop.yarn.server.resourcemanager.TestClientRMTokens
          hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerQueueACLs
          hadoop.yarn.server.resourcemanager.TestRMAdminService
        Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestApplicationCleanup
          org.apache.hadoop.yarn.server.resourcemanager.ahs.TestRMApplicationHistoryWriter
          org.apache.hadoop.yarn.server.resourcemanager.TestSignalContainer
          org.apache.hadoop.yarn.server.resourcemanager.rmcontainer.TestRMContainerImpl
          org.apache.hadoop.yarn.server.resourcemanager.TestRMRestart
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationPriority
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.TestAbstractYarnScheduler
          org.apache.hadoop.yarn.server.resourcemanager.TestRMHA
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestNodeLabelContainerAllocation
          org.apache.hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRMRPCResponseId
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerResizing
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerDynamicBehavior
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.TestSchedulingWithAllocationRequestId
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationLimitsByPartition
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.TestSchedulerUtils
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation
          org.apache.hadoop.yarn.server.resourcemanager.TestApplicationMasterService
          org.apache.hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesAppsModification
          org.apache.hadoop.yarn.server.resourcemanager.TestResourceTrackerService
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestLeafQueue
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestWorkPreservingRMRestartForNodeLabel
          org.apache.hadoop.yarn.server.resourcemanager.TestApplicationMasterLauncher
          org.apache.hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesSchedulerActivities
          org.apache.hadoop.yarn.server.resourcemanager.TestDecommissioningNodesWatcher
          org.apache.hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesApps
          org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA
          org.apache.hadoop.yarn.server.resourcemanager.TestMoveApplication
          org.apache.hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRMRPCNodeUpdates
          org.apache.hadoop.yarn.server.resourcemanager.TestKillApplicationWithRMHA
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestIncreaseAllocationExpirer
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.policy.TestFairOrderingPolicy
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerQueueACLs
          org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerNodeLabelUpdate
          org.apache.hadoop.yarn.server.resourcemanager.TestWorkPreservingRMRestart



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827855/YARN-3141.1.patch
        JIRA Issue YARN-3141
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux ae21a711a9f5 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / bee9f57
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13066/testReport/
        asflicense https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/patch-asflicense-problems.txt
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13066/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated. +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. +1 mvninstall 6m 36s trunk passed +1 compile 0m 31s trunk passed +1 checkstyle 0m 21s trunk passed +1 mvnsite 0m 37s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 0m 54s trunk passed +1 javadoc 0m 20s trunk passed +1 mvninstall 0m 30s the patch passed +1 compile 0m 29s the patch passed +1 javac 0m 29s the patch passed -1 checkstyle 0m 18s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 21 new + 49 unchanged - 23 fixed = 70 total (was 72) +1 mvnsite 0m 33s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 59s the patch passed +1 javadoc 0m 18s the patch passed -1 unit 586m 35s hadoop-yarn-server-resourcemanager in the patch failed. -1 asflicense 0m 16s The patch generated 1 ASF License warnings. 600m 36s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestClientRMService   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerLazyPreemption   hadoop.yarn.server.resourcemanager.TestAMAuthorization   hadoop.yarn.server.resourcemanager.rmapp.TestNodesListManager   hadoop.yarn.server.resourcemanager.security.TestDelegationTokenRenewer   hadoop.yarn.server.resourcemanager.TestContainerResourceUsage   hadoop.yarn.server.resourcemanager.security.TestClientToAMTokens   hadoop.yarn.server.resourcemanager.TestNodeBlacklistingOnAMFailures   hadoop.yarn.server.resourcemanager.TestApplicationACLs   hadoop.yarn.server.resourcemanager.TestRM   hadoop.yarn.server.resourcemanager.scheduler.fifo.TestFifoScheduler   hadoop.yarn.server.resourcemanager.security.TestAMRMTokens   hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerSurgicalPreemption   hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart   hadoop.yarn.server.resourcemanager.TestClientRMTokens   hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerQueueACLs   hadoop.yarn.server.resourcemanager.TestRMAdminService Timed out junit tests org.apache.hadoop.yarn.server.resourcemanager.TestApplicationCleanup   org.apache.hadoop.yarn.server.resourcemanager.ahs.TestRMApplicationHistoryWriter   org.apache.hadoop.yarn.server.resourcemanager.TestSignalContainer   org.apache.hadoop.yarn.server.resourcemanager.rmcontainer.TestRMContainerImpl   org.apache.hadoop.yarn.server.resourcemanager.TestRMRestart   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacityScheduler   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationPriority   org.apache.hadoop.yarn.server.resourcemanager.scheduler.TestAbstractYarnScheduler   org.apache.hadoop.yarn.server.resourcemanager.TestRMHA   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestNodeLabelContainerAllocation   org.apache.hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRMRPCResponseId   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerResizing   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerDynamicBehavior   org.apache.hadoop.yarn.server.resourcemanager.scheduler.TestSchedulingWithAllocationRequestId   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestApplicationLimitsByPartition   org.apache.hadoop.yarn.server.resourcemanager.scheduler.TestSchedulerUtils   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestContainerAllocation   org.apache.hadoop.yarn.server.resourcemanager.TestApplicationMasterService   org.apache.hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesAppsModification   org.apache.hadoop.yarn.server.resourcemanager.TestResourceTrackerService   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestLeafQueue   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestWorkPreservingRMRestartForNodeLabel   org.apache.hadoop.yarn.server.resourcemanager.TestApplicationMasterLauncher   org.apache.hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesSchedulerActivities   org.apache.hadoop.yarn.server.resourcemanager.TestDecommissioningNodesWatcher   org.apache.hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesApps   org.apache.hadoop.yarn.server.resourcemanager.TestSubmitApplicationWithRMHA   org.apache.hadoop.yarn.server.resourcemanager.TestMoveApplication   org.apache.hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRMRPCNodeUpdates   org.apache.hadoop.yarn.server.resourcemanager.TestKillApplicationWithRMHA   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestIncreaseAllocationExpirer   org.apache.hadoop.yarn.server.resourcemanager.scheduler.policy.TestFairOrderingPolicy   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerQueueACLs   org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerNodeLabelUpdate   org.apache.hadoop.yarn.server.resourcemanager.TestWorkPreservingRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827855/YARN-3141.1.patch JIRA Issue YARN-3141 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ae21a711a9f5 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / bee9f57 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13066/testReport/ asflicense https://builds.apache.org/job/PreCommit-YARN-Build/13066/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13066/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Uploaded ver.2, fixed a locking issue and some important checkstyle warnings.

        Show
        leftnoteasy Wangda Tan added a comment - Uploaded ver.2, fixed a locking issue and some important checkstyle warnings.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 18s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        -1 test4tests 0m 0s The patch 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 mvninstall 9m 4s trunk passed
        +1 compile 0m 44s trunk passed
        +1 checkstyle 0m 30s trunk passed
        +1 mvnsite 0m 51s trunk passed
        +1 mvneclipse 0m 21s trunk passed
        +1 findbugs 1m 15s trunk passed
        +1 javadoc 0m 30s trunk passed
        +1 mvninstall 0m 42s the patch passed
        +1 compile 0m 41s the patch passed
        +1 javac 0m 41s the patch passed
        -1 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 16 new + 48 unchanged - 23 fixed = 64 total (was 71)
        +1 mvnsite 0m 46s the patch passed
        +1 mvneclipse 0m 18s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 20s the patch passed
        +1 javadoc 0m 25s the patch passed
        -1 unit 45m 0s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 19s The patch does not generate ASF License warnings.
        64m 18s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.TestRMHA



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827898/YARN-3141.2.patch
        JIRA Issue YARN-3141
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux a02c35f94ce8 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / bee9f57
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13072/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13072/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13072/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13072/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13072/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch 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 mvninstall 9m 4s trunk passed +1 compile 0m 44s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 0m 51s trunk passed +1 mvneclipse 0m 21s trunk passed +1 findbugs 1m 15s trunk passed +1 javadoc 0m 30s trunk passed +1 mvninstall 0m 42s the patch passed +1 compile 0m 41s the patch passed +1 javac 0m 41s the patch passed -1 checkstyle 0m 24s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 16 new + 48 unchanged - 23 fixed = 64 total (was 71) +1 mvnsite 0m 46s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 20s the patch passed +1 javadoc 0m 25s the patch passed -1 unit 45m 0s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 64m 18s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMHA Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827898/YARN-3141.2.patch JIRA Issue YARN-3141 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a02c35f94ce8 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / bee9f57 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13072/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13072/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13072/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13072/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13072/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Jian He, Karthik Kambatla could you take a look at this patch?

        Show
        leftnoteasy Wangda Tan added a comment - Jian He , Karthik Kambatla could you take a look at this patch?
        Hide
        kasha Karthik Kambatla added a comment -

        I ll need at least until later this week. Daniel Templeton - are you able to take a quick look meanwhile?

        Show
        kasha Karthik Kambatla added a comment - I ll need at least until later this week. Daniel Templeton - are you able to take a quick look meanwhile?
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks Karthik Kambatla / Daniel Templeton in advance! Meanwhile, I'm running large scale perf test using SLS to make sure there's no performance regression / deadlock happens. Will keep this thread posted.

        Show
        leftnoteasy Wangda Tan added a comment - Thanks Karthik Kambatla / Daniel Templeton in advance! Meanwhile, I'm running large scale perf test using SLS to make sure there's no performance regression / deadlock happens. Will keep this thread posted.
        Show
        leftnoteasy Wangda Tan added a comment - Please refer to https://issues.apache.org/jira/browse/YARN-3139?focusedCommentId=15491678&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15491678 for performance test report.
        Hide
        templedf Daniel Templeton added a comment -

        Thanks for the patch, Wangda Tan. I only did a superficial review. I still need to look carefully at the locking and data access to make sure it's clean.

        Comments:

        • You axed the javadoc for SchedulerApplicationAttempt.isReserved()
        • In SchedulerApplicationAttempt.showRequests(), the lock can be inside the test if debug in enabled
        • In FSAppAttempt. unreserveInternal() and FSAppAttempt. allocate() you could downgrade the lock before the logging at the end.
        • In FSAppAttempt. resetAllowedLocalityLevel(), it would be better to do:
              NodeType old;
          
              try {
                writeLock.lock();
                old = allowedLocalityLevel.put(schedulerKey, level);
              } finally {
                writeLock.unlock();
              }
          
              LOG.info("Raising locality level from " + old + " to " + level + " at "
                  + " priority " + schedulerKey.getPriority());
          

          It doesn't look like the schedulerKey.getPriority() needs protection.

        • In FSAppAttempt line 804 (I think) you deleted a space before a brace in the else
        • It would be nice in the javadoc for all the methods that are no longer synchronized to note that they're MT safe. That's a convention that exists nowhere else in YARN, but it should...
        Show
        templedf Daniel Templeton added a comment - Thanks for the patch, Wangda Tan . I only did a superficial review. I still need to look carefully at the locking and data access to make sure it's clean. Comments: You axed the javadoc for SchedulerApplicationAttempt.isReserved() In SchedulerApplicationAttempt.showRequests() , the lock can be inside the test if debug in enabled In FSAppAttempt. unreserveInternal() and FSAppAttempt. allocate() you could downgrade the lock before the logging at the end. In FSAppAttempt. resetAllowedLocalityLevel() , it would be better to do: NodeType old; try { writeLock.lock(); old = allowedLocalityLevel.put(schedulerKey, level); } finally { writeLock.unlock(); } LOG.info( "Raising locality level from " + old + " to " + level + " at " + " priority " + schedulerKey.getPriority()); It doesn't look like the schedulerKey.getPriority() needs protection. In FSAppAttempt line 804 (I think) you deleted a space before a brace in the else It would be nice in the javadoc for all the methods that are no longer synchronized to note that they're MT safe. That's a convention that exists nowhere else in YARN, but it should...
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks for review, Daniel Templeton,
        I addressed all your suggestions except:

        You axed the javadoc for SchedulerApplicationAttempt.isReserved()

        isReserved is not used by anyone, so I removed that method

        It would be nice in the javadoc for all the methods that are no longer synchronized to note that they're MT safe.

        This is a good suggestion, but I think it's better to come in a separate patch, since we have to update almost every method in scheduler.

        Any other thoughts?

        (Attached ver.3 patch)

        Show
        leftnoteasy Wangda Tan added a comment - Thanks for review, Daniel Templeton , I addressed all your suggestions except: You axed the javadoc for SchedulerApplicationAttempt.isReserved() isReserved is not used by anyone, so I removed that method It would be nice in the javadoc for all the methods that are no longer synchronized to note that they're MT safe. This is a good suggestion, but I think it's better to come in a separate patch, since we have to update almost every method in scheduler. Any other thoughts? (Attached ver.3 patch)
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +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.
        +1 mvninstall 8m 29s trunk passed
        +1 compile 0m 41s trunk passed
        +1 checkstyle 0m 23s trunk passed
        +1 mvnsite 0m 42s trunk passed
        +1 mvneclipse 0m 19s trunk passed
        +1 findbugs 1m 4s trunk passed
        +1 javadoc 0m 25s trunk passed
        +1 mvninstall 0m 33s the patch passed
        +1 compile 0m 31s the patch passed
        +1 javac 0m 31s the patch passed
        -1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 16 new + 49 unchanged - 23 fixed = 65 total (was 72)
        +1 mvnsite 0m 42s the patch passed
        +1 mvneclipse 0m 16s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 10s the patch passed
        +1 javadoc 0m 23s the patch passed
        +1 unit 33m 45s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        51m 1s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828699/YARN-3141.3.patch
        JIRA Issue YARN-3141
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux af9636090cb8 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / fcbac00
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13117/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13117/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13117/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +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. +1 mvninstall 8m 29s trunk passed +1 compile 0m 41s trunk passed +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 42s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 1m 4s trunk passed +1 javadoc 0m 25s trunk passed +1 mvninstall 0m 33s the patch passed +1 compile 0m 31s the patch passed +1 javac 0m 31s the patch passed -1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 16 new + 49 unchanged - 23 fixed = 65 total (was 72) +1 mvnsite 0m 42s the patch passed +1 mvneclipse 0m 16s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 10s the patch passed +1 javadoc 0m 23s the patch passed +1 unit 33m 45s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 51m 1s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828699/YARN-3141.3.patch JIRA Issue YARN-3141 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux af9636090cb8 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fcbac00 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13117/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13117/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13117/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        Continuing with more comments on v2. Sorry, I started these before you uploaded v3. These comments are a little more speculative. I'm not 100% certain that everything I'm recommending is safe.

        • SchedulerApplicationAttempt.getLiveContainersMap() should be default visibility and @VisibleForTesting
        • SchedulerApplicationAttempt.addRMContainer(), SchedulerApplicationAttempt.removeRMContainer(), SchedulerApplicationAttempt.updateResourceRequests(), SchedulerApplicationAttempt.recoverResourceRequestsForContainer(), SchedulerApplicationAttempt.reserve(), and SchedulerApplicationAttempt.updateBlacklist() should have the write locks pushed down to inside the if
        • SchedulerApplicationAttempt.getHeadroom() and SchedulerApplicationAttempt.getResourceLimit() are identical. SchedulerApplicationAttempt.getResourceLimit() is not used outside SchedulerApplicationAttempt
        • In SchedulerApplicationAttempt.resetSchedulingOpportunities(), is the write lock needed?
        • In SchedulerApplicationAttempt.getLiveContainers() is the read lock needed?
        • In SchedulerApplicationAttempt.stop() the isStopped update can happen before the lock
        • In SchedulerApplicationAttempt.getReservedContainers() the lock should only cover the for loop
        • In FSAppAttempt.getAllowedLocalityLevel(), FSAppAttempt.getAllowedLocalityLevelByTime(), FSAppAttempt.setReservation(), and FSAppAttempt.clearReservation() the write lock acquisition can be delayed until after the arg validation
        • There's an unused import in FaCiSchedulerApp
        • In FaCiSchedulerApp.containerCompleted() the write lock acquisition can be delayed until after removing from liveContainers
        • In FaCiSchedulerApp.allocate() the write lock acquisition can be delayed until after the stop check, and maybe after the sanity check
        • In FaCiSchedulerApp.unreserve() the write lock acquisition can be delayed until after canceling the increase request
        • In FaCiSchedulerApp.markContainerForPreemption() the write lock acquisition can be push down inside the if
        Show
        templedf Daniel Templeton added a comment - Continuing with more comments on v2. Sorry, I started these before you uploaded v3. These comments are a little more speculative. I'm not 100% certain that everything I'm recommending is safe. SchedulerApplicationAttempt.getLiveContainersMap() should be default visibility and @VisibleForTesting SchedulerApplicationAttempt.addRMContainer() , SchedulerApplicationAttempt.removeRMContainer() , SchedulerApplicationAttempt.updateResourceRequests() , SchedulerApplicationAttempt.recoverResourceRequestsForContainer() , SchedulerApplicationAttempt.reserve() , and SchedulerApplicationAttempt.updateBlacklist() should have the write locks pushed down to inside the if SchedulerApplicationAttempt.getHeadroom() and SchedulerApplicationAttempt.getResourceLimit() are identical. SchedulerApplicationAttempt.getResourceLimit() is not used outside SchedulerApplicationAttempt In SchedulerApplicationAttempt.resetSchedulingOpportunities() , is the write lock needed? In SchedulerApplicationAttempt.getLiveContainers() is the read lock needed? In SchedulerApplicationAttempt.stop() the isStopped update can happen before the lock In SchedulerApplicationAttempt.getReservedContainers() the lock should only cover the for loop In FSAppAttempt.getAllowedLocalityLevel() , FSAppAttempt.getAllowedLocalityLevelByTime() , FSAppAttempt.setReservation() , and FSAppAttempt.clearReservation() the write lock acquisition can be delayed until after the arg validation There's an unused import in FaCiSchedulerApp In FaCiSchedulerApp.containerCompleted() the write lock acquisition can be delayed until after removing from liveContainers In FaCiSchedulerApp.allocate() the write lock acquisition can be delayed until after the stop check, and maybe after the sanity check In FaCiSchedulerApp.unreserve() the write lock acquisition can be delayed until after canceling the increase request In FaCiSchedulerApp.markContainerForPreemption() the write lock acquisition can be push down inside the if
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks Daniel Templeton,

        However I think for most of the comments, we should keep them as-is, a volatile varible doesn't mean we don't need extra lock to protect consistency between variables.
        For a simplest example,

        volatile boolean a;
        volatile int b;
        
        void update_b(b') {
        	if (a) {
        		b = b'
        	}
        }
        
        void update_a(a') {
        	a = a'
        }
        
        boolean get_a() {
        	return a; 
        }
        
        boolean get_b() {
        	return b;
        }
        

        If two separate thread, thread #1 calls update_b first, and thread #2 calls update_a, it is possible that update_a returns before update_b returns. And if we read the two variables, data inconsistency happens.
        So I would rather be more conservative: if a method read some fields and write some fields, the safest way is add a single write lock to protect all them. Same to a method read multiple fields, we should have a single readlock for them.

        Most of the comments fall into the category, we could not shorten the critical sections of them.

        What I have addressed in this patch:

        SchedulerApplicationAttempt.getLiveContainersMap() should be default visibility and @VisibleForTesting

        In FSAppAttempt.getAllowedLocalityLevel(), FSAppAttempt.getAllowedLocalityLevelByTime(), FSAppAttempt.setReservation(), and FSAppAttempt.clearReservation() the write lock acquisition can be delayed until after the arg validation

        There's an unused import in FaCiSchedulerApp

        Show
        leftnoteasy Wangda Tan added a comment - Thanks Daniel Templeton , However I think for most of the comments, we should keep them as-is, a volatile varible doesn't mean we don't need extra lock to protect consistency between variables . For a simplest example, volatile boolean a; volatile int b; void update_b(b') { if (a) { b = b' } } void update_a(a') { a = a' } boolean get_a() { return a; } boolean get_b() { return b; } If two separate thread, thread #1 calls update_b first, and thread #2 calls update_a, it is possible that update_a returns before update_b returns. And if we read the two variables, data inconsistency happens. So I would rather be more conservative: if a method read some fields and write some fields, the safest way is add a single write lock to protect all them. Same to a method read multiple fields, we should have a single readlock for them. Most of the comments fall into the category, we could not shorten the critical sections of them. What I have addressed in this patch: SchedulerApplicationAttempt.getLiveContainersMap() should be default visibility and @VisibleForTesting In FSAppAttempt.getAllowedLocalityLevel(), FSAppAttempt.getAllowedLocalityLevelByTime(), FSAppAttempt.setReservation(), and FSAppAttempt.clearReservation() the write lock acquisition can be delayed until after the arg validation There's an unused import in FaCiSchedulerApp
        Hide
        leftnoteasy Wangda Tan added a comment -

        Attached ver.4 patch.

        Show
        leftnoteasy Wangda Tan added a comment - Attached ver.4 patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 19s Docker mode activated.
        +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.
        +1 mvninstall 7m 57s trunk passed
        +1 compile 0m 33s trunk passed
        +1 checkstyle 0m 21s trunk passed
        +1 mvnsite 0m 40s trunk passed
        +1 mvneclipse 0m 16s trunk passed
        +1 findbugs 1m 0s trunk passed
        +1 javadoc 0m 21s trunk passed
        +1 mvninstall 0m 36s the patch passed
        +1 compile 0m 30s the patch passed
        +1 javac 0m 30s the patch passed
        -1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 15 new + 48 unchanged - 23 fixed = 63 total (was 71)
        +1 mvnsite 0m 37s the patch passed
        +1 mvneclipse 0m 14s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 5s the patch passed
        +1 javadoc 0m 19s the patch passed
        -1 unit 37m 55s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        54m 1s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828731/YARN-3141.4.patch
        JIRA Issue YARN-3141
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 4b1c39304449 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / fcbac00
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13118/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13118/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13118/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13118/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13118/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 19s Docker mode activated. +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. +1 mvninstall 7m 57s trunk passed +1 compile 0m 33s trunk passed +1 checkstyle 0m 21s trunk passed +1 mvnsite 0m 40s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 1m 0s trunk passed +1 javadoc 0m 21s trunk passed +1 mvninstall 0m 36s the patch passed +1 compile 0m 30s the patch passed +1 javac 0m 30s the patch passed -1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 15 new + 48 unchanged - 23 fixed = 63 total (was 71) +1 mvnsite 0m 37s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 5s the patch passed +1 javadoc 0m 19s the patch passed -1 unit 37m 55s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 54m 1s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.TestRMRestart Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828731/YARN-3141.4.patch JIRA Issue YARN-3141 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4b1c39304449 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / fcbac00 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13118/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13118/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13118/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13118/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13118/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        I know that volatile doesn't make things magically thread safe. In the cases that I pointed out, it didn't look to me like the whole of the critical section needed to be atomic. But, it's fair to punt optimizing the critical sections to another JIRA or three.

        I'll review the latest patch when I get a chance.

        Show
        templedf Daniel Templeton added a comment - I know that volatile doesn't make things magically thread safe. In the cases that I pointed out, it didn't look to me like the whole of the critical section needed to be atomic. But, it's fair to punt optimizing the critical sections to another JIRA or three. I'll review the latest patch when I get a chance.
        Hide
        templedf Daniel Templeton added a comment -

        OK, I got a chance.

        Looks like in FSAppAttempt.updateInternal() you dropped the lock instead of downgrading it before the log. I'm not sure that's safe since appSchedulingInfo and reservedContainers aren't thread safe.

        In FSAppAttempt.resetAllowedLocalityLevel(), this:

              old = allowedLocalityLevel.get(schedulerKey);
              allowedLocalityLevel.put(schedulerKey, level);
        

        can be this:

              old = allowedLocalityLevel.put(schedulerKey, level);
        
        Show
        templedf Daniel Templeton added a comment - OK, I got a chance. Looks like in FSAppAttempt.updateInternal() you dropped the lock instead of downgrading it before the log. I'm not sure that's safe since appSchedulingInfo and reservedContainers aren't thread safe. In FSAppAttempt.resetAllowedLocalityLevel() , this: old = allowedLocalityLevel.get(schedulerKey); allowedLocalityLevel.put(schedulerKey, level); can be this: old = allowedLocalityLevel.put(schedulerKey, level);
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks Daniel Templeton,

        it didn't look to me like the whole of the critical section needed to be atomic. But, it's fair to punt optimizing the critical sections to another JIRA or three.

        Agree

        Looks like in FSAppAttempt.updateInternal()

        Sorry but I cannot find the FSAppAttempt.updateInternal(), did you give me a wrong name?

        Rest comments looks fine, will address together in the next patch

        Show
        leftnoteasy Wangda Tan added a comment - Thanks Daniel Templeton , it didn't look to me like the whole of the critical section needed to be atomic. But, it's fair to punt optimizing the critical sections to another JIRA or three. Agree Looks like in FSAppAttempt.updateInternal() Sorry but I cannot find the FSAppAttempt.updateInternal() , did you give me a wrong name? Rest comments looks fine, will address together in the next patch
        Hide
        templedf Daniel Templeton added a comment -

        Oops. Meant FSAppAttempt.unreserveInternal().

        Show
        templedf Daniel Templeton added a comment - Oops. Meant FSAppAttempt.unreserveInternal() .
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks for confirmation,

        Updated ver.5 patch addressed your comments.

        you dropped the lock instead of downgrading it before the log.

        In this patch, I didn't do improvements like downgrading locks in a method. Beyond FSAttempt.allocate/unreserveInternal, there're several similar methods in CS as well. I would prefer to have a separate JIRA to do more detailed improvements. And I don't know how much performance benefit we can gain from downgrading the lock. But it does make code becomes more complexity and harder to be understood.
        Are you OK with doing such detailed improvements as a separate task? Let's try to make this patch simply convert synchronized lock to R/W lock.

        Thanks,

        Show
        leftnoteasy Wangda Tan added a comment - Thanks for confirmation, Updated ver.5 patch addressed your comments. you dropped the lock instead of downgrading it before the log. In this patch, I didn't do improvements like downgrading locks in a method. Beyond FSAttempt.allocate/unreserveInternal, there're several similar methods in CS as well. I would prefer to have a separate JIRA to do more detailed improvements. And I don't know how much performance benefit we can gain from downgrading the lock. But it does make code becomes more complexity and harder to be understood. Are you OK with doing such detailed improvements as a separate task? Let's try to make this patch simply convert synchronized lock to R/W lock. Thanks,
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 23s Docker mode activated.
        +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.
        +1 mvninstall 8m 43s trunk passed
        +1 compile 0m 39s trunk passed
        +1 checkstyle 0m 25s trunk passed
        +1 mvnsite 0m 48s trunk passed
        +1 mvneclipse 0m 19s trunk passed
        +1 findbugs 1m 14s trunk passed
        +1 javadoc 0m 25s trunk passed
        +1 mvninstall 0m 39s the patch passed
        +1 compile 0m 39s the patch passed
        +1 javac 0m 39s the patch passed
        -1 checkstyle 0m 26s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 17 new + 48 unchanged - 23 fixed = 65 total (was 71)
        +1 mvnsite 0m 49s the patch passed
        +1 mvneclipse 0m 18s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 19s the patch passed
        +1 javadoc 0m 26s the patch passed
        -1 unit 43m 45s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 asflicense 0m 18s The patch does not generate ASF License warnings.
        62m 23s



        Reason Tests
        Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828870/YARN-3141.5.patch
        JIRA Issue YARN-3141
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 21f47d112817 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / b09a03c
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13125/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit https://builds.apache.org/job/PreCommit-YARN-Build/13125/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13125/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13125/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13125/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 23s Docker mode activated. +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. +1 mvninstall 8m 43s trunk passed +1 compile 0m 39s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 48s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 1m 14s trunk passed +1 javadoc 0m 25s trunk passed +1 mvninstall 0m 39s the patch passed +1 compile 0m 39s the patch passed +1 javac 0m 39s the patch passed -1 checkstyle 0m 26s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 17 new + 48 unchanged - 23 fixed = 65 total (was 71) +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 19s the patch passed +1 javadoc 0m 26s the patch passed -1 unit 43m 45s hadoop-yarn-server-resourcemanager in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 62m 23s Reason Tests Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestContinuousScheduling Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828870/YARN-3141.5.patch JIRA Issue YARN-3141 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 21f47d112817 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b09a03c Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13125/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit https://builds.apache.org/job/PreCommit-YARN-Build/13125/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt unit test logs https://builds.apache.org/job/PreCommit-YARN-Build/13125/artifact/patchprocess/patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13125/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13125/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        Are you OK with doing such detailed improvements as a separate task? Let's try to make this patch simply convert synchronized lock to R/W lock.

        Sure. Converting over to the R/W lock is certainly no worse than what was there before.

        I'm OK with the last version of this patch after you clear up the checkstyle issues.

        Show
        templedf Daniel Templeton added a comment - Are you OK with doing such detailed improvements as a separate task? Let's try to make this patch simply convert synchronized lock to R/W lock. Sure. Converting over to the R/W lock is certainly no worse than what was there before. I'm OK with the last version of this patch after you clear up the checkstyle issues.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks Daniel Templeton,

        Even though most of checkstyle warnings are not related to the patch, I fixed most of them in ver.6.

        Only a couple of checkstyle which complains:

        .. Variable 'queue' must be private and have accessor methods.

        We defines some variables like queue/stopped in base class (SchedulerApplicationAttempt) in protected. And we use these variables in inherited classes. I think it makes code to be more complex If we define methods like getQueue() to force sub class use these method. I would prefer to not change them.

        Thoughts?

        Show
        leftnoteasy Wangda Tan added a comment - Thanks Daniel Templeton , Even though most of checkstyle warnings are not related to the patch, I fixed most of them in ver.6. Only a couple of checkstyle which complains: .. Variable 'queue' must be private and have accessor methods. We defines some variables like queue/stopped in base class (SchedulerApplicationAttempt) in protected. And we use these variables in inherited classes. I think it makes code to be more complex If we define methods like getQueue() to force sub class use these method. I would prefer to not change them. Thoughts?
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 10s Docker mode activated.
        +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.
        +1 mvninstall 7m 55s trunk passed
        +1 compile 0m 36s trunk passed
        +1 checkstyle 0m 23s trunk passed
        +1 mvnsite 0m 39s trunk passed
        +1 mvneclipse 0m 18s trunk passed
        +1 findbugs 1m 2s trunk passed
        +1 javadoc 0m 23s trunk passed
        +1 mvninstall 0m 31s the patch passed
        +1 compile 0m 31s the patch passed
        +1 javac 0m 31s the patch passed
        -1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 5 new + 49 unchanged - 23 fixed = 54 total (was 72)
        +1 mvnsite 0m 38s the patch passed
        +1 mvneclipse 0m 15s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 11s the patch passed
        -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in the patch failed.
        +1 unit 34m 8s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 15s The patch does not generate ASF License warnings.
        50m 16s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828935/YARN-3141.6.patch
        JIRA Issue YARN-3141
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 82a34bcc44e4 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 8a40953
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13133/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13133/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13133/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13133/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated. +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. +1 mvninstall 7m 55s trunk passed +1 compile 0m 36s trunk passed +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 39s trunk passed +1 mvneclipse 0m 18s trunk passed +1 findbugs 1m 2s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 31s the patch passed +1 compile 0m 31s the patch passed +1 javac 0m 31s the patch passed -1 checkstyle 0m 19s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: The patch generated 5 new + 49 unchanged - 23 fixed = 54 total (was 72) +1 mvnsite 0m 38s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 11s the patch passed -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in the patch failed. +1 unit 34m 8s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 15s The patch does not generate ASF License warnings. 50m 16s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12828935/YARN-3141.6.patch JIRA Issue YARN-3141 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 82a34bcc44e4 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 8a40953 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-YARN-Build/13133/artifact/patchprocess/diff-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13133/artifact/patchprocess/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13133/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13133/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        No need to fix the accessor complaints.

        Show
        templedf Daniel Templeton added a comment - No need to fix the accessor complaints.
        Hide
        jianhe Jian He added a comment -

        Committed to trunk and branch-2, thanks Wangda Tan !

        Show
        jianhe Jian He added a comment - Committed to trunk and branch-2, thanks Wangda Tan !
        Hide
        jianhe Jian He added a comment -

        Thanks Daniel Templeton for reviewing the patch !

        Show
        jianhe Jian He added a comment - Thanks Daniel Templeton for reviewing the patch !
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10459 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10459/)
        YARN-3141. Improve locks in (jianhe: rev b8a30f2f170ffbd590e7366c3c944ab4919e40df)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/allocator/RegularContainerAllocator.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSAppAttempt.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java
        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/SchedulerApplicationAttempt.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10459 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10459/ ) YARN-3141 . Improve locks in (jianhe: rev b8a30f2f170ffbd590e7366c3c944ab4919e40df) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/allocator/RegularContainerAllocator.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FSAppAttempt.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/SchedulerApplicationAttempt.java
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks Jian He and Daniel Templeton for reviewing the patch!

        Show
        leftnoteasy Wangda Tan added a comment - Thanks Jian He and Daniel Templeton for reviewing the patch!
        Hide
        aw Allen Wittenauer added a comment -

        -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in the patch failed.

        I'm not sure why this error from precommit was ignored, but this patch just broke javadoc generation for everyone. Please fix or revert ASAP.

        Show
        aw Allen Wittenauer added a comment - -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in the patch failed. I'm not sure why this error from precommit was ignored, but this patch just broke javadoc generation for everyone. Please fix or revert ASAP.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Thanks for pointing out, Allen Wittenauer. Looking at it now.

        Show
        leftnoteasy Wangda Tan added a comment - Thanks for pointing out, Allen Wittenauer . Looking at it now.
        Hide
        leftnoteasy Wangda Tan added a comment -

        Ah my bad, there's one line change which causes javadocs fail.

        There're tons of message in javadocs building which start with "[ERROR]", however most of them are warnings, the real error message submerged in these output:

        [ERROR] /Users/wtan/project/github/hadoop-common-trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java:331: error: @param name not found

        Uploaded addendum-0 patch.

        Show
        leftnoteasy Wangda Tan added a comment - Ah my bad, there's one line change which causes javadocs fail. There're tons of message in javadocs building which start with " [ERROR] ", however most of them are warnings, the real error message submerged in these output: [ERROR] /Users/wtan/project/github/hadoop-common-trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java:331: error: @param name not found Uploaded addendum-0 patch.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 17s Docker mode activated.
        +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.
        +1 mvninstall 6m 38s trunk passed
        +1 compile 0m 32s trunk passed
        +1 checkstyle 0m 20s trunk passed
        +1 mvnsite 0m 38s trunk passed
        +1 mvneclipse 0m 17s trunk passed
        +1 findbugs 0m 58s trunk passed
        -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in trunk failed.
        +1 mvninstall 0m 31s the patch passed
        +1 compile 0m 32s the patch passed
        +1 javac 0m 32s the patch passed
        +1 checkstyle 0m 21s the patch passed
        +1 mvnsite 0m 44s the patch passed
        +1 mvneclipse 0m 16s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 1m 7s the patch passed
        -1 javadoc 0m 19s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 941 new + 0 unchanged - 942 fixed = 941 total (was 942)
        +1 unit 34m 18s hadoop-yarn-server-resourcemanager in the patch passed.
        +1 asflicense 0m 16s The patch does not generate ASF License warnings.
        49m 8s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829413/YARN-3141.addendum-0.patch
        JIRA Issue YARN-3141
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux a5169d83015a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / c6d1d74
        Default Java 1.8.0_101
        findbugs v3.0.0
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13163/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13163/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt
        Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13163/testReport/
        modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager
        Console output https://builds.apache.org/job/PreCommit-YARN-Build/13163/console
        Powered by Apache Yetus 0.3.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 17s Docker mode activated. +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. +1 mvninstall 6m 38s trunk passed +1 compile 0m 32s trunk passed +1 checkstyle 0m 20s trunk passed +1 mvnsite 0m 38s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 58s trunk passed -1 javadoc 0m 21s hadoop-yarn-server-resourcemanager in trunk failed. +1 mvninstall 0m 31s the patch passed +1 compile 0m 32s the patch passed +1 javac 0m 32s the patch passed +1 checkstyle 0m 21s the patch passed +1 mvnsite 0m 44s the patch passed +1 mvneclipse 0m 16s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 7s the patch passed -1 javadoc 0m 19s hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager generated 941 new + 0 unchanged - 942 fixed = 941 total (was 942) +1 unit 34m 18s hadoop-yarn-server-resourcemanager in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 49m 8s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829413/YARN-3141.addendum-0.patch JIRA Issue YARN-3141 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a5169d83015a 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c6d1d74 Default Java 1.8.0_101 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13163/artifact/patchprocess/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt javadoc https://builds.apache.org/job/PreCommit-YARN-Build/13163/artifact/patchprocess/diff-javadoc-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt Test Results https://builds.apache.org/job/PreCommit-YARN-Build/13163/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager Console output https://builds.apache.org/job/PreCommit-YARN-Build/13163/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
        Hide
        leftnoteasy Wangda Tan added a comment -

        The latest patch fixes the javadocs failures. All warnings are from existing CapacityScheduler, which are not caused by the patch.

        Committed to trunk/branch-2.

        Thanks again for pointing this out, Allen Wittenauer.

        Show
        leftnoteasy Wangda Tan added a comment - The latest patch fixes the javadocs failures. All warnings are from existing CapacityScheduler, which are not caused by the patch. Committed to trunk/branch-2. Thanks again for pointing this out, Allen Wittenauer .
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10467 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10467/)
        Addendum patch for fix javadocs failure which is caused by YARN-3141. (wangda: rev e45307c9a063248fcfb08281025d87c4abd343b1)

        • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10467 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10467/ ) Addendum patch for fix javadocs failure which is caused by YARN-3141 . (wangda: rev e45307c9a063248fcfb08281025d87c4abd343b1) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java
        Hide
        zhengchenyu zhengchenyu added a comment -

        I think read-write lock is Coarse-grained.
        For example, there is no need to use read-write lock in updateResourceRequests.updateResourceRequests, because isStopped is volatile and appSchedulingInfo is protected by itself.

        Show
        zhengchenyu zhengchenyu added a comment - I think read-write lock is Coarse-grained. For example, there is no need to use read-write lock in updateResourceRequests.updateResourceRequests, because isStopped is volatile and appSchedulingInfo is protected by itself.

          People

          • Assignee:
            leftnoteasy Wangda Tan
            Reporter:
            leftnoteasy Wangda Tan
          • Votes:
            0 Vote for this issue
            Watchers:
            14 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development