[ 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