HADOOP-17009: Embrace Immutability of Java Collections #1974

belugabehr · 2020-04-23T02:54:24Z

No description provided.

hadoop-yetus · 2020-04-23T05:43:10Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	29m 12s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	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 💚	mvninstall	21m 49s	trunk passed
-1 ❌	compile	17m 59s	root in trunk failed.
+1 💚	checkstyle	0m 49s	trunk passed
+1 💚	mvnsite	1m 23s	trunk passed
+1 💚	shadedclient	18m 12s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 55s	trunk passed
+0 🆗	spotbugs	2m 9s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	2m 5s	trunk passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 52s	the patch passed
-1 ❌	compile	17m 22s	root in the patch failed.
-1 ❌	javac	17m 22s	root in the patch failed.
-0 ⚠️	checkstyle	0m 48s	hadoop-common-project/hadoop-common: The patch generated 4 new + 326 unchanged - 3 fixed = 330 total (was 329)
+1 💚	mvnsite	1m 23s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	15m 30s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 54s	the patch passed
+1 💚	findbugs	2m 15s	the patch passed
		_ Other Tests _
-1 ❌	unit	9m 9s	hadoop-common in the patch passed.
+1 💚	asflicense	0m 46s	The patch does not generate ASF License warnings.
		142m 26s

Reason	Tests
Failed junit tests	hadoop.security.TestRaceWhenRelogin

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/artifact/out/Dockerfile
GITHUB PR	#1974
JIRA Issue	HADOOP-17009
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 7c4575a5fc62 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `5958af4`
Default Java	Private Build-1.8.0_252-8u252-b09-1~16.04-b09
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/artifact/out/branch-compile-root.txt
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/artifact/out/patch-compile-root.txt
javac	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/artifact/out/patch-compile-root.txt
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
unit	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/testReport/
Max. process+thread count	1385 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/1/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2020-04-23T16:20:42Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 8s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	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 💚	mvninstall	22m 14s	trunk passed
-1 ❌	compile	18m 34s	root in trunk failed.
+1 💚	checkstyle	0m 49s	trunk passed
+1 💚	mvnsite	1m 23s	trunk passed
+1 💚	shadedclient	18m 0s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 54s	trunk passed
+0 🆗	spotbugs	2m 9s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	2m 7s	trunk passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 51s	the patch passed
-1 ❌	compile	17m 11s	root in the patch failed.
-1 ❌	javac	17m 11s	root in the patch failed.
-0 ⚠️	checkstyle	0m 48s	hadoop-common-project/hadoop-common: The patch generated 1 new + 326 unchanged - 3 fixed = 327 total (was 329)
+1 💚	mvnsite	1m 25s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	15m 27s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 53s	the patch passed
+1 💚	findbugs	2m 16s	the patch passed
		_ Other Tests _
+1 💚	unit	9m 8s	hadoop-common in the patch passed.
+1 💚	asflicense	0m 45s	The patch does not generate ASF License warnings.
		114m 57s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/2/artifact/out/Dockerfile
GITHUB PR	#1974
JIRA Issue	HADOOP-17009
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 98e69cbf294c 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `459eb2a`
Default Java	Private Build-1.8.0_252-8u252-b09-1~16.04-b09
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/2/artifact/out/branch-compile-root.txt
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/2/artifact/out/patch-compile-root.txt
javac	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/2/artifact/out/patch-compile-root.txt
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/2/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/2/testReport/
Max. process+thread count	2763 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/2/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

...n-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java

hadoop-yetus · 2020-04-23T20:49:08Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 5s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	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 💚	mvninstall	21m 24s	trunk passed
-1 ❌	compile	18m 1s	root in trunk failed.
+1 💚	checkstyle	0m 49s	trunk passed
+1 💚	mvnsite	1m 21s	trunk passed
+1 💚	shadedclient	17m 59s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 55s	trunk passed
+0 🆗	spotbugs	2m 8s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	2m 5s	trunk passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 51s	the patch passed
-1 ❌	compile	17m 21s	root in the patch failed.
-1 ❌	javac	17m 21s	root in the patch failed.
-0 ⚠️	checkstyle	0m 48s	hadoop-common-project/hadoop-common: The patch generated 1 new + 326 unchanged - 3 fixed = 327 total (was 329)
+1 💚	mvnsite	1m 23s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	15m 15s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	0m 56s	the patch passed
+1 💚	findbugs	2m 15s	the patch passed
		_ Other Tests _
+1 💚	unit	9m 10s	hadoop-common in the patch passed.
+1 💚	asflicense	0m 45s	The patch does not generate ASF License warnings.
		113m 29s

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/3/artifact/out/Dockerfile
GITHUB PR	#1974
JIRA Issue	HADOOP-17009
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux eace52f3e6a4 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `459eb2a`
Default Java	Private Build-1.8.0_252-8u252-b09-1~16.04-b09
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/3/artifact/out/branch-compile-root.txt
compile	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/3/artifact/out/patch-compile-root.txt
javac	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/3/artifact/out/patch-compile-root.txt
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/3/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/3/testReport/
Max. process+thread count	3230 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/3/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

