[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
