[
https://issues.apache.org/jira/browse/YARN-897?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13706327#comment-13706327
]
Carlo Curino commented on YARN-897:
-----------------------------------
Omkar, thanks for the quick feedback...
bq. any reason for this even after this patch? if we don't see any other issues
then why not just use childQueues.remove instead of iterating?
I initially thought the same, but I worried that since the underlying capacity
attribute has been changed, the TreeSet is already non-consistent? [~dedcode]
can you check whether this is true or not? Also can we use some careful
operation ordering, and get away with Omkar suggestion?
bq. reinsertQueue could be marked synchronized? thoughts? But yeah.. without
that too it is thread safe as we are locking it at
CapacitySchedulder.nodeUpdate(). but still it is better to mark it.
We should probably follow your suggestion (especially if this method will be
reused elsewhere), or at least use the lock annotations properly. (again this
patch wasn't quite ready)
bq. nit. line > 80
will do
bq. at present we send the container completed event to leaf queue and then
keep propagating it till root. why not sent the event to root grab the locks
from root->leaf and update it? any thoughts?
Lock ordering is somewhat delicate (and I worry not very consistent). In
general, the idea to lock bottom up should allow for part of the operations
(updating of two leaf queues) to be concurrent until the recursion meet at some
common ancestor, at which point we serialize. However, at least for some of the
operations this is inside a global scheduler lock, so we loose that benefit in
the first-place. It might be interesting to review the locks carefully and see
whether we can rationalize them further. Although this is delicate, and unless
we are lock-bound on the scheduler in practice would not buy us much.
We didn't have time to test this through to a level I would be confident PAing
this. Omkar do you have any cycle to test this? [~acmurthy],[~tgraves] do you
guys have a moment to review this?
BTW we are working on a discrete event simulator, which should allow us to
lock-step/debug the entire RM codebase... that would make for easy testing of
some of this stuff (more as soon as we get it ready to show it around).
> CapacityScheduler wrongly sorted queues
> ---------------------------------------
>
> Key: YARN-897
> URL: https://issues.apache.org/jira/browse/YARN-897
> Project: Hadoop YARN
> Issue Type: Bug
> Components: capacityscheduler
> Reporter: Djellel Eddine Difallah
> Attachments: TestBugParentQueue.java, YARN-897-1.patch
>
>
> The childQueues of a ParentQueue are stored in a TreeSet where UsedCapacity
> defines the sort order. This ensures the queue with least UsedCapacity to
> receive resources next. On containerAssignment we correctly update the order,
> but we miss to do so on container completions. This corrupts the TreeSet
> structure, and under-capacity queues might starve for resources.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
