[
https://issues.apache.org/jira/browse/YARN-2495?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14243283#comment-14243283
]
Craig Welch commented on YARN-2495:
-----------------------------------
Other comments on the patch as such, assuming we really do need this part of
the change...
DECENTRALIZED_CONFIGURATION_ENABLED et all - I do see the basis for enabling
and disabling the enforcement of a centralized list and discussion around that,
but I don't see any reason to have conditional enablement of the node manager
side of things as well as a provider specification and I think it just adds
unnecessary complexity and possible surprise at configuration time - at the
level of this configuration I think it should just be enabled (and I don't mean
just by default, I mean if we add this, it should just be a way to manage node
labels, not conditionally enabled or disabled, any more than the web service or
cli are conditionally enabled or disabled, and so we don't have this parameter
/ it's associated branching at all).
I think the default node labels provider service should definitely be a null
provider that always returns an empty list and areLabelsUpdated false - this
takes out the need to decide "which is default", a no-op one is, and it allows
us to get rid of the extra enabled/disabled configuration above without adding
a new configuration (the provider will be specified anyway if the feature is
going to be used)
NodeHeartbeatRequest -
isNodeLabelsUpdated - I would go with areNodeLablesSet (all "isNodeLabels" =>
"areNodeLabels" wherever it appears, actually) - wrt "Set" vs "Updated" - this
is primarily a workaround for the null/empty ambiguity and I think this name
better reflects what is really going on (am I sending a value to act on or
not), but I also think that this is a better contract, the receiver (rm)
shouldn't really care about the logic the nm side is using to decide whether or
not to set it's labels (freshness, "updatedness", whatever), so all that should
be communicated in the api is whether or not the value is set, not whether it's
an update/whether it's checking freshness, etc. that's a nit, but I think it's
a clearer name.
RegisterNodeLabelManagerResponse - get/set IsNodeLabelsAcceptedByRM - I would
make it get/set AreNodeLablesAcceptedByRM (and on imples, etc, of course)
RegisterNodeManagerRequest - missing spaces in args (l 42) also, assuming we
drop the "distributed on/off" config as I'm suggesting, you'll need the
"areNodeLablesSet" to be passed here as well. (I also like this better because
it harmonizes the api between registration and heartbeat, which is easier to
understand b/c they are doing the same thing / should do it the same way).
> Allow admin specify labels from each NM (Distributed configuration)
> -------------------------------------------------------------------
>
> Key: YARN-2495
> URL: https://issues.apache.org/jira/browse/YARN-2495
> Project: Hadoop YARN
> Issue Type: Sub-task
> Components: resourcemanager
> Reporter: Wangda Tan
> Assignee: Naganarasimha G R
> Attachments: YARN-2495.20141023-1.patch, YARN-2495.20141024-1.patch,
> YARN-2495.20141030-1.patch, YARN-2495.20141031-1.patch,
> YARN-2495.20141119-1.patch, YARN-2495.20141126-1.patch,
> YARN-2495.20141204-1.patch, YARN-2495.20141208-1.patch,
> YARN-2495_20141022.1.patch
>
>
> Target of this JIRA is to allow admin specify labels in each NM, this covers
> - User can set labels in each NM (by setting yarn-site.xml (YARN-2923) or
> using script suggested by [~aw] (YARN-2729) )
> - NM will send labels to RM via ResourceTracker API
> - RM will set labels in NodeLabelManager when NM register/update labels
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
