[ https://issues.apache.org/jira/browse/YARN-3141?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15491810#comment-15491810 ]
Daniel Templeton commented on YARN-3141: ---------------------------------------- Thanks for the patch, [~leftnoteasy]. 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: {code} 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()); {code} 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... > Improve locks in SchedulerApplicationAttempt/FSAppAttempt/FiCaSchedulerApp > -------------------------------------------------------------------------- > > Key: YARN-3141 > URL: https://issues.apache.org/jira/browse/YARN-3141 > Project: Hadoop YARN > Issue Type: Sub-task > Components: resourcemanager, scheduler > Reporter: Wangda Tan > Assignee: Wangda Tan > Attachments: YARN-3141.1.patch, YARN-3141.2.patch > > > 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. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org