You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/05/17 14:23:00 UTC
[GitHub] [hadoop] ZanderXu opened a new pull request, #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
ZanderXu opened a new pull request, #4320:
URL: https://github.com/apache/hadoop/pull/4320
Remove duplicate locks in NetworkTopology
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] jojochuang commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
jojochuang commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1132277684
Looks correct to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ZanderXu commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1130159428
Thanks @Hexiaoqiao for your comment, and I have read [HADOOP-16028](https://issues.apache.org/jira/browse/HADOOP-16028) in detail. If we look at this function without context, it is indeed necessary to add the read lock. But if with the context, the read lock in this function is indeed redundant, and instead will cause this logic to look a little confusing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ZanderXu commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1140132428
Thanks @ayushtkn @jojochuang @Hexiaoqiao for your review and I learned a lot. I will close this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] Hexiaoqiao commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
Hexiaoqiao commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1130021206
Thanks @ZanderXu for your proposal. Please reference HADOOP-16028, it will offer some more information about the read lock.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ayushtkn commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
ayushtkn commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1137686111
Logically from the present code flow this lock is redundant and there is enough discussion on the original jira, this was kept because the method calling was protected and a comment there confirms that it is indeed deliberate
https://issues.apache.org/jira/browse/HADOOP-16028?focusedCommentId=16731433&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16731433
I don't think we can change this access modifier to private for compat reasons and the reason for which it was done then still holds true
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ZanderXu commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
ZanderXu commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1134123379
@Hexiaoqiao Thanks. I think the perspective of this [comment](https://issues.apache.org/jira/browse/HADOOP-16028?focusedCommentId=16731763&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16731763) is just this method, not from the context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1129174359
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 58s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 41m 2s | | trunk passed |
| +1 :green_heart: | compile | 27m 3s | | trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 22m 36s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 37s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 58s | | trunk passed |
| -1 :x: | javadoc | 1m 36s | [/branch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4320/1/artifact/out/branch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) | hadoop-common in trunk failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1. |
| +1 :green_heart: | javadoc | 1m 59s | | trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 3m 3s | | trunk passed |
| +1 :green_heart: | shadedclient | 26m 2s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 6s | | the patch passed |
| +1 :green_heart: | compile | 24m 11s | | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 24m 11s | | the patch passed |
| +1 :green_heart: | compile | 21m 29s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 21m 29s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 1m 24s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 56s | | the patch passed |
| -1 :x: | javadoc | 1m 26s | [/patch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4320/1/artifact/out/patch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt) | hadoop-common in the patch failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1. |
| +1 :green_heart: | javadoc | 1m 58s | | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | spotbugs | 3m 2s | | the patch passed |
| +1 :green_heart: | shadedclient | 25m 57s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 18m 11s | | hadoop-common in the patch passed. |
| +1 :green_heart: | asflicense | 1m 15s | | The patch does not generate ASF License warnings. |
| | | 230m 14s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4320/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/4320 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 14ea81897937 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / bb272376e748af7c84b568526937e7bc5e348941 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4320/1/testReport/ |
| Max. process+thread count | 1246 (vs. ulimit of 5500) |
| modules | C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4320/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] Hexiaoqiao commented on pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
Hexiaoqiao commented on PR #4320:
URL: https://github.com/apache/hadoop/pull/4320#issuecomment-1133907833
@ZanderXu Thanks for your reply. I am +1 on the first sight. But there is [one comment](https://issues.apache.org/jira/browse/HADOOP-16028?focusedCommentId=16731763&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16731763) leaved at [HADOOP-16028](https://issues.apache.org/jira/browse/HADOOP-16028) however I can't recall why add read lock here once more.
cc @goiri @ayushtkn would you mind give another reviews. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] ZanderXu closed pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
Posted by GitBox <gi...@apache.org>.
ZanderXu closed pull request #4320: HADOOP-18236. Remove duplicate locks in NetworkTopology
URL: https://github.com/apache/hadoop/pull/4320
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org