[
https://issues.apache.org/jira/browse/YARN-10532?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17276450#comment-17276450
]
Andras Gyori commented on YARN-10532:
-------------------------------------
Hi [~zhuqi], thank you for the patch! I have analysed the provided code and I
have the following suggestions:
h2. AbstractCSQueue
Since the queues have some common conditional, I think we could encapsulate
these logic in methods inside the queue, which greatly simplifies the overall
code. I would advise to add:
{code:java}
public boolean isEligibleForAutoDeletion() { return false; }
public boolean isInactiveDynamicQueue() {
long idleDurationSeconds = Time.monotonicNow() - lastSubmittedTimestamp;
return isDynamicQueue() && isEligibleForAutoDeletion() &&
(idleDurationSeconds > this.csContext.getConfiguration().
getAutoExpiredDeletionTime());
}
{code}
h2. LeafQueue
Since LeafQueues have their own way of deciding whether it is eligible for
deletion, override isEligibleForAutoDeletion:
{code:java}
@Override
public boolean isEligibleForAutoDeletion() {
return isDynamicQueue() && getNumActiveApplications() == 0;
}
{code}
h2. ParentQueue
Same goes for ParentQueues:
{code:java}
@Override
public boolean isEligibleForAutoDeletion() {
return isDynamicQueue() && getChildQueues().size() == 0;
}
{code}
Also, the lastSubmittedTimestamp means something else in case of ParentQueues.
When we add a new child queue, we refresh this timestamp, just as we refreshed
the LeafQueue's timestamp on application submission:
{code:java}
private CSQueue addDynamicChildQueue(String childQueuePath, boolean isLeaf)
throws SchedulerDynamicEditException {
...
CSQueue newQueue = createNewQueue(childQueuePath, isLeaf);
this.childQueues.add(newQueue);
signalToSubmitToQueue();
...
}
{code}
h2. CapacityScheduler
The logic in CapacityScheduler#AutoCreatedQueueDeletion (which violates the
Java naming convention by the way, so it should be renamed) will be simplified
to:
{code:java}
private void AutoCreatedQueueDeletion(CSQueue checkQueue)
throws SchedulerDynamicEditException{
writeLock.lock();
try {
if (checkQueue instanceof AbstractCSQueue
&& ((AbstractCSQueue) checkQueue).isInactiveDynamicQueue()) {
removeQueue(checkQueue);
}
} finally {
writeLock.lock();
}
}
{code}
h2. AutoDeletionForExpiredQueuePolicy
I think this policy should be changed a bit. I advise to add two sets:
{code:java}
private Set<String> markedForDeletion = new HashSet<>();
private Set<String> sentForDeletion = new HashSet<>();
{code}
* markedForDeletion: in each interval, this set is extended by queues that are
eligible for auto deletion
* sentForDeletion: if in the next interval, there is queue, that is eligible
for auto deletion, and is already marked for deletion, move it to this queue
* if a queue is in markedForDeletion in the next interval, but not moved to
sentForDeletion, its mark should be reset (therefore clear the
markedForDeletion)
The logic to invoke in each interval is:
{code:java}
for (Map.Entry<String, CSQueue> queues : scheduler
.getCapacitySchedulerQueueManager()
.getQueues().entrySet()) {
String queueName = queues.getKey();
CSQueue queue = queues.getValue();
Set<String> newMarks = new HashSet<>();
if (queue instanceof AbstractCSQueue &&
((AbstractCSQueue) queue).isEligibleForAutoDeletion()) {
if (markedForDeletion.contains(queueName)) {
sentForDeletion.add(queueName);
markedForDeletion.remove(queueName);
} else {
newMarks.add(queueName);
}
}
markedForDeletion.clear();
markedForDeletion.addAll(newMarks);
}
for (String queueName : sentForDeletion) {
CSQueue checkQueue =
scheduler.getCapacitySchedulerQueueManager().
getQueue(queueName);
queueAutoDeletionCheck(checkQueue);
}
sentForDeletion.clear();
{code}
The monitoringInterval should be set to the value of the queue-expiration-time,
which you have introduced.
{code:java}
monitoringInterval = csConfig.getAutoExpiredDeletionTime(); {code}
Also please fix the following items:
* AutoCreatedQueueDeletionCheckEvent should be renamed to only
AutoCreatedQueueDeletionEvent
* AutoCreatedQueueDeletion should be renamed to removeAutoCreatedQueue
* AutoDeletionForExpiredQueuePolicy should be renamed to
AutoCreatedQueueDeletionPolicy
* The config expired-deletion-enabled should be renamed to
queue-auto-removal.enabled
* The config expired-deletion-time should be renamed to queue-expiration-time
* There are a handful of unused imports and methods, which should be removed
* Please do not use wildcard import in CapacityScheduler (line 42)
> Capacity Scheduler Auto Queue Creation: Allow auto delete queue when queue is
> not being used
> --------------------------------------------------------------------------------------------
>
> Key: YARN-10532
> URL: https://issues.apache.org/jira/browse/YARN-10532
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Wangda Tan
> Assignee: zhuqi
> Priority: Major
> Attachments: YARN-10532.001.patch, YARN-10532.002.patch,
> YARN-10532.003.patch, YARN-10532.004.patch, YARN-10532.005.patch,
> YARN-10532.006.patch, YARN-10532.007.patch, YARN-10532.008.patch
>
>
> It's better if we can delete auto-created queues when they are not in use for
> a period of time (like 5 mins). It will be helpful when we have a large
> number of auto-created queues (e.g. from 500 users), but only a small subset
> of queues are actively used.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
