[
https://issues.apache.org/jira/browse/YARN-2009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15525174#comment-15525174
]
Wangda Tan commented on YARN-2009:
----------------------------------
Thanks [~sunilg],
More comments:
1) Config:
General, it's better to use "-" instead of "_", we made such mistakes in
previous preemption settings, let's use the new name convention for new added
configs.
And we can add a prefix for intra-queue preemption related configs to better
group these options.
1.1 select_based_on_intra_queue_policie rename to
{{intra-queue-preemption.enabled}}? For the name
{{select_based_on_intra_queue_policies}} you mentioned before, I would prefer
to avoid words like "policy", it is scary to end user :).
1.2 ignore_preemption_below_used_capacity to
{{intra-queue-preemption.minimum-threshold}}?
1.3 max_allowable_preempt_limit_for_intra_queue to
{{intra-queue-preemption.max-allowable-limit}}?
And to me all above options should have a default global option and per-queue
option, having different settings for different workload/queue is one of the
most important requirement of this feature to me.
2) PreemptionCandidateSelector,
Still have one TODO comment, plz delete if it is done.
3) IntraQueueCandidatesSelector
- fifoBasedPreemptionPolicy -> fifoPreemptionComputePlugin
- {{3. Loop through all partitions to calculate demand}}, demand is already
calculated, this is for select containers to be preempted, update comment?
- tryPreemptContainerAndDeductResToObtain, if they have same logic, can we
merge it with tryPreemptContainerAndDeductResToObtain of
FifoCandidatesSelector, which can be moved to CapacitySchedulerPreemptionUtils
4) TempQueuePerPartition,
- It seems to me unAllocated/selectedContainers are not required,
selectedContainers is not used. And unAllocated are all used within the same
code block, I would prefer to use a variable to store it.
5) FifoIntraQueuePreemptionPolicy
- Rename it to -Plugin?
- Rename calculateSelectedResourcePerApp ->
getAlreadySelectedPreemptionCandidateResource? ("SelectedResource" here is not
very clear), and add proper comment
- computeAppsIdealAllocation
a. initialIdealAssigned -> appIdealAssigned
b. resourceLimitPerUserForApp is not requried, appAssigned can be reused
c. remainingUsed -> appUsedExcludedSelected
d. As mentioned in 4), tq.unAllocated could move out to queueTotalUnassigned
e. computation of preemptionLimit should deduct already selected candidates?
f. orderedApps is invented to avoid sorting, correct? If you need to sort it
again, we don't need it.
g. As you mentioned, this method is a little condense, it's better to break
down to several children methods.
- validateOutSameAppPriorityFromDemand
a. My personal preferrence: use a TempAppPerPartition[] to store the data, and
use two integers to indicate head and tail. The Java Iterator is not very
readable to me :)
b. Suggest move it from TempQueuePerPartition to FifoIntraQueuePreemptionPolicy
6) AbstractKeyParamsForPreemption -> something like BasePreemptionEntity? The
"key param" is not quite understandable to me.
7) intraQPreemptableAmountCalculator:
- I think we may not need it to be a separated now, we have {{-Plugin}}
already, {{-Calculator}} is a little confusing with the {{-Plugin}} regarding
to responsibility, and it has only one public method, I think we can move it to
IntraQueueCandidatesSelector, thoughts?
- computeIntraQueuePreemptionDemand ->
computeIntraQueuePreemptionIdealAllocation? (Ideal allocation or
to-be-preempted is the goal, but demand is not)
8) Others:
- New method of SchedulerApplicationAttempt may not required, you can use
getAppAttemptResourceUsage instead.
> Priority support for preemption in ProportionalCapacityPreemptionPolicy
> -----------------------------------------------------------------------
>
> Key: YARN-2009
> URL: https://issues.apache.org/jira/browse/YARN-2009
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: capacityscheduler
> Reporter: Devaraj K
> Assignee: Sunil G
> Attachments: YARN-2009.0001.patch, YARN-2009.0002.patch,
> YARN-2009.0003.patch
>
>
> While preempting containers based on the queue ideal assignment, we may need
> to consider preempting the low priority application containers first.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
