[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14203469#comment-14203469 ] Hudson commented on YARN-2753: -- FAILURE: Integrated in Hadoop-Mapreduce-trunk #1951 (See [https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1951/]) YARN-2753. Fixed a bunch of bugs in the NodeLabelsManager classes. Contributed by Zhihai xu. (vinodkv: rev 4cfd5bc7c18bb9a828f573b5c4d2b13fa28e732a) * hadoop-yarn-project/CHANGES.txt * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Fix For: 2.6.0 > > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14203444#comment-14203444 ] Hudson commented on YARN-2753: -- FAILURE: Integrated in Hadoop-Hdfs-trunk #1927 (See [https://builds.apache.org/job/Hadoop-Hdfs-trunk/1927/]) YARN-2753. Fixed a bunch of bugs in the NodeLabelsManager classes. Contributed by Zhihai xu. (vinodkv: rev 4cfd5bc7c18bb9a828f573b5c4d2b13fa28e732a) * hadoop-yarn-project/CHANGES.txt * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Fix For: 2.6.0 > > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14203399#comment-14203399 ] Hudson commented on YARN-2753: -- FAILURE: Integrated in Hadoop-Yarn-trunk #737 (See [https://builds.apache.org/job/Hadoop-Yarn-trunk/737/]) YARN-2753. Fixed a bunch of bugs in the NodeLabelsManager classes. Contributed by Zhihai xu. (vinodkv: rev 4cfd5bc7c18bb9a828f573b5c4d2b13fa28e732a) * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java * hadoop-yarn-project/CHANGES.txt * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Fix For: 2.6.0 > > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202847#comment-14202847 ] zhihai xu commented on YARN-2753: - thanks [~vinodkv] for the thorough review and commit, thanks [~leftnoteasy] to review and coordinate the patch with other patches. > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Fix For: 2.6.0 > > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202824#comment-14202824 ] Hudson commented on YARN-2753: -- FAILURE: Integrated in Hadoop-trunk-Commit #6485 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/6485/]) YARN-2753. Fixed a bunch of bugs in the NodeLabelsManager classes. Contributed by Zhihai xu. (vinodkv: rev 4cfd5bc7c18bb9a828f573b5c4d2b13fa28e732a) * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/nodelabels/TestRMNodeLabelsManager.java * hadoop-yarn-project/CHANGES.txt * hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/nodelabels/CommonNodeLabelsManager.java > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Fix For: 2.6.0 > > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202802#comment-14202802 ] Vinod Kumar Vavilapalli commented on YARN-2753: --- Thanks [~zxu], this looks good. +1. Checking this in. > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202729#comment-14202729 ] zhihai xu commented on YARN-2753: - The two test failures are not related to my patch. All these two tests are passed at my local latest build. > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[
https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202579#comment-14202579
]
Hadoop QA commented on YARN-2753:
-
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12680231/YARN-2753.008.patch
against trunk revision 2ac1be7.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.TestAllocationFileLoaderService
The following test timeouts occurred in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.TestCapacitySchedulerQueueACLs
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/5785//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5785//console
This message is automatically generated.
> Fix potential issues and code clean up for *NodeLabelsManager
> -
>
> Key: YARN-2753
> URL: https://issues.apache.org/jira/browse/YARN-2753
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: zhihai xu
>Assignee: zhihai xu
> Attachments: YARN-2753.000.patch, YARN-2753.001.patch,
> YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch,
> YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch,
> YARN-2753.008.patch
>
>
> Issues include:
> * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value
> in labelCollections if the key already exists otherwise the Label.resource
> will be changed(reset).
> * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of
> CommonNodeLabelsManager.
> ** because when a Node is created, Node.labels can be null.
> ** In this case, nm.labels; may be null. So we need check originalLabels not
> null before use it(originalLabels.containsAll).
> * addToCluserNodeLabels should be protected by writeLock in
> RMNodeLabelsManager.java. because we should protect labelCollections in
> RMNodeLabelsManager.
> * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is
> invoked, some event may not be processed, see
> [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202517#comment-14202517 ] Wangda Tan commented on YARN-2753: -- +1, thanks for update! > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202491#comment-14202491 ] zhihai xu commented on YARN-2753: - Hi [~leftnoteasy], Good finding, addressed your comment in the new patch YARN-2753.008.patch. thanks zhihai > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch, > YARN-2753.008.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14202440#comment-14202440 ] Wangda Tan commented on YARN-2753: -- [~zxu], One minor comment is, when the newLabels is empty when addToClusterNodeLabel, you shouldn't send it to store. Beside this, LGTM. Wangda > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14201795#comment-14201795 ] zhihai xu commented on YARN-2753: - Cool, good catch. addressed your comment in the new patch YARN-2753.007.patch. [~vinodkv], thanks for the review. > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch, YARN-2753.007.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14201384#comment-14201384 ] Vinod Kumar Vavilapalli commented on YARN-2753: --- Let me look.. > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14201038#comment-14201038 ] zhihai xu commented on YARN-2753: - Hi [~xgong], Could you help review and commit the patch? since Wangda already reviewed the patch. thanks zhihai > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14200973#comment-14200973 ] Wangda Tan commented on YARN-2753: -- +1 for latest patch, thanks for update! [~zxu] > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[
https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14200048#comment-14200048
]
Hadoop QA commented on YARN-2753:
-
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12679811/YARN-2753.006.patch
against trunk revision 80d7d18.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/5752//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5752//console
This message is automatically generated.
> Fix potential issues and code clean up for *NodeLabelsManager
> -
>
> Key: YARN-2753
> URL: https://issues.apache.org/jira/browse/YARN-2753
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: zhihai xu
>Assignee: zhihai xu
> Attachments: YARN-2753.000.patch, YARN-2753.001.patch,
> YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch,
> YARN-2753.005.patch, YARN-2753.006.patch
>
>
> Issues include:
> * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value
> in labelCollections if the key already exists otherwise the Label.resource
> will be changed(reset).
> * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of
> CommonNodeLabelsManager.
> ** because when a Node is created, Node.labels can be null.
> ** In this case, nm.labels; may be null. So we need check originalLabels not
> null before use it(originalLabels.containsAll).
> * addToCluserNodeLabels should be protected by writeLock in
> RMNodeLabelsManager.java. because we should protect labelCollections in
> RMNodeLabelsManager.
> * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is
> invoked, some event may not be processed, see
> [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=1427#comment-1427 ] zhihai xu commented on YARN-2753: - Hi [~leftnoteasy], thanks for the thorough review. item 1). fixed item 2). fixed item 3). fixed item 4). fixed. I agree to you and also the ForwardingEventHandler is only active(registered) when CommonNodeLabelsManager#serviceStart is called, and serviceStart will be only called in STATE.STARTED. I attached a new patch YARN-2753.006.patch which addressed all your comments. thanks zhihai > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch, YARN-2753.006.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * Fix a potential bug in CommonsNodeLabelsManager, after serviceStop(...) is > invoked, some event may not be processed, see > [comment|https://issues.apache.org/jira/browse/YARN-2753?focusedCommentId=14197206&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14197206] -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[
https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14197206#comment-14197206
]
Wangda Tan commented on YARN-2753:
--
Hi Zhihai,
I just review your patch, some comments:
1) Would you please add a comment above
{code}
+ if (this.labelCollections.get(label) == null) {
{code}
To indicate we shouldn't overwrite it
{code}
+ if (labels.isEmpty()) {
{code}
To indicate the labels will never be null
{code}
+ if (originalLabels == null || !originalLabels.containsAll(labels)) {
{code}
To indicate why is it possible originalLabels can be null
which will help with other's review and read the code in the future :)
2) Would you please remove the changes to addToClusterNodeLabels in this patch,
another patch I'm working on (YARN-2800) need to print a warning here, I think
it's better to address the case there since no conflict no matter YARN-2800 or
this JIRA go first.
3) In TestRMNodeLabelsManager,
It's better to change the comment
{code}
+// add label "p1" again, then check whether resource for "p1" is changed.
{code}
To explicitly say to check add labels multiple times shouldn't overwrite
original attributes on labels like resource
4) Would you mind to add one more fix from my side in CommonNodeLabelsManager
Fix is in CommonNodeLabelsManager#ForwardingEventHandler:
Remove {{if (isInState(STATE.STARTED))}} checking, do handleStoreEvent()
directly.
The reason is: in RMAdminCLI, we will create a CommonNodeLabelsManager when we
need access local node labels store directly. And we will call stop() of
CommonNodeLabelsManager when method invoke returned (like calling
addToClusterNodelabels).
What CommonNodeLabelsManager do is async add the actual node labels store event
to a blockingQueue via AsyncDispatcher, and will be finally handled by
CommonNodeLabelsManager#ForwardingEventHandler.
A race condition is, even if we called setDrainEventsOnStop when initialize the
dispatcher, but the state of AbstractService (her wis CommonNodeLabelsManager)
will become “stopped” as soon as we called the stop() of
CommonNodeLabelsManager. So even though AbstractDispatcher can get these event
delivery to ForwardingEventHandler.handle, but it is still possible ignored by
ForwardingEventHandler since the service is not in “started” state any longer.
The behavior of this issue is, sometimes we called methods like
addToClusterNodeLabels, and it successfully returned, but the CLI program exit
before edit log persisted.
I don’t have a good idea about how to test this without too much effort, but I
think this fix should be simple for review.
Thanks,
Wangda
> Fix potential issues and code clean up for *NodeLabelsManager
> -
>
> Key: YARN-2753
> URL: https://issues.apache.org/jira/browse/YARN-2753
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: zhihai xu
>Assignee: zhihai xu
> Attachments: YARN-2753.000.patch, YARN-2753.001.patch,
> YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch,
> YARN-2753.005.patch
>
>
> Issues include:
> * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value
> in labelCollections if the key already exists otherwise the Label.resource
> will be changed(reset).
> * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of
> CommonNodeLabelsManager.
> ** because when a Node is created, Node.labels can be null.
> ** In this case, nm.labels; may be null. So we need check originalLabels not
> null before use it(originalLabels.containsAll).
> * addToCluserNodeLabels should be protected by writeLock in
> RMNodeLabelsManager.java. because we should protect labelCollections in
> RMNodeLabelsManager.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14192724#comment-14192724 ] zhihai xu commented on YARN-2753: - Hi [~leftnoteasy], thanks to review the patch. zhihai > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14192017#comment-14192017 ] Wangda Tan commented on YARN-2753: -- [~zxu], I haven't reviewed the consolidated patch yet, will review it today. Thanks for update! > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14191173#comment-14191173 ] zhihai xu commented on YARN-2753: - Hi [~xgong], since [~leftnoteasy] agreed to my patch(YARN-2756 is discussed separately) and also the patch passed Hadoop QA, Could you review/commit the patch? thanks zhihai > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[
https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14189707#comment-14189707
]
Hadoop QA commented on YARN-2753:
-
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12678125/YARN-2753.005.patch
against trunk revision 0126cf1.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/5637//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5637//console
This message is automatically generated.
> Fix potential issues and code clean up for *NodeLabelsManager
> -
>
> Key: YARN-2753
> URL: https://issues.apache.org/jira/browse/YARN-2753
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: zhihai xu
>Assignee: zhihai xu
> Attachments: YARN-2753.000.patch, YARN-2753.001.patch,
> YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch,
> YARN-2753.005.patch
>
>
> Issues include:
> * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value
> in labelCollections if the key already exists otherwise the Label.resource
> will be changed(reset).
> * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of
> CommonNodeLabelsManager.
> ** because when a Node is created, Node.labels can be null.
> ** In this case, nm.labels; may be null. So we need check originalLabels not
> null before use it(originalLabels.containsAll).
> * addToCluserNodeLabels should be protected by writeLock in
> RMNodeLabelsManager.java. because we should protect labelCollections in
> RMNodeLabelsManager.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[
https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14187718#comment-14187718
]
Hadoop QA commented on YARN-2753:
-
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12677725/YARN-2753.005.patch
against trunk revision 8984e9b.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:
org.apache.hadoop.yarn.server.resourcemanager.scheduler.fair.TestAllocationFileLoaderService
org.apache.hadoop.yarn.server.resourcemanager.security.TestClientToAMTokens
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/5614//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5614//console
This message is automatically generated.
> Fix potential issues and code clean up for *NodeLabelsManager
> -
>
> Key: YARN-2753
> URL: https://issues.apache.org/jira/browse/YARN-2753
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: zhihai xu
>Assignee: zhihai xu
> Attachments: YARN-2753.000.patch, YARN-2753.001.patch,
> YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch,
> YARN-2753.005.patch
>
>
> Issues include:
> * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value
> in labelCollections if the key already exists otherwise the Label.resource
> will be changed(reset).
> * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of
> CommonNodeLabelsManager.
> ** because when a Node is created, Node.labels can be null.
> ** In this case, nm.labels; may be null. So we need check originalLabels not
> null before use it(originalLabels.containsAll).
> * addToCluserNodeLabels should be protected by writeLock in
> RMNodeLabelsManager.java. because we should protect labelCollections in
> RMNodeLabelsManager.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[
https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14187703#comment-14187703
]
Hadoop QA commented on YARN-2753:
-
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12677715/YARN-2753.004.patch
against trunk revision 8984e9b.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 1 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 eclipse:eclipse{color}. The patch built with
eclipse:eclipse.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager.
{color:green}+1 contrib tests{color}. The patch passed contrib unit tests.
Test results:
https://builds.apache.org/job/PreCommit-YARN-Build/5612//testReport/
Console output: https://builds.apache.org/job/PreCommit-YARN-Build/5612//console
This message is automatically generated.
> Fix potential issues and code clean up for *NodeLabelsManager
> -
>
> Key: YARN-2753
> URL: https://issues.apache.org/jira/browse/YARN-2753
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: zhihai xu
>Assignee: zhihai xu
> Attachments: YARN-2753.000.patch, YARN-2753.001.patch,
> YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch,
> YARN-2753.005.patch
>
>
> Issues include:
> * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value
> in labelCollections if the key already exists otherwise the Label.resource
> will be changed(reset).
> * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of
> CommonNodeLabelsManager.
> ** because when a Node is created, Node.labels can be null.
> ** In this case, nm.labels; may be null. So we need check originalLabels not
> null before use it(originalLabels.containsAll).
> * addToCluserNodeLabels should be protected by writeLock in
> RMNodeLabelsManager.java. because we should protect labelCollections in
> RMNodeLabelsManager.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14187619#comment-14187619 ] zhihai xu commented on YARN-2753: - I remove YARN-2756 from this JIRA YARN-2753, so we can discuss issue YARN-2756 separately. The new patch YARN-2753.005.patch will include 3 patches:YARN-2753, YARN-2759 and YARN-2754. Hi [~leftnoteasy] thanks to review the patch. I will move the discussion to YARN-2756. zhihai > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch, > YARN-2753.005.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[
https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14187584#comment-14187584
]
Wangda Tan commented on YARN-2753:
--
[~zxu],
I agree with other fixes but this one:
bq. use static variable (Resources.none()) for not-running Node.resource in
CommonNodeLabelsManager to save memory.
The Resources.none() is used to do checking such as if
(Resources.greaterThan(resource, Resources.none())) { .. do something }. Even
if in nowadays, it will be replaced, but you cannot say in the future, some
guys may write like node.resource.setMemory(...), basically I think it's a bad
style.
That will throw runtime exception and destroy YARN daemons, comparing to memory
it can save, the risk is much more serious, do you agree?
Thanks,
Wangda
> Fix potential issues and code clean up for *NodeLabelsManager
> -
>
> Key: YARN-2753
> URL: https://issues.apache.org/jira/browse/YARN-2753
> Project: Hadoop YARN
> Issue Type: Sub-task
>Reporter: zhihai xu
>Assignee: zhihai xu
> Attachments: YARN-2753.000.patch, YARN-2753.001.patch,
> YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch
>
>
> Issues include:
> * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value
> in labelCollections if the key already exists otherwise the Label.resource
> will be changed(reset).
> * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of
> CommonNodeLabelsManager.
> ** because when a Node is created, Node.labels can be null.
> ** In this case, nm.labels; may be null. So we need check originalLabels not
> null before use it(originalLabels.containsAll).
> * addToCluserNodeLabels should be protected by writeLock in
> RMNodeLabelsManager.java. because we should protect labelCollections in
> RMNodeLabelsManager.
> * use static variable (Resources.none()) for not-running Node.resource in
> CommonNodeLabelsManager to save memory.
> ** When a Node is not activated, the resource is never used, When a Node is
> activated, a new resource will be assigned to it in
> RMNodeLabelsManager#activateNode (nm.resource = resource) So it would be
> better to use static variable Resources.none() instead of allocating a new
> variable(Resource.newInstance(0, 0)) for each node deactivation.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14187579#comment-14187579 ] zhihai xu commented on YARN-2753: - The new patch YARN-2753.004.patch will include 4 patches:YARN-2753, YARN-2759, YARN-2754 and YARN-2756. > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * use static variable (Resources.none()) for not-running Node.resource in > CommonNodeLabelsManager to save memory. > ** When a Node is not activated, the resource is never used, When a Node is > activated, a new resource will be assigned to it in > RMNodeLabelsManager#activateNode (nm.resource = resource) So it would be > better to use static variable Resources.none() instead of allocating a new > variable(Resource.newInstance(0, 0)) for each node deactivation. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14187575#comment-14187575 ] zhihai xu commented on YARN-2753: - Hi [~leftnoteasy], thanks for your suggestion. I also merged YARN-2754 and YARN-2756 to this Jira. zhihai > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch, YARN-2753.004.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). > * addToCluserNodeLabels should be protected by writeLock in > RMNodeLabelsManager.java. because we should protect labelCollections in > RMNodeLabelsManager. > * use static variable (Resources.none()) for not-running Node.resource in > CommonNodeLabelsManager to save memory. > ** When a Node is not activated, the resource is never used, When a Node is > activated, a new resource will be assigned to it in > RMNodeLabelsManager#activateNode (nm.resource = resource) So it would be > better to use static variable Resources.none() instead of allocating a new > variable(Resource.newInstance(0, 0)) for each node deactivation. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (YARN-2753) Fix potential issues and code clean up for *NodeLabelsManager
[ https://issues.apache.org/jira/browse/YARN-2753?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14187114#comment-14187114 ] Wangda Tan commented on YARN-2753: -- [~zxu], I've changed the title and description, I suggest to merge other RMNodeLabelsManager fixes here also. Basically they're same module, and close others as duplicated. Please comment such issues here. Which we can do quick review and commit this easier.. Thanks, Wangda > Fix potential issues and code clean up for *NodeLabelsManager > - > > Key: YARN-2753 > URL: https://issues.apache.org/jira/browse/YARN-2753 > Project: Hadoop YARN > Issue Type: Sub-task >Reporter: zhihai xu >Assignee: zhihai xu > Attachments: YARN-2753.000.patch, YARN-2753.001.patch, > YARN-2753.002.patch, YARN-2753.003.patch > > > Issues include: > * CommonNodeLabelsManager#addToCluserNodeLabels should not change the value > in labelCollections if the key already exists otherwise the Label.resource > will be changed(reset). > * potential NPE(NullPointerException) in checkRemoveLabelsFromNode of > CommonNodeLabelsManager. > ** because when a Node is created, Node.labels can be null. > ** In this case, nm.labels; may be null. So we need check originalLabels not > null before use it(originalLabels.containsAll). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
