[ https://issues.apache.org/jira/browse/MAPREDUCE-861?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12749304#action_12749304 ]
rahul k singh commented on MAPREDUCE-861: ----------------------------------------- Attaching the new patch with above comments taken care of , except few of them: bq. createHierarchy should be refactored to parse leaf level queues and non leaf level queues separately. I would suggest something along the lines of: - The existing implementation is better , as it traverses much lesser time than the suggestion above. bq. In QueueManager.initialize, allQueues.putAll(leafQueues) seems unnecessary, since getInnerQueues already does this. - getInnerQueues returns the list of inner queues . i.e . queues which have children. Hence the union of leafQueues and getInnerQueues would be the complete list. However in the deprecated conf case , leafQueue is sufficient. bq. I assume the order of child elements of an XML tag will be in the order in which they are defined in the file. I think we must explicitly ensure that elements come in a fixed order. For e.g. currently, we are not enforcing an order on how tags under the 'queue' node are defined. This could lead to cases where if a 'queue' element comes before the 'name' element, we would call parse hierarchy for the child queue without the parent's name. - Done, but in different way. We still don't enforce the order ,but we make sure in code that parent is fully created before there is an attempt to create the child. Hence instead of depth first traversing we are now doing breadth first traversing. bq. getInnerQueues can be renamed as getDescendentQueues - getInnerQueus method intent is to return list of queues which have children. Discussion points : bq. Do we need leafQueues and allQueues stored separately ? - There are more than one occasions where we need only leafQueues to be used and more than one for only allQueues. Initial thought was to maintain the list of inner queues(i.e , queues which always have children) and leaf queues and generate there union whenever we need to access all the queues. Decided against it , as cost of putAll()(which we will call while union) would be costly if we need to do that everytime and also The cost of extra storage of leafQueues is not very high. bq. Should we validate using an XSD ? I know you were planning on this. After looking at the code, I am thinking this makes good sense. - Yes i agree with this. bq. I am unable to decide if refreshConfiguration should be in the base class for QueueHierarchyBuilder, because it would need a Configuration in the deprecated case, and none in the new case. - As of now we have single method refreshConfiguration(Configuration conf) and we pass null in case of HierarchyQueueBuilder class. bq. We need a test case to test default values in mapred-queues.xml, which will just have the default queue, with no children and no properties, and acls disabled. bq. dumpConfiguration is not yet done , will add 1 more patch to cover that. - Wiil do above 2 and upload the new patch. > Modify queue configuration format and parsing to support a hierarchy of > queues. > ------------------------------------------------------------------------------- > > Key: MAPREDUCE-861 > URL: https://issues.apache.org/jira/browse/MAPREDUCE-861 > Project: Hadoop Map/Reduce > Issue Type: Sub-task > Reporter: Hemanth Yamijala > Assignee: rahul k singh > Attachments: MAPREDUCE-861-1.patch > > > MAPREDUCE-853 proposes to introduce a hierarchy of queues into the Map/Reduce > framework. This JIRA is for defining changes to the configuration related to > queues. > The current format for defining a queue and its properties is as follows: > mapred.queue.<queue-name>.<property-name>. For e.g. > mapred.queue.<queue-name>.acl-submit-job. The reason for using this verbose > format was to be able to reuse the Configuration parser in Hadoop. However, > administrators currently using the queue configuration have already indicated > a very strong desire for a more manageable format. Since, this becomes more > unwieldy with hierarchical queues, the time may be good to introduce a new > format for representing queue configuration. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.