[ 
https://issues.apache.org/jira/browse/YARN-9879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17058407#comment-17058407
 ] 

Gergely Pollak commented on YARN-9879:
--------------------------------------

[~wangda] Thank you for the feedback, my original thought process was I will 
keep the getMap and fullQueue list as a concurent hash map, since those are the 
most frequently read maps, and probably ConcurentHashMap can handle the locking 
internally more efficiently than me externally. The other internal maps were 
protected by the locks you suggested. About synchronized, to be honest I simply 
missed removing those, I did not plan to use both, thank you for bringing it to 
my attention.

However the try / catch wrapping for locks was a huge deal, thanks for pointing 
it out.

Since the current partial locking solution was confusing, I've just simply 
removed the ConcurentHasmaps, and added locking to all externally accessible 
methods, which access any of the internal data structures, just as you've 
suggested.

[~prabhujoseph] I've managed to test the issues you've brought to my attention, 
and thank you for that again. Issue

1) Batch is ambiguous, so the rule will fail, if you use root.batch, as 
[~pbacsko] suggested, it will work from now on, it had not before this fix, 
thanks for finding the issue.

2) It is a bit more tricky, I had a discussion with [~wangda] and [~wilfreds] 
and the conclusion was, if there are any queue name collisions (even between 
parent and leaf), we simply mark those ambiguous, and we won't allow access any 
of those queues by their short name. With the exception of root, root is always 
accessible, and always means root, root is protected, root is king. This goes 
against what I've said in one of my previous comments, but currently if we have 
these queues:

root.a.b

root.b.a

Neither a or b can be accessed by their name. It is possible to give leaf queue 
a priority, however in that case the ambiguity check becomes a bit harder, and 
the CSQueue store will become a lot more complex. But I can do it, however we 
need a consensus about it.

 

Also fixed a bunch of checkstyle issues, if there is no regression due to the 
changes I've made to fix Prabhu's findings, I only have to remove the TODOs and 
create Jiras about them, and then we are getting ready.

 

> Allow multiple leaf queues with the same name in CS
> ---------------------------------------------------
>
>                 Key: YARN-9879
>                 URL: https://issues.apache.org/jira/browse/YARN-9879
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>            Reporter: Gergely Pollak
>            Assignee: Gergely Pollak
>            Priority: Major
>              Labels: fs2cs
>         Attachments: CSQueue.getQueueUsage.txt, DesignDoc_v1.pdf, 
> YARN-9879.POC001.patch, YARN-9879.POC002.patch, YARN-9879.POC003.patch, 
> YARN-9879.POC004.patch, YARN-9879.POC005.patch, YARN-9879.POC006.patch, 
> YARN-9879.POC007.patch, YARN-9879.POC008.patch, YARN-9879.POC009.patch, 
> YARN-9879.POC010.patch, YARN-9879.POC011.patch, YARN-9879.POC012.patch, 
> YARN-9879.POC013.patch
>
>
> Currently the leaf queue's name must be unique regardless of its position in 
> the queue hierarchy. 
> Design doc and first proposal is being made, I'll attach it as soon as it's 
> done.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to