goiri · 2020-04-25T17:14:29Z

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/net/NetworkTopology.java

-        return null;
-      }
-      return new ArrayList<Node>(rack.getChildren());
+      return (rack == null) ? Collections.emptyList()


Why can we use unmodifiable here?

Ya, I guess we probably shouldn't. My thinking wsa "

Returning null is usually a bad idea, return Collections.emptyList()

Collections.emptyList() is immutable

To make the method consistent, the list of racks should be immutable too.

I will revert this to return an emptyList (new ArrayList<>(0))

hadoop-yetus · 2020-04-27T15:50:58Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 44s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
-1 ❌	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 💚	mvninstall	19m 55s	trunk passed
+1 💚	compile	17m 18s	trunk passed
+1 💚	checkstyle	1m 1s	trunk passed
+1 💚	mvnsite	1m 30s	trunk passed
+1 💚	shadedclient	16m 53s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	1m 4s	trunk passed
+0 🆗	spotbugs	2m 13s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	2m 11s	trunk passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 52s	the patch passed
+1 💚	compile	16m 31s	the patch passed
+1 💚	javac	16m 31s	the patch passed
-0 ⚠️	checkstyle	0m 59s	hadoop-common-project/hadoop-common: The patch generated 1 new + 326 unchanged - 3 fixed = 327 total (was 329)
+1 💚	mvnsite	1m 28s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	14m 8s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	1m 7s	the patch passed
+1 💚	findbugs	2m 14s	the patch passed
		_ Other Tests _
-1 ❌	unit	9m 17s	hadoop-common in the patch passed.
+1 💚	asflicense	0m 54s	The patch does not generate ASF License warnings.
		109m 20s

Reason	Tests
Failed junit tests	hadoop.fs.ftp.TestFTPFileSystem
	hadoop.metrics2.source.TestJvmMetrics
	hadoop.io.compress.snappy.TestSnappyCompressorDecompressor
	hadoop.security.token.delegation.TestZKDelegationTokenSecretManager
	hadoop.io.compress.TestCompressorDecompressor

Subsystem	Report/Notes
Docker	ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/4/artifact/out/Dockerfile
GITHUB PR	#1974
JIRA Issue	HADOOP-17009
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux fca82cb31ab0 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `18d7dfb`
Default Java	Private Build-1.8.0_252-8u252-b09-1~18.04-b09
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/4/artifact/out/diff-checkstyle-hadoop-common-project_hadoop-common.txt
unit	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/4/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/4/testReport/
Max. process+thread count	1975 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1974/4/console
versions	git=2.17.1 maven=3.6.0 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Cosss7 · 2020-06-10T09:33:28Z

Hi~ @goiri
It looks like you have approved the PR. Why don't you merge it ?

goiri · 2020-06-18T20:58:08Z

I wanted to give others a chance to comment.
@Cosss7 any thoughts?

Cosss7 · 2020-06-19T11:28:43Z

@goiri I'm not commiter but the patch LGTM.
+1

goiri · 2020-06-19T17:25:11Z

@Cosss7 thanks for the review.
Merged.

HADOOP-17009: Embrace Immutability of Java Collections

d7c818d

Fix checkstyle warnings

dbf1e0e

goiri reviewed Apr 23, 2020

View reviewed changes

...n-project/hadoop-common/src/main/java/org/apache/hadoop/security/CompositeGroupsMapping.java Outdated Show resolved Hide resolved

Revert some of the returning immutable collections on public APIs

98e9ebe

goiri reviewed Apr 25, 2020

View reviewed changes

Revert immutability of getDatanodesInRack

36a2219

goiri approved these changes Apr 28, 2020

View reviewed changes

goiri approved these changes Jun 19, 2020

View reviewed changes

goiri merged commit 100ec8e into apache:trunk Jun 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-17009: Embrace Immutability of Java Collections #1974

HADOOP-17009: Embrace Immutability of Java Collections #1974

belugabehr commented Apr 23, 2020

hadoop-yetus commented Apr 23, 2020

hadoop-yetus commented Apr 23, 2020

hadoop-yetus commented Apr 23, 2020

goiri Apr 25, 2020

belugabehr Apr 27, 2020

hadoop-yetus commented Apr 27, 2020

Cosss7 commented Jun 10, 2020

goiri commented Jun 18, 2020

Cosss7 commented Jun 19, 2020

goiri commented Jun 19, 2020

HADOOP-17009: Embrace Immutability of Java Collections #1974

HADOOP-17009: Embrace Immutability of Java Collections #1974

Conversation

belugabehr commented Apr 23, 2020

hadoop-yetus commented Apr 23, 2020

hadoop-yetus commented Apr 23, 2020

hadoop-yetus commented Apr 23, 2020

goiri Apr 25, 2020

Choose a reason for hiding this comment

belugabehr Apr 27, 2020

Choose a reason for hiding this comment

hadoop-yetus commented Apr 27, 2020

Cosss7 commented Jun 10, 2020

goiri commented Jun 18, 2020

Cosss7 commented Jun 19, 2020

goiri commented Jun 19, 2020