[
https://issues.apache.org/jira/browse/YARN-8379?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16524608#comment-16524608
]
Zian Chen edited comment on YARN-8379 at 6/27/18 5:52 AM:
--
Hi [~leftnoteasy]. thanks for the comments.
# This test case was intend to demonstrate selected candidates will be
actually killed after custom timeout was reached. This part of code is the
intention.
{code:java}
editPolicy.editSchedule();
Assert.assertEquals(4, editPolicy.getToPreemptContainers().size());
// check live containers immediately, nothing happen
Assert.assertEquals(39, schedulerApp1.getLiveContainers().size());
Thread.sleep(20*1000);
// Call editSchedule again: selected containers are killed
editPolicy.editSchedule();
waitNumberOfLiveContainersFromApp(schedulerApp1, 35);{code}
# Totally understand your suggestion here, actually I was define a private
member variable inside interface PreemptionCandidatesSelector so that we don't
need to pass curCandidates for every selector instance. However I found
TestCapacitySchedulerSurgicalPreemption#testPriorityPreemptionFromHighestPriorityQueueAndOldestContainer()
get failed which helped me realize that curCandidates is a "per round
reinitialize thing" rather than a "per selector" thing, we should give a clean
curCandidates HashMap every time we call editSchedule, otherwise like this UT,
we call editschedule multiple times but the selector remain the same instance,
then curCandidates will contains old selected containers the second time we
call the editSchedule, we could definitely make a method inside
PreemptionCandidatesSelector, and call it explicitly to reset curCandidates per
round, but this way it makes the code even harder to read. Any better
suggestions here?
# Your suggestion is not updating toPreempt on the fly but only update
curCandidates, then after each round, update toPreempt right after
selectCandidates is finished? If this is the correct understanding, I think
from time complexity point of view, these two strategy should be same since we
always need to go through each temp selected candidates and add it into
toPreempt with duplicate check, but it helps with maintaining status of
selected containers in one place(curCandidates) and avoid inconsistency. We can
further discuss this part.
I'll address the asflicense issue with the next patch. Thanks!
was (Author: zian chen):
Hi [~leftnoteasy]. thanks for the comments.
# This test case was intend to demonstrate selected candidates will be
actually killed after custom timeout was reached. This part of code is the
intention.
{code:java}
editPolicy.editSchedule();
Assert.assertEquals(4, editPolicy.getToPreemptContainers().size());
// check live containers immediately, nothing happen
Assert.assertEquals(39, schedulerApp1.getLiveContainers().size());
Thread.sleep(20*1000);
// Call editSchedule again: selected containers are killed
editPolicy.editSchedule();
waitNumberOfLiveContainersFromApp(schedulerApp1, 35);{code}
# Totally understand your suggestion here, actually I was define a private
member variable inside interface PreemptionCandidatesSelector so that we don't
need to pass curCandidates for every selector instance. However I found
TestCapacitySchedulerSurgicalPreemption#testPriorityPreemptionFromHighestPriorityQueueAndOldestContainer()
get failed which helped me realize that curCandidates is a "per round
reinitialize thing" rather than a "per selector" thing, we should give a clean
curCandidates HashMap every time we call editSchedule, otherwise like this UT,
we call editschedule multiple times but the selector remain the same instance,
then curCandidates will contains old selected containers the second time we
call the editSchedule, we could definitely make a method inside
PreemptionCandidatesSelector, and call it explicitly to reset curCandidates per
round, but this way it makes the code even harder to read. Any better
suggestions here?
# Your suggestion is not updating toPreempt on the fly but only update
curCandidates, then after each round, update toPreempt right after
selectCandidates is finished? If this is the correct understanding, I think
from time complexity point of view, these two strategy should be same since we
always need to go through each temp selected candidates and add it into
toPreempt with duplicate check, but it helps with maintaining status of
selected containers in one place(curCandidates) and avoid inconsistency. We can
further discuss this part.
I'll address the asflicense issue with the next patch. Thanks!
> Add an option to allow Capacity Scheduler preemption to balance satisfied
> queues
>
>
> Key: YARN-8379
> URL: https://issues.apache.org/jira/browse/YARN-8379
> Project: Hadoop YARN
>
