[
https://issues.apache.org/jira/browse/MAPREDUCE-893?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12756058#action_12756058
]
rahul k singh commented on MAPREDUCE-893:
-----------------------------------------
Some comments:
QueueManager
1. TO DO : Comment for getQueueConfigurationParser is not required.
Dont think there is any need of introducing parser.parse method. As for ,
there will always be 1 parser that is QueueConfigurationParser
and existing DeprecatedConfigurationParser is just for deprecation and would
go away.
2.The parameter name in "synchronized void refreshQueues(Configuration conf,
QueueRefresher scheduler)"
) should be refresher instead of scheduler.
QueueRefresher
1.QueueRefresher should be an abstract class with refreshQueues being abstract.
Can it be something like this:
-----------------------------
abstract class QueueRefresher {
abstract void refreshQueues(List<JobQueueInfo> newRootQueues)
throws Throwable;
}
QueueRefresher getQueueRefresher() {
return null;
}
}
Queue
1.In QueueState no need for enumMap. QueueState.valueOf(String name) would give
the QueueState object.
2.newState.getChildren().size() state in isHierarchySame would return
NullPointerException
incase of no children.
3.Do we need to define an extra state of UNDEFINED in QueueState , as Queues
with no state would have a default state of RUNNING.
which is same behaviour before states were introduced.
It basically boils down to the question , what is the value of state in case
of queues with children.
CapacityTaskScheduler
1. Remove instance of schedConf.
CapacitySchedulerConf constructor should just be instantiated and get the value
most of the time we any how use QueueSchedulingContext values.
Incase of not passing any properties ,CapacitySchedulerConf would give default
values.
Incase of distributedUnConfiguredCapacity some of the testcases directly check
the configuration xml.So they need to be taken care of.
2. Remove SchedulingDisplayInfo object alltogether , instead pass
QueueSchedulingContext directly.
Also check incase above case do we need queueNameToQueueContextMap anymore.
3. We can remove the synchronized block from initializeQueues , instead we can
have more fine grained locking
when we just change root or update root or not at all.
4. We can move most of the code inside initializeQueues to
QueueHierarchyBuilder.
so initializeQueues becomes following :
QueueHierarchyBuilder builder = new QueueHierarchyBuilder();
if(refreshingQueues){
//do current stuff
}else{
this.root = builder.getRoot();
}
5. instead of updateConfigurationOfAbstractQueues in CapacityTaskScheduler ,
can we move this method to AbstractQueues class ,
so we can call root.refresh(newRoot) such that parents let there
children update themselves first then parents update themselves.
so refreshing logic becomes simply as.
root.sort(comparator);
newRoot.sort(comparator);
root.refresh(newRoot);
6.updateQueueMaps updates jobQueueManager unnecessarily as the list of JobQueue
should not be changed in case of refresh.
this method might need to be revisited once we implement 2. This would also
remove the need of synchronization in JobQueuesManager
7.getOrderedQueue signature should be reverted back to return AbstractQueue
8.initializationPoller instance variable scope is changed to package it shuold
be private
9.getDisplayInfo neednot be synchronized as it is only reading data.
AbstractQueue
1. It is incorrect to change the signature of getDescendentJobQueues to return
List of JobQueue. return List of AbstractQueue provides
the correct abstraction. JobQueue is just another specialized AbstractQueue
with containers for JIP.
JobQueuesManager
1. We dont need to synchronize any of the methods in JobQueuesManager. As it is
always called from JobTracker in synchronized block
also the queue structure jobQueues is only created once and never changed.
double check ??
TaskSchedulingContext
1.Removal of unused code. not related to patch though
JobQueueInfo
1. there is no need for maintaining the fullyQualifiedName and name separatly .
Instead we shuold always use fullyQualified name everywhere
and can provide method to get the normal queueName.
> Provide an ability to refresh queue configuration without restart.
> ------------------------------------------------------------------
>
> Key: MAPREDUCE-893
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-893
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: jobtracker
> Reporter: Hemanth Yamijala
> Assignee: Vinod K V
> Attachments: MAPREDUCE-893-20090915.1.txt
>
>
> While administering a cluster using multiple queues, administrators feel a
> need to refresh queue properties on the fly without needing to restart the
> JobTracker. This is partially supported for some properties such as queue
> ACLs (HADOOP-5396) and state (HADOOP-5913). The idea is to extend the
> facility to refresh other queue properties as well, including scheduler
> properties.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
