[
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: [email protected]
For additional commands, e-mail: [email protected]
