Varun Saxena commented on YARN-3075:

[~leftnoteasy], for other review comments.

bq. 2) getLabelsToNodes: When there's no NodeLabel associated with label, it's 
better print warn message.

bq. 4) NodeLabel.getNodeIdInfo may not precise enough, rename to 
getAssociatedNode(Id)s? (or other name if you have)

bq. 5) Also NodeLabel.getNodeIdInfo: I think we can assume NodeId will not 
changed (nobody calls NodeId.setHost/Port), so copy reference should be enough, 
agree? If yes, just return new HashSet<NodeId>(nodeIds).
I had kept it like this because same NodeId will be shared across threads after 
call to CommonNodeLabelsManager#getLabelsToNodes completes. My major concern 
was there should be no repitition of an issue like YARN-2978 . But QueueInfo 
there had an underlying list. And in current code, there should be no call to 
setHost/Port, so we can change it like above.

bq. You can use getLabelsByNode to get labels from Host-Node hierarchy.
I am not sure why I added both host and nm labels in oldLabels but on the face 
of it, the pre-existing function can be used. Will change the code. 3) is 
related to this comment so getHostLabels can be removed as well.

bq. 8) Add nodeId to Node to avoid loop like:
Sorry didn't quite get as to what you mean by this.

bq. 7) When a node/host has no label, it belongs to a special NodeLabel with 
key = CommonNodeLabelsManager.NO_LABEL. This is necessary because node without 
label can be considered as a "partition" as well. We need support it here (even 
if getLabelsToNode not return it now.).
Support it in what sense ? 

> NodeLabelsManager implementation to retrieve label to node mapping
> ------------------------------------------------------------------
>                 Key: YARN-3075
>                 URL: https://issues.apache.org/jira/browse/YARN-3075
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: resourcemanager
>    Affects Versions: 2.7.0
>            Reporter: Varun Saxena
>            Assignee: Varun Saxena
>         Attachments: YARN-3075.001.patch, YARN-3075.002.patch

This message was sent by Atlassian JIRA

Reply via email to