[
https://issues.apache.org/jira/browse/YARN-10674?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17302548#comment-17302548
]
Peter Bacsko commented on YARN-10674:
-------------------------------------
Thanks [~zhuqi] this is definitely looks better. We're close to the final
version.
Some comments:
1.
{noformat}
Disable the preemption with nopolicy or observeonly mode, " +
"default mode is nopolicy with no arg." +
"When use nopolicy arg, it means to remove " +
"ProportionalCapacityPreemptionPolicy for CS preemption, " +
"When use observeonly arg, " +
"it means to set " +
"yarn.resourcemanager.monitor.capacity.preemption.observe_only " +
"to true"
{noformat}
I'd to slightly modify this text:
{noformat}
Disable the preemption with \"nopolicy\" or \"observeonly\" mode.
Default is \"nopolicy\".
\"nopolicy\" removes ProportionalCapacityPreemptionPolicy from
the list of monitor policies.
\"observeronly\" sets
\"yarn.resourcemanager.monitor.capacity.preemption.observe_only\"
to true.
{noformat}
2. This definition:
{{private String disablePreemptionMode;}}
This should be a simple enum like:
{noformat}
public enum DisablePreemptionMode {
OBSERVE_ONLY {
@Override
String getCliOption() {
return "observeonly";
}
},
NO_POLICY {
@Override
String getCliOption() {
return "nopolicy";
}
};
abstract String getCliOption();
}
{noformat}
So you can also use them here:
{noformat}
private static void checkDisablePreemption(CliOption cliOption,
String disablePreemptionMode) {
if (disablePreemptionMode == null ||
disablePreemptionMode.trim().isEmpty()) {
// The default mode is nopolicy.
return;
}
try {
DisablePreemptionMode.valueOf(disablePreemptionMode);
} catch (IllegalArgumentException e) {
throw new PreconditionException(
String.format("Specified disable-preemption option %s is
illegal, " +
" use \"nopolicy\" or \"observeonly\""));
}
{noformat}
"disablePreemptionMode" should be an enum everywhere.
3.
{noformat}
public void convertSiteProperties(Configuration conf,
Configuration yarnSiteConfig, boolean drfUsed, boolean
enableAsyncScheduler)
boolean enableAsyncScheduler, boolean userPercentage,
boolean disablePreemption, String disablePreemptionMode) {
{noformat}
Here "disablePreemptionMode" should be an enum also and make sure that it
always has a value. If it always has a value, this part becomes much simpler:
{noformat}
if (disablePreemption &&
disablePreemptionMode == DisablePreemptionMode.NO_POLICY) {
yarnSiteConfig.set(YarnConfiguration.RM_SCHEDULER_MONITOR_POLICIES, "");
}
}
{noformat}
4.
{{AutoCreatedQueueDeletionPolicy.class.getCanonicalName())}}
This string is referenced very often in the tests. Instead, use a final String:
{noformat}
private static final String DELETION_POLICY_CLASS =
AutoCreatedQueueDeletionPolicy.class.getCanonicalName();
{noformat}
So the readability becomes much better.
> fs2cs: should support auto created queue deletion.
> --------------------------------------------------
>
> Key: YARN-10674
> URL: https://issues.apache.org/jira/browse/YARN-10674
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Qi Zhu
> Assignee: Qi Zhu
> Priority: Major
> Labels: fs2cs
> Attachments: YARN-10674.001.patch, YARN-10674.002.patch,
> YARN-10674.003.patch, YARN-10674.004.patch, YARN-10674.005.patch,
> YARN-10674.006.patch, YARN-10674.007.patch, YARN-10674.008.patch,
> YARN-10674.009.patch, YARN-10674.010.patch
>
>
> In FS the auto deletion check interval is 10s.
> {code:java}
> @Override
> public void onCheck() {
> queueMgr.removeEmptyDynamicQueues();
> queueMgr.removePendingIncompatibleQueues();
> }
> while (running) {
> try {
> synchronized (this) {
> reloadListener.onCheck();
> }
> ...
> Thread.sleep(reloadIntervalMs);
> }
> /** Time to wait between checks of the allocation file */
> public static final long ALLOC_RELOAD_INTERVAL_MS = 10 * 1000;{code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
