[
https://issues.apache.org/jira/browse/MAPREDUCE-1067?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Hemanth Yamijala updated MAPREDUCE-1067:
----------------------------------------
Status: Open (was: Patch Available)
I have some feedback:
- In QueueConfigurationParser.getQueueElement, we have specifically removed the
check queueState != null when appending a 'state' element to the Queue element.
It seems like the check is useful for preventing errors. Is there a specific
reason for doing this ? For e.g. would it not be true that there could be a
leaf queue given a null State on construction ? We should either prevent it in
the construction or here, no ?
- bq. Also, state of a container queue should not be allowed to change once the
hierarchy is built.
I think the way this works is when queue configuration is refreshed, a state
change for container queues will result in exceptions being thrown when the
QueueConfigurationParser parses and constructs Queue objects (using setState or
addChild). This results in a RuntimeException that will be handled only by the
IPC handler on the server. I think no useful information about the error will
reach the admin. Can you please verify if this is the case ? If yes, I think it
must be fixed to be handled more gracefully, and a better error message must be
given. Same holds for first time construction of hierarchy as well.
- testQueueState should be split into multiple test cases. Typically, it is
good practice to test one condition / aspect of a unit in one unit test case.
It makes test case maintenance easier. There are atleast 3 distinct tests I am
seeing in this test case
- The first part of the test testQueueState is verifying a message that doesn't
seem correct. For e.g. I think the XML is semantically correct, but since a
queue state is defined for a container queue, construction fails. Then why are
we getting an exception whose message is "Malformed xml formation queue tag and
acls tags or state tags are siblings" ? The code is incorrect if this test is
passing.
- Shouldn't there be test cases testing refresh of queue state for container
queues and verify they fail ?
- There should also be a case to test that when a queue is constructed with
state stopped, and then children are added to it, an exception is thrown. It is
arguable if this is the right design - i.e. we should probably have
ContainerQueue as a first class citizen and then we can always prevent an
invalid state for such an instance, but possibly that's a separate JIRA.
> Default state of queues is undefined when unspecified
> -----------------------------------------------------
>
> Key: MAPREDUCE-1067
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-1067
> Project: Hadoop Map/Reduce
> Issue Type: Bug
> Components: jobtracker
> Affects Versions: 0.21.0
> Reporter: V.V.Chaitanya Krishna
> Assignee: V.V.Chaitanya Krishna
> Priority: Blocker
> Fix For: 0.21.0
>
> Attachments: MAPREDUCE-1067-1.patch, MAPREDUCE-1067-2.patch,
> MAPREDUCE-1067-3.patch, MAPREDUCE-1067-4.patch, MAPREDUCE-1067-5.patch,
> MAPREDUCE-1067-6.patch
>
>
> Currently, if the state of a queue is not specified, it is being set to
> "undefined" state instead of running state.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
