[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17828448#comment-17828448 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-2007703551 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 44s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 1s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. | _ trunk Compile Tests _ | | +0 :ok: | mvndep | 14m 22s | | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 36m 21s | | trunk passed | | +1 :green_heart: | compile | 19m 1s | | trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 | | +1 :green_heart: | compile | 17m 30s | | trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06 | | +1 :green_heart: | checkstyle | 4m 42s | | trunk passed | | +1 :green_heart: | mvnsite | 2m 53s | | trunk passed | | +1 :green_heart: | javadoc | 2m 17s | | trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 | | +1 :green_heart: | javadoc | 1m 48s | | trunk passed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06 | | -1 :x: | spotbugs | 2m 34s | [/branch-spotbugs-hadoop-common-project_hadoop-common-warnings.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/16/artifact/out/branch-spotbugs-hadoop-common-project_hadoop-common-warnings.html) | hadoop-common-project/hadoop-common in trunk has 1 extant spotbugs warnings. | | +1 :green_heart: | shadedclient | 41m 12s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 30s | | Maven dependency ordering for patch | | -1 :x: | mvninstall | 0m 32s | [/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/16/artifact/out/patch-mvninstall-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt) | hadoop-yarn-server-resourcemanager in the patch failed. | | -1 :x: | compile | 8m 13s | [/patch-compile-root-jdkUbuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/16/artifact/out/patch-compile-root-jdkUbuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1.txt) | root in the patch failed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1. | | -1 :x: | javac | 8m 13s | [/patch-compile-root-jdkUbuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/16/artifact/out/patch-compile-root-jdkUbuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1.txt) | root in the patch failed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1. | | -1 :x: | compile | 7m 37s | [/patch-compile-root-jdkPrivateBuild-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/16/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06.txt) | root in the patch failed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06. | | -1 :x: | javac | 7m 37s | [/patch-compile-root-jdkPrivateBuild-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/16/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06.txt) | root in the patch failed with JDK Private Build-1.8.0_402-8u402-ga-2ubuntu1~20.04-b06. | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | -0 :warning: | checkstyle | 4m 20s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/16/artifact/out/results-checkstyle-root.txt) | root: The patch generated 6 new + 139 unchanged - 0 fixed = 145 total (was 139) | | -1 :x: | mvnsite | 0m 37s |
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17774052#comment-17774052 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1757622191 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 47s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 0s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. | _ trunk Compile Tests _ | | +0 :ok: | mvndep | 16m 20s | | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 35m 56s | | trunk passed | | +1 :green_heart: | compile | 18m 14s | | trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04 | | +1 :green_heart: | compile | 16m 37s | | trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05 | | +1 :green_heart: | checkstyle | 4m 38s | | trunk passed | | +1 :green_heart: | mvnsite | 2m 45s | | trunk passed | | +1 :green_heart: | javadoc | 2m 13s | | trunk passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04 | | +1 :green_heart: | javadoc | 1m 46s | | trunk passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05 | | +1 :green_heart: | spotbugs | 4m 40s | | trunk passed | | +1 :green_heart: | shadedclient | 41m 32s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 1m 59s | | the patch passed | | +1 :green_heart: | compile | 18m 37s | | the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04 | | +1 :green_heart: | javac | 18m 37s | | the patch passed | | +1 :green_heart: | compile | 16m 35s | | the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05 | | +1 :green_heart: | javac | 16m 35s | | the patch passed | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | -0 :warning: | checkstyle | 4m 30s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/15/artifact/out/results-checkstyle-root.txt) | root: The patch generated 6 new + 150 unchanged - 0 fixed = 156 total (was 150) | | +1 :green_heart: | mvnsite | 2m 41s | | the patch passed | | +1 :green_heart: | javadoc | 2m 8s | | the patch passed with JDK Ubuntu-11.0.20+8-post-Ubuntu-1ubuntu120.04 | | +1 :green_heart: | javadoc | 1m 46s | | the patch passed with JDK Private Build-1.8.0_382-8u382-ga-1~20.04.1-b05 | | -1 :x: | spotbugs | 2m 44s | [/new-spotbugs-hadoop-common-project_hadoop-common.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/15/artifact/out/new-spotbugs-hadoop-common-project_hadoop-common.html) | hadoop-common-project/hadoop-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0) | | +1 :green_heart: | shadedclient | 40m 52s | | patch has no errors when building and testing our client artifacts. | _ Other Tests _ | | +1 :green_heart: | unit | 19m 4s | | hadoop-common in the patch passed. | | +1 :green_heart: | unit | 104m 41s | | hadoop-yarn-server-resourcemanager in the patch passed. | | +1 :green_heart: | asflicense | 0m 58s | | The patch does not generate ASF License warnings. | | | | 370m 0s | | | | Reason | Tests | |---:|:--| | SpotBugs | module:hadoop-common-project/hadoop-common | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propertiesAddListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4094] | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propertiesRemoveListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4101] | | | org.apache.hadoop.conf.Configuration$PropertiesWithListener doesn't override java.util.Hashtable.equals(Object) At Configuration.java:At Configuration.java:[line 1] | | Subsystem | Report/Notes |
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656665#comment-17656665 ] ASF GitHub Bot commented on YARN-11216: --- szilard-nemeth commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1377399850 Hi @K0K0V0K Added one comment. Otherwise patch LGTM. Please check all the failures reported by Jenkins above. > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656664#comment-17656664 ] ASF GitHub Bot commented on YARN-11216: --- szilard-nemeth commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r1065875469 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java: ## @@ -871,6 +877,15 @@ public Configuration(Configuration other) { setQuietMode(other.getQuietMode()); } + protected synchronized void setPropListeners( + BiConsumer propAddListener, + Consumer propRemoveListener + ) { +this.properties = null; Review Comment: I don't think it's okay to use the assert statement here, see: https://www.baeldung.com/java-avoid-null-check#assertions Objects.requireNonNull is fine I think. > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656243#comment-17656243 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1376128025 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 47s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 0s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. | _ trunk Compile Tests _ | | +0 :ok: | mvndep | 15m 4s | | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 29m 6s | | trunk passed | | +1 :green_heart: | compile | 25m 46s | | trunk passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 | | +1 :green_heart: | compile | 22m 4s | | trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 | | +1 :green_heart: | checkstyle | 4m 12s | | trunk passed | | +1 :green_heart: | mvnsite | 2m 49s | | trunk passed | | -1 :x: | javadoc | 1m 7s | [/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/12/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) | hadoop-common in trunk failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. | | -1 :x: | javadoc | 0m 55s | [/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/12/artifact/out/branch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) | hadoop-yarn-server-resourcemanager in trunk failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. | | +1 :green_heart: | javadoc | 1m 31s | | trunk passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 | | +1 :green_heart: | spotbugs | 4m 52s | | trunk passed | | +1 :green_heart: | shadedclient | 24m 25s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 24s | | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 1m 57s | | the patch passed | | +1 :green_heart: | compile | 24m 24s | | the patch passed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04 | | +1 :green_heart: | javac | 24m 24s | | the patch passed | | +1 :green_heart: | compile | 21m 50s | | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 | | +1 :green_heart: | javac | 21m 50s | | the patch passed | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | -0 :warning: | checkstyle | 3m 54s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/12/artifact/out/results-checkstyle-root.txt) | root: The patch generated 8 new + 150 unchanged - 0 fixed = 158 total (was 150) | | +1 :green_heart: | mvnsite | 2m 44s | | the patch passed | | -1 :x: | javadoc | 1m 0s | [/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/12/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) | hadoop-common in the patch failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. | | -1 :x: | javadoc | 0m 54s | [/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/12/artifact/out/patch-javadoc-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager-jdkUbuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04.txt) | hadoop-yarn-server-resourcemanager in the patch failed with JDK Ubuntu-11.0.17+8-post-Ubuntu-1ubuntu220.04. | | +1 :green_heart: | javadoc | 1m 31s | | the patch passed with JDK Private Build-1.8.0_352-8u352-ga-1~20.04-b08 | |
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656116#comment-17656116 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1375672632 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 0s | | Docker mode activated. | | -1 :x: | patch | 0m 21s | | https://github.com/apache/hadoop/pull/4655 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help. | | Subsystem | Report/Notes | |--:|:-| | GITHUB PR | https://github.com/apache/hadoop/pull/4655 | | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/14/console | | versions | git=2.17.1 | | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org | This message was automatically generated. > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656108#comment-17656108 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1375631201 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 0s | | Docker mode activated. | | -1 :x: | patch | 0m 21s | | https://github.com/apache/hadoop/pull/4655 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help. | | Subsystem | Report/Notes | |--:|:-| | GITHUB PR | https://github.com/apache/hadoop/pull/4655 | | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/13/console | | versions | git=2.17.1 | | Powered by | Apache Yetus 0.14.0 https://yetus.apache.org | This message was automatically generated. > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656106#comment-17656106 ] ASF GitHub Bot commented on YARN-11216: --- K0K0V0K commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r1064638015 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestCapacitySchedulerConfiguration.java: ## @@ -107,6 +110,40 @@ public void testDefaultSubmitACLForRootAllAllowed() { assertTrue(acl.isAllAllowed()); } + /** + * dfs.nfs.exports.allowed.hosts + * prop is deprecated, now we use + * nfs.exports.allowed.hosts + * instead + */ + @Test + public void testDeprecationFeatureWorks() { Review Comment: When we set a deprecated properite, there is some logic under the hood what keeps the old value and add an alias to the properties object, and we have to ensure the alias add also present in the tree. The unset unfortunately not works fine, but i created a ticket for that https://issues.apache.org/jira/browse/YARN-11348 > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17656103#comment-17656103 ] ASF GitHub Bot commented on YARN-11216: --- K0K0V0K commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r1064630413 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java: ## @@ -94,6 +108,42 @@ public Map getPropertiesWithPrefix( return properties; } + /** + * Update or create value in the nodes. + * @param name name of the property + * @param value value of the property + */ + public void set(String name, String value) { +PrefixNode node = getNode(name); Review Comment: if the node is null, than the getNode will create a WARNING message ``` /** * Finds the node that matches the whole key or create it, if it does not exist. * @param name name of the property * @return the found or created node, if the name is empty, than return with null */ private PrefixNode getNode(String name) { List propertyKeyParts = splitPropertyByDelimiter(name); if (!propertyKeyParts.isEmpty()) { return findOrCreatePrefixNode(null, propertyKeyParts.iterator()); } else { LOG.warn("Empty configuration property"); return null; } } ``` ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java: ## @@ -94,6 +108,42 @@ public Map getPropertiesWithPrefix( return properties; } + /** + * Update or create value in the nodes. + * @param name name of the property + * @param value value of the property + */ + public void set(String name, String value) { +PrefixNode node = getNode(name); +if (node != null) { + node.getValues().put(name, value); +} + } + + /** + * Delete value from nodes. + * @param name name of the property + */ + public void unset(String name) { +PrefixNode node = getNode(name); +if (node != null) { Review Comment: if the node is null, than the getNode will create a WARNING message ``` /** * Finds the node that matches the whole key or create it, if it does not exist. * @param name name of the property * @return the found or created node, if the name is empty, than return with null */ private PrefixNode getNode(String name) { List propertyKeyParts = splitPropertyByDelimiter(name); if (!propertyKeyParts.isEmpty()) { return findOrCreatePrefixNode(null, propertyKeyParts.iterator()); } else { LOG.warn("Empty configuration property"); return null; } } ``` > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17636759#comment-17636759 ] ASF GitHub Bot commented on YARN-11216: --- szilard-nemeth commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r1028179146 ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java: ## @@ -242,6 +244,10 @@ public class Configuration implements Iterable>, private boolean restrictSystemProps = restrictSystemPropsDefault; private boolean allowNullValueProperties = false; + private BiConsumer propAddListener; Review Comment: Can we use 'properties' as a name for fields, setters and the class (PropertiesWithListener) without the abbreviation? I think it's not too long and abbreviating is not really necessary in this case. ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java: ## @@ -4060,4 +4077,26 @@ private void putIntoUpdatingResource(String key, String[] value) { } localUR.put(key, value); } + private class PropWithListener extends Properties { + +private final Configuration configuration; + +public PropWithListener(Configuration configuration) { + this.configuration = configuration; +} +@Override Review Comment: Nit: Add newline between constructor and setProperty method. ## hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java: ## @@ -871,6 +877,15 @@ public Configuration(Configuration other) { setQuietMode(other.getQuietMode()); } + protected synchronized void setPropListeners( + BiConsumer propAddListener, + Consumer propRemoveListener + ) { +this.properties = null; Review Comment: Here, you could accept null values for the consumers, at least there's no prevention for them to be null. In getProps, the PropWithListener is created if any of the listeners are not null (so the other one is allowed to be null). Calling setProperty on PropWithListener is not checking if those fields are null, which is dangerous. Either prevent them to be null in the constructor (fail fast) or do a null check in setProperty. Could you please also add a testcase to cover the null scenarios? ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java: ## @@ -55,6 +58,17 @@ public ConfigurationProperties(Map props) { storePropertiesInPrefixNodes(props); } + /** + * A constructor defined in order to conform to the type used by + * {@code Configuration}. It must only be called by String keys and values. Review Comment: ```suggestion * {@code Configuration}. It must only be called with String keys and values. ``` ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java: ## @@ -18,15 +18,27 @@ package org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity; -import org.apache.hadoop.classification.VisibleForTesting; Review Comment: Please exclude formatting (i.e. organize imports) from your commit and only add required changes in imports. ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java: ## @@ -158,39 +208,49 @@ private void copyProperties( */ private void storePropertiesInPrefixNodes(Map props) { for (Map.Entry prop : props.entrySet()) { - List propertyKeyParts = splitPropertyByDelimiter(prop.getKey()); - if (!propertyKeyParts.isEmpty()) { -PrefixNode node = findOrCreatePrefixNode(nodes, -propertyKeyParts.iterator()); + PrefixNode node = getNode(prop.getKey()); + if (node != null) { node.getValues().put(prop.getKey(), prop.getValue()); - } else { -LOG.warn("Empty configuration property, skipping..."); } } } + /** + * Finds the node that matches the whole key or create it, if it does not exist. + * @param name name of the property + * @return the found or created node, if the name is empty, than return with null + */ Review Comment: ```suggestion * Finds the node that matches the whole key or create it if it does not exist. * @param name name of the property * @return the found or newly created node, otherwise return null if the name is empty */ ``` ##
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17631251#comment-17631251 ] ASF GitHub Bot commented on YARN-11216: --- 9uapaw commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1309275394 Thanks for the changes @K0K0V0K, can you deal with the checkstyle issues please? > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17630457#comment-17630457 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1307323876 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 47s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 0s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. | _ trunk Compile Tests _ | | +0 :ok: | mvndep | 15m 33s | | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 25m 41s | | trunk passed | | +1 :green_heart: | compile | 23m 22s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | compile | 20m 57s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | checkstyle | 4m 8s | | trunk passed | | +1 :green_heart: | mvnsite | 3m 29s | | trunk passed | | +1 :green_heart: | javadoc | 2m 45s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 24s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | spotbugs | 5m 40s | | trunk passed | | +1 :green_heart: | shadedclient | 22m 39s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 29s | | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 2m 4s | | the patch passed | | +1 :green_heart: | compile | 22m 38s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javac | 22m 38s | | the patch passed | | +1 :green_heart: | compile | 20m 49s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | javac | 20m 49s | | the patch passed | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | -0 :warning: | checkstyle | 4m 8s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/11/artifact/out/results-checkstyle-root.txt) | root: The patch generated 8 new + 150 unchanged - 0 fixed = 158 total (was 150) | | +1 :green_heart: | mvnsite | 3m 32s | | the patch passed | | +1 :green_heart: | javadoc | 3m 1s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 26s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | -1 :x: | spotbugs | 3m 8s | [/new-spotbugs-hadoop-common-project_hadoop-common.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/11/artifact/out/new-spotbugs-hadoop-common-project_hadoop-common.html) | hadoop-common-project/hadoop-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0) | | +1 :green_heart: | shadedclient | 22m 41s | | patch has no errors when building and testing our client artifacts. | _ Other Tests _ | | +1 :green_heart: | unit | 18m 43s | | hadoop-common in the patch passed. | | +1 :green_heart: | unit | 99m 44s | | hadoop-yarn-server-resourcemanager in the patch passed. | | +1 :green_heart: | asflicense | 1m 24s | | The patch does not generate ASF License warnings. | | | | 337m 42s | | | | Reason | Tests | |---:|:--| | SpotBugs | module:hadoop-common-project/hadoop-common | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propAddListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4091] | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propRemoveListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4098] | | | org.apache.hadoop.conf.Configuration$PropWithListener doesn't override java.util.Hashtable.equals(Object) At Configuration.java:At Configuration.java:[line 1] | | Subsystem | Report/Notes |
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17617293#comment-17617293 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1278136548 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 1m 8s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 1s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. | _ trunk Compile Tests _ | | +0 :ok: | mvndep | 15m 25s | | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 28m 52s | | trunk passed | | +1 :green_heart: | compile | 26m 36s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | compile | 24m 6s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | checkstyle | 4m 33s | | trunk passed | | +1 :green_heart: | mvnsite | 3m 54s | | trunk passed | | +1 :green_heart: | javadoc | 2m 46s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 7s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | spotbugs | 6m 14s | | trunk passed | | +1 :green_heart: | shadedclient | 27m 8s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 28s | | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 2m 9s | | the patch passed | | +1 :green_heart: | compile | 26m 51s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javac | 26m 51s | | the patch passed | | +1 :green_heart: | compile | 24m 43s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | javac | 24m 43s | | the patch passed | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | -0 :warning: | checkstyle | 4m 39s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/9/artifact/out/results-checkstyle-root.txt) | root: The patch generated 5 new + 150 unchanged - 0 fixed = 155 total (was 150) | | +1 :green_heart: | mvnsite | 3m 37s | | the patch passed | | +1 :green_heart: | javadoc | 2m 50s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 14s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | -1 :x: | spotbugs | 3m 9s | [/new-spotbugs-hadoop-common-project_hadoop-common.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/9/artifact/out/new-spotbugs-hadoop-common-project_hadoop-common.html) | hadoop-common-project/hadoop-common generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) | | +1 :green_heart: | shadedclient | 26m 13s | | patch has no errors when building and testing our client artifacts. | _ Other Tests _ | | +1 :green_heart: | unit | 19m 1s | | hadoop-common in the patch passed. | | +1 :green_heart: | unit | 104m 15s | | hadoop-yarn-server-resourcemanager in the patch passed. | | +1 :green_heart: | asflicense | 1m 9s | | The patch does not generate ASF License warnings. | | | | 369m 4s | | | | Reason | Tests | |---:|:--| | SpotBugs | module:hadoop-common-project/hadoop-common | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propAddListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4084] | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propRemoveListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4089] | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/9/artifact/out/Dockerfile | | GITHUB PR |
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17617291#comment-17617291 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1278129187 :broken_heart: **-1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 53s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 0s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. | _ trunk Compile Tests _ | | +0 :ok: | mvndep | 16m 18s | | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 25m 58s | | trunk passed | | +1 :green_heart: | compile | 23m 17s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | compile | 20m 44s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | checkstyle | 4m 11s | | trunk passed | | +1 :green_heart: | mvnsite | 3m 27s | | trunk passed | | +1 :green_heart: | javadoc | 2m 44s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 19s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | spotbugs | 5m 25s | | trunk passed | | +1 :green_heart: | shadedclient | 22m 42s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 30s | | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 1m 59s | | the patch passed | | +1 :green_heart: | compile | 22m 37s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javac | 22m 37s | | the patch passed | | +1 :green_heart: | compile | 21m 0s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | javac | 21m 0s | | the patch passed | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | -0 :warning: | checkstyle | 4m 5s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/10/artifact/out/results-checkstyle-root.txt) | root: The patch generated 7 new + 150 unchanged - 0 fixed = 157 total (was 150) | | +1 :green_heart: | mvnsite | 3m 45s | | the patch passed | | +1 :green_heart: | javadoc | 2m 37s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 18s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | -1 :x: | spotbugs | 2m 58s | [/new-spotbugs-hadoop-common-project_hadoop-common.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/10/artifact/out/new-spotbugs-hadoop-common-project_hadoop-common.html) | hadoop-common-project/hadoop-common generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) | | +1 :green_heart: | shadedclient | 24m 42s | | patch has no errors when building and testing our client artifacts. | _ Other Tests _ | | +1 :green_heart: | unit | 19m 20s | | hadoop-common in the patch passed. | | +1 :green_heart: | unit | 100m 25s | | hadoop-yarn-server-resourcemanager in the patch passed. | | +1 :green_heart: | asflicense | 1m 20s | | The patch does not generate ASF License warnings. | | | | 341m 29s | | | | Reason | Tests | |---:|:--| | SpotBugs | module:hadoop-common-project/hadoop-common | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propAddListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4084] | | | Inconsistent synchronization of org.apache.hadoop.conf.Configuration.propRemoveListener; locked 66% of time Unsynchronized access at Configuration.java:66% of time Unsynchronized access at Configuration.java:[line 4089] | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/10/artifact/out/Dockerfile | | GITHUB PR |
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17617095#comment-17617095 ] ASF GitHub Bot commented on YARN-11216: --- K0K0V0K commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r994719147 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestCapacitySchedulerConfiguration.java: ## @@ -107,6 +110,40 @@ public void testDefaultSubmitACLForRootAllAllowed() { assertTrue(acl.isAllAllowed()); } + /** + * dfs.nfs.exports.allowed.hosts + * prop is deprecated, new we use Review Comment: typo > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17616852#comment-17616852 ] ASF GitHub Bot commented on YARN-11216: --- tomicooler commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r994283630 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/TestConfigurationProperties.java: ## @@ -122,4 +125,58 @@ public void testGetPropertiesWithPrefixEmptyResult() { Assert.assertEquals(0, propsLongPrefix.size()); Assert.assertEquals(0, propsNonExistingRootPrefix.size()); } + + @Test + public void testWhiteListConstructor() { Review Comment: The latest revision looks good to me, but please add unit tests to verify that the properties kept in sync even with deprecation and alternative names. (To verify PropWithListener) > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17616679#comment-17616679 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1276695722 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 0m 48s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 1s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. | _ trunk Compile Tests _ | | +0 :ok: | mvndep | 15m 18s | | Maven dependency ordering for branch | | +1 :green_heart: | mvninstall | 28m 49s | | trunk passed | | +1 :green_heart: | compile | 25m 27s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | compile | 22m 15s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | checkstyle | 4m 27s | | trunk passed | | +1 :green_heart: | mvnsite | 3m 21s | | trunk passed | | +1 :green_heart: | javadoc | 2m 33s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 5s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | spotbugs | 5m 25s | | trunk passed | | +1 :green_heart: | shadedclient | 25m 16s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +0 :ok: | mvndep | 0m 24s | | Maven dependency ordering for patch | | +1 :green_heart: | mvninstall | 2m 8s | | the patch passed | | +1 :green_heart: | compile | 24m 41s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javac | 24m 41s | | the patch passed | | +1 :green_heart: | compile | 24m 22s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | javac | 24m 22s | | the patch passed | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | -0 :warning: | checkstyle | 6m 1s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/8/artifact/out/results-checkstyle-root.txt) | root: The patch generated 2 new + 150 unchanged - 0 fixed = 152 total (was 150) | | +1 :green_heart: | mvnsite | 5m 24s | | the patch passed | | +1 :green_heart: | javadoc | 3m 2s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 2m 50s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | spotbugs | 5m 57s | | the patch passed | | +1 :green_heart: | shadedclient | 25m 29s | | patch has no errors when building and testing our client artifacts. | _ Other Tests _ | | +1 :green_heart: | unit | 18m 42s | | hadoop-common in the patch passed. | | +1 :green_heart: | unit | 102m 22s | | hadoop-yarn-server-resourcemanager in the patch passed. | | +1 :green_heart: | asflicense | 1m 9s | | The patch does not generate ASF License warnings. | | | | 361m 41s | | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/8/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hadoop/pull/4655 | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets | | uname | Linux ac834aaf35a0 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/bin/hadoop.sh | | git revision | trunk / f77d7e93496c548b3405ca984e184c5b7b50c325 | | Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/8/testReport/ | |
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17616289#comment-17616289 ] ASF GitHub Bot commented on YARN-11216: --- K0K0V0K commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r993197109 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java: ## @@ -1200,7 +1207,24 @@ public ConfigurationProperties getConfigurationProperties() { public void reinitializeConfigurationProperties() { // Props are always Strings, therefore this cast is safe Map props = (Map) getProps(); -configurationProperties = new ConfigurationProperties(props); +configurationProperties = new ConfigurationProperties(props, PREFIX); + } + + @Override + public void set(String name, String value) { +super.set(name, value); +if (configurationProperties != null) { + //The super#get method used cause the super#set contains some logic + configurationProperties.set(super.get(name), value); Review Comment: I think we should keep them in sync, cause if later someone wants to use the deprecation feature here, than a bug can be easily made > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17615111#comment-17615111 ] ASF GitHub Bot commented on YARN-11216: --- tomicooler commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r991274835 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java: ## @@ -1200,7 +1207,24 @@ public ConfigurationProperties getConfigurationProperties() { public void reinitializeConfigurationProperties() { // Props are always Strings, therefore this cast is safe Map props = (Map) getProps(); -configurationProperties = new ConfigurationProperties(props); +configurationProperties = new ConfigurationProperties(props, PREFIX); + } + + @Override + public void set(String name, String value) { +super.set(name, value); +if (configurationProperties != null) { + //The super#get method used cause the super#set contains some logic + configurationProperties.set(super.get(name), value); Review Comment: I don't think that this is enough. The set may create multiple entries in the configuration (deprecation handling logic and alternative names), the get() just returns one (last?). (The documentation lies, it says "first"): ``` /** * Get the value of the name. If the key is deprecated, * it returns the value of the first key which replaces the deprecated key * and is not null. * If no such property exists, * then defaultValue is returned. * * @param name property name, will be trimmed before get value. * @param defaultValue default value. * @return property value, or defaultValue if the property * doesn't exist. */ public String get(String name, String defaultValue) { String[] names = handleDeprecation(deprecationContext.get(), name); String result = null; for(String n : names) { result = substituteVars(getProps().getProperty(n, defaultValue)); } return result; } ``` The Configuration class is marked as stable API, so it's hard to change these methods. I'm not sure if there is any way to know which configs were set during this call other than copying the object then comparing it the the altered. Need to investigate if this is even feasible (keeping everything in sync). > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17614475#comment-17614475 ] ASF GitHub Bot commented on YARN-11216: --- hadoop-yetus commented on PR #4655: URL: https://github.com/apache/hadoop/pull/4655#issuecomment-1272305412 :confetti_ball: **+1 overall** | Vote | Subsystem | Runtime | Logfile | Comment | |::|--:|:|::|:---:| | +0 :ok: | reexec | 1m 31s | | Docker mode activated. | _ Prechecks _ | | +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. | | +0 :ok: | codespell | 0m 0s | | codespell was not available. | | +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. | | +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. | | +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. | _ trunk Compile Tests _ | | +1 :green_heart: | mvninstall | 45m 6s | | trunk passed | | +1 :green_heart: | compile | 1m 19s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | compile | 1m 11s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | checkstyle | 1m 2s | | trunk passed | | +1 :green_heart: | mvnsite | 1m 9s | | trunk passed | | +1 :green_heart: | javadoc | 1m 4s | | trunk passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 0m 51s | | trunk passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | spotbugs | 2m 17s | | trunk passed | | +1 :green_heart: | shadedclient | 25m 18s | | branch has no errors when building and testing our client artifacts. | _ Patch Compile Tests _ | | +1 :green_heart: | mvninstall | 0m 57s | | the patch passed | | +1 :green_heart: | compile | 1m 3s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javac | 1m 3s | | the patch passed | | +1 :green_heart: | compile | 0m 54s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | javac | 0m 54s | | the patch passed | | +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. | | +1 :green_heart: | checkstyle | 0m 45s | | the patch passed | | +1 :green_heart: | mvnsite | 0m 59s | | the patch passed | | +1 :green_heart: | javadoc | 0m 45s | | the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 | | +1 :green_heart: | javadoc | 0m 40s | | the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | +1 :green_heart: | spotbugs | 2m 6s | | the patch passed | | +1 :green_heart: | shadedclient | 24m 27s | | patch has no errors when building and testing our client artifacts. | _ Other Tests _ | | +1 :green_heart: | unit | 106m 17s | | hadoop-yarn-server-resourcemanager in the patch passed. | | +1 :green_heart: | asflicense | 0m 37s | | The patch does not generate ASF License warnings. | | | | 219m 21s | | | | Subsystem | Report/Notes | |--:|:-| | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/7/artifact/out/Dockerfile | | GITHUB PR | https://github.com/apache/hadoop/pull/4655 | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets | | uname | Linux 5817927f14d3 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | dev-support/bin/hadoop.sh | | git revision | trunk / 573dd3f965623b027a4c9ace981ea85d2357d225 | | Default Java | Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07 | | Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/7/testReport/ | | Max. process+thread count | 955 (vs. ulimit of 5500) | | modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager | | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4655/7/console | | versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 | | Powered by | Apache Yetus 0.14.0
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17613195#comment-17613195 ] ASF GitHub Bot commented on YARN-11216: --- K0K0V0K commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r985932320 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerConfiguration.java: ## @@ -1200,7 +1207,23 @@ public ConfigurationProperties getConfigurationProperties() { public void reinitializeConfigurationProperties() { // Props are always Strings, therefore this cast is safe Map props = (Map) getProps(); -configurationProperties = new ConfigurationProperties(props); +configurationProperties = new ConfigurationProperties(props, PREFIX); + } + + @Override + public void set(String name, String value) { +super.set(name, value); +if (configurationProperties != null) { Review Comment: The getter will initialise the tree if it is not present in the object, based on the configuration. If we set values on this object but wont use the tree view of the data, then the tree building will be wasted. Why we should use the getter here? > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-11216) Avoid unnecessary reconstruction of ConfigurationProperties
[ https://issues.apache.org/jira/browse/YARN-11216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17613191#comment-17613191 ] ASF GitHub Bot commented on YARN-11216: --- K0K0V0K commented on code in PR #4655: URL: https://github.com/apache/hadoop/pull/4655#discussion_r985825269 ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java: ## @@ -55,6 +59,17 @@ public ConfigurationProperties(Map props) { storePropertiesInPrefixNodes(props); } + /** + * A constructor defined in order to conform to the type used by + * {@code Configuration}. It must only be called by String keys and values. + * @param props properties to store + * @param whiteListPrefix only those properties will be in the nodes + *which starts with one of the provided prefixes. + */ + public ConfigurationProperties(Map props, String... whiteListPrefix) { +this(Maps.filterKeys(props, key -> StringUtils.startsWithAny(key, whiteListPrefix))); Review Comment: Thanks for the review @9uapaw and sorry for the late response ## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ConfigurationProperties.java: ## @@ -55,6 +59,17 @@ public ConfigurationProperties(Map props) { storePropertiesInPrefixNodes(props); } + /** + * A constructor defined in order to conform to the type used by + * {@code Configuration}. It must only be called by String keys and values. + * @param props properties to store + * @param whiteListPrefix only those properties will be in the nodes + *which starts with one of the provided prefixes. + */ + public ConfigurationProperties(Map props, String... whiteListPrefix) { +this(Maps.filterKeys(props, key -> StringUtils.startsWithAny(key, whiteListPrefix))); Review Comment: Thanks for the review @9uapaw and sorry for the late response I dont think the lambda has performance issue here. I created a small perf test to prove it, and based on it the vanilia solution is way slower than guava. I suggest the Hashmap.put() method requires some mem allocation time when the map reach the limit. [benchmark_code.txt](https://github.com/apache/hadoop/files/9698166/benchmark_code.txt) [benchmark_log.txt](https://github.com/apache/hadoop/files/9698167/benchmark_log.txt) > Avoid unnecessary reconstruction of ConfigurationProperties > --- > > Key: YARN-11216 > URL: https://issues.apache.org/jira/browse/YARN-11216 > Project: Hadoop YARN > Issue Type: Improvement > Components: capacity scheduler >Reporter: András Győri >Assignee: Bence Kosztolnik >Priority: Major > Labels: pull-request-available > Time Spent: 1h 10m > Remaining Estimate: 0h > > ConfigurationProperties is expensive to create, however, due to its immutable > nature it is possible to copy it/share it between configuration objects (eg. > create a copy constructor). -- This message was sent by Atlassian Jira (v8.20.10#820010) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org