[
https://issues.apache.org/jira/browse/YARN-7574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16410291#comment-16410291
]
Wangda Tan commented on YARN-7574:
----------------------------------
Thanks [~suma.shivaprasad],
In general the patch is very nice, some comments:
1) Changes in SingleConstraintAppPlacementAllocator:
- Before the patch only SELF supported, after this patch several ALLOCATION_TAG
supported. I'm not sure if we should do that since we haven't validated if
inter-app target works for SingleConstraintAppPlacementAllocator.
2) TargetApplications:
- Removed unused methods.
3) aggregateAllocationTagsByApps/aggregateAllocationTagsByRack
- The two methods can be merged.
- appIds != null check is redundant (since
{{AllocationTagNamespace#getNamespaceScope}} return non-null always).
4) getNamespaceScope:
- There're several "scope" related methods, I suggest to rename them to
get/setApplicationIds. The name of "scope" is confusing to me.
- Following methods are used by test only, suggest to remove them:
{code}
/**
* @return true if the namespace is effective in all applications
* in this cluster. Specifically the namespace prefix should be
* "all".
*/
public boolean isGlobal() {
return AllocationTagNamespaceType.ALL.equals(getNamespaceType());
}
/**
* @return true if the namespace is effective within a single application
* by its application ID, the namespace prefix should be "app-id";
* false otherwise.
*/
public boolean isSingleInterApp() {
return AllocationTagNamespaceType.APP_ID.equals(getNamespaceType());
}
/**
* @return true if the namespace is effective to the application itself,
* the namespace prefix should be "self"; false otherwise.
*/
public boolean isIntraApp() {
return AllocationTagNamespaceType.SELF.equals(getNamespaceType());
}
/**
* @return true if the namespace is effective to all applications except
* itself, the namespace prefix should be "not-self"; false otherwise.
*/
public boolean isNotSelf() {
return AllocationTagNamespaceType.NOT_SELF.equals(getNamespaceType());
}
{code}
And actually I suggest to remove all the is... methods since we can check
{{AllocationTagNamespaceType.XYZ.equals(getNamespaceType)}}. This is optional
to me, depends on you.
- Several checks are check unsupported namespaces:
{code}
// TODO Complete remove this check once we support app-label.
if (namespace.isAppLabel()) {
throw new InvalidAllocationTagsQueryException(
namespace.toString() + " is not supported yet!");
}
{code}
I suggest to add a method like {{throwExceptionIfNamespaceTypeNotSupported}} so
we don't need to change all the places in the future.
> Add support for Node Labels on Auto Created Leaf Queue Template
> ---------------------------------------------------------------
>
> Key: YARN-7574
> URL: https://issues.apache.org/jira/browse/YARN-7574
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: capacity scheduler
> Reporter: Suma Shivaprasad
> Assignee: Suma Shivaprasad
> Priority: Major
> Attachments: YARN-7574.1.patch, YARN-7574.2.patch, YARN-7574.3.patch,
> YARN-7574.4.patch, YARN-7574.5.patch
>
>
> YARN-7473 adds support for auto created leaf queues to inherit node labels
> capacities from parent queues. Howebver there is no support for leaf queue
> template to allow different configured capacities for different node labels.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
