hadoop-mapreduce-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hemanth Yamijala (JIRA)" <j...@apache.org>
Subject [jira] Commented: (MAPREDUCE-861) Modify queue configuration format and parsing to support a hierarchy of queues.
Date Mon, 31 Aug 2009 07:43:32 GMT

    [ https://issues.apache.org/jira/browse/MAPREDUCE-861?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12749406#action_12749406
] 

Hemanth Yamijala commented on MAPREDUCE-861:
--------------------------------------------

Started looking at the new patch. Some comments:

- DeprecatedQueueBuilder should not load the queue resources from mapred-queues.xml unless
it plans to parse the new format, which I think will complicate things. So, I suppose the
right thing to do is to have DeprecatedQueueBuilder build a hierarchy from the old format
configuration. If successful, we will simply discard the mapred-queues.xml file. This is a
change from existing trunk behavior, but I think it is simpler and correct, thoughts ?
- Also, DeprecatedQueueBuilder should build a hierarchy only if some configuration exists
in mapred-site.xml. For instance, since it is looking up for configured queues with a default
value of 'default', it will always end up loading a queue, which is not what we want. This
can be checked possibly by returning a true / false in the checkDeprecation method.
- We should simplify DeprecatedQueueBuilder to not initialize a root if there is no deprecated
configuration. If we do that, then in QueueManager constructor, this check becomes redundant:
{code}
        builder.getRoot().getChildren() == null 
{code}
- Do we need the setAclsEnabled() call from QueueManager, as it is only setting the acls in
the builder, which the builder object itself can do.
- Document in the constructor for the QueueManager(String confFilePath) that this should stick
to the format expected in mapred-queues.xml
- In populateProperties, we are checking for KEY_TAG being null, but not value tag.
- In getInnerQueues(), we are adding to the returned map, the list of children of the current
queue, as in:
{code}
        l.put(child.getName(),child);
{code}
Doesn't this mean that we are going to return leaf level queues as well, and not just returns
queues which have children ? For this same reason, it is behaving was similar to getDescendentQueues()
in MAPREDUCE-824 which is why I suggested that name.
- The refresh code could overwrite acls and state for some queues and if it gets a problem
in parsing other queues, it would leave the system in an inconsistent state. So, q.setAcls
and q.setState should be done only after refreshing the properties for all the queues.
- HierarchyQueueBuilder.refreshQueues should not take a configuration object, as it is unused.
(I suspect this *may* lead to a findbugs warning). That is the reason why I was confused about
the signature of this method in the abstract class. The only solution I can think of is to
handle refresh separately for the deprecated case. So, refreshQueues would not be part of
the AbstractQueueBuilder interface. Instead, HierarchyQueueBuilder would have a refreshQueues
that takes no parameter and DeprecatedQueueBuilder would have a refreshQueues with a Configuration
parameter, and depending on the type of the builder used, the QueueManager would cast and
make the right call. This is ugly, but noting that the refresh code is only at one place and
the deprecated code would get removed in Hadoop 0.22, I think we can live with it for one
release.
- Atleast one class (Queue.java) is still Importing all classes.
- Testcases in TestCapacitySchedulerConf may need a relook. Any test case that is reading
properties that no longer are in capacity-scheduler.xml are suspect and can be knocked out.


> 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-861-2.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.


Mime
View raw message