HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of Public APIs. #1803

vinayakumarb · 2020-01-13T07:00:15Z

Removed below APIs from Token.java and replaced usages with helper methods moved from hadoop-hdfs-client module

  /**
   * Construct a Token from a TokenProto.
   * @param tokenPB the TokenProto object
   */
  public Token(TokenProto tokenPB){
   ...
  }

  /**
   * Construct a TokenProto from this Token instance.
   * @return a new TokenProto object holding copies of data in this instance
   */
  public TokenProto toTokenProto() {
   ...
  }

… Public APIs

hadoop-yetus · 2020-01-13T09:28:23Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	25m 11s	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 _
+0 🆗	mvndep	1m 10s	Maven dependency ordering for branch
+1 💚	mvninstall	18m 6s	trunk passed
+1 💚	compile	16m 37s	trunk passed
+1 💚	checkstyle	2m 37s	trunk passed
+1 💚	mvnsite	2m 26s	trunk passed
+1 💚	shadedclient	19m 6s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 19s	trunk passed
+0 🆗	spotbugs	2m 24s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 23s	trunk passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 25s	Maven dependency ordering for patch
+1 💚	mvninstall	1m 31s	the patch passed
+1 💚	compile	15m 53s	the patch passed
+1 💚	javac	15m 53s	the patch passed
-0 ⚠️	checkstyle	2m 33s	root: The patch generated 1 new + 49 unchanged - 0 fixed = 50 total (was 49)
+1 💚	mvnsite	2m 27s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedclient	12m 45s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 18s	the patch passed
-1 ❌	findbugs	2m 18s	hadoop-common-project/hadoop-common generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
-1 ❌	findbugs	2m 33s	hadoop-hdfs-project/hadoop-hdfs-client generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
		_ Other Tests _
+1 💚	unit	9m 6s	hadoop-common in the patch passed.
+1 💚	unit	2m 18s	hadoop-hdfs-client in the patch passed.
+1 💚	asflicense	0m 52s	The patch does not generate ASF License warnings.
		147m 3s

Reason	Tests
FindBugs	module:hadoop-common-project/hadoop-common
	Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.ipc.ProtobufHelper.getFixedByteString(String) At ProtobufHelper.java:may not be atomic in org.apache.hadoop.ipc.ProtobufHelper.getFixedByteString(String) At ProtobufHelper.java:[line 91]
	Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.ipc.ProtobufHelper.getFixedByteString(Text) At ProtobufHelper.java:may not be atomic in org.apache.hadoop.ipc.ProtobufHelper.getFixedByteString(Text) At ProtobufHelper.java:[line 77]
FindBugs	module:hadoop-hdfs-project/hadoop-hdfs-client
	Private method org.apache.hadoop.hdfs.protocolPB.PBHelperClient.getFixedByteString(Text) is never called At PBHelperClient.java:called At PBHelperClient.java:[line 236]

Subsystem	Report/Notes
Docker	Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/1/artifact/out/Dockerfile
GITHUB PR	#1803
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux cc80fed1cb0c 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `52b360a`
Default Java	1.8.0_232
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/1/artifact/out/diff-checkstyle-root.txt
findbugs	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/1/artifact/out/new-findbugs-hadoop-common-project_hadoop-common.html
findbugs	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/1/artifact/out/new-findbugs-hadoop-hdfs-project_hadoop-hdfs-client.html
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/1/testReport/
Max. process+thread count	1360 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/1/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

… Public APIs. Fixed findbugs

steveloughran

LGTM; a few minor comments

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufHelper.java

steveloughran · 2020-01-13T11:02:20Z

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufHelper.java

+    return (bytes.length == 0) ? ByteString.EMPTY : ByteString.copyFrom(bytes);
+  }
+
+  public static Token<? extends TokenIdentifier> convert(


I think I'd prefer some explicit description "tokenFromProto", but not that fussy about overloading if you prefer that

Okay, Will change method name.

Updated PR. please check.

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufHelper.java

hadoop-yetus · 2020-01-13T12:20:18Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 23s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	@author	0m 1s	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 _
+0 🆗	mvndep	1m 9s	Maven dependency ordering for branch
+1 💚	mvninstall	21m 55s	trunk passed
+1 💚	compile	19m 11s	trunk passed
+1 💚	checkstyle	2m 48s	trunk passed
+1 💚	mvnsite	2m 17s	trunk passed
+1 💚	shadedclient	19m 47s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 8s	trunk passed
+0 🆗	spotbugs	2m 43s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	5m 11s	trunk passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 28s	Maven dependency ordering for patch
+1 💚	mvninstall	1m 47s	the patch passed
+1 💚	compile	19m 31s	the patch passed
+1 💚	javac	19m 31s	the patch passed
-0 ⚠️	checkstyle	2m 52s	root: The patch generated 1 new + 49 unchanged - 0 fixed = 50 total (was 49)
+1 💚	mvnsite	2m 37s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	xml	0m 3s	The patch has no ill-formed XML file.
+1 💚	shadedclient	13m 46s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 6s	the patch passed
+1 💚	findbugs	4m 58s	the patch passed
		_ Other Tests _
+1 💚	unit	10m 44s	hadoop-common in the patch passed.
+1 💚	unit	2m 12s	hadoop-hdfs-client in the patch passed.
+1 💚	asflicense	0m 46s	The patch does not generate ASF License warnings.
		137m 17s

Subsystem	Report/Notes
Docker	Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/2/artifact/out/Dockerfile
GITHUB PR	#1803
Optional Tests	dupname asflicense xml compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 60162d94a380 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `52b360a`
Default Java	1.8.0_232
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/2/artifact/out/diff-checkstyle-root.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/2/testReport/
Max. process+thread count	1377 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/2/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

… Public APIs. Fixed Review comments

ayushtkn

LGTM(Pending Jenkins)

hadoop-yetus · 2020-01-13T15:17:13Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	2m 6s	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 1s	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 _
+0 🆗	mvndep	1m 33s	Maven dependency ordering for branch
+1 💚	mvninstall	24m 9s	trunk passed
+1 💚	compile	22m 29s	trunk passed
+1 💚	checkstyle	3m 14s	trunk passed
+1 💚	mvnsite	2m 44s	trunk passed
+1 💚	shadedclient	21m 57s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 8s	trunk passed
+0 🆗	spotbugs	2m 26s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 30s	trunk passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 20s	Maven dependency ordering for patch
+1 💚	mvninstall	1m 34s	the patch passed
+1 💚	compile	16m 51s	the patch passed
+1 💚	javac	16m 51s	the patch passed
-0 ⚠️	checkstyle	2m 47s	root: The patch generated 1 new + 49 unchanged - 0 fixed = 50 total (was 49)
+1 💚	mvnsite	2m 18s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	xml	0m 2s	The patch has no ill-formed XML file.
+1 💚	shadedclient	13m 55s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	2m 3s	the patch passed
+1 💚	findbugs	4m 47s	the patch passed
		_ Other Tests _
+1 💚	unit	9m 26s	hadoop-common in the patch passed.
+1 💚	unit	2m 6s	hadoop-hdfs-client in the patch passed.
+1 💚	asflicense	0m 45s	The patch does not generate ASF License warnings.
		140m 40s

Subsystem	Report/Notes
Docker	Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/3/artifact/out/Dockerfile
GITHUB PR	#1803
Optional Tests	dupname asflicense xml compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux bf1d36b4a053 4.15.0-66-generic #75-Ubuntu SMP Tue Oct 1 05:24:09 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `52b360a`
Default Java	1.8.0_232
checkstyle	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/3/artifact/out/diff-checkstyle-root.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/3/testReport/
Max. process+thread count	1347 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/3/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2020-01-14T18:25:16Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	166m 53s	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 _
+0 🆗	mvndep	2m 42s	Maven dependency ordering for branch
+1 💚	mvninstall	46m 6s	trunk passed
+1 💚	compile	42m 4s	trunk passed
+1 💚	checkstyle	3m 52s	trunk passed
+1 💚	mvnsite	3m 11s	trunk passed
+1 💚	shadedclient	27m 37s	branch has no errors when building and testing our client artifacts.
+1 💚	javadoc	3m 24s	trunk passed
+0 🆗	spotbugs	2m 54s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	5m 46s	trunk passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 21s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 3s	the patch passed
+1 💚	compile	29m 28s	the patch passed
+1 💚	javac	29m 27s	the patch passed
+1 💚	checkstyle	4m 14s	the patch passed
+1 💚	mvnsite	2m 46s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	xml	0m 3s	The patch has no ill-formed XML file.
+1 💚	shadedclient	19m 33s	patch has no errors when building and testing our client artifacts.
+1 💚	javadoc	3m 27s	the patch passed
+1 💚	findbugs	6m 36s	the patch passed
		_ Other Tests _
-1 ❌	unit	18m 36s	hadoop-common in the patch failed.
+1 💚	unit	3m 17s	hadoop-hdfs-client in the patch passed.
+1 💚	asflicense	1m 9s	The patch does not generate ASF License warnings.
		393m 23s

Reason	Tests
Failed junit tests	hadoop.metrics2.lib.TestMutableRollingAverages
	hadoop.metrics2.lib.TestMutableMetrics
	hadoop.io.nativeio.TestNativeIO
	hadoop.ha.TestZKFailoverController

Subsystem	Report/Notes
Docker	Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/4/artifact/out/Dockerfile
GITHUB PR	#1803
Optional Tests	dupname asflicense xml compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle
uname	Linux 2606c56d7b0f 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/hadoop.sh
git revision	trunk / `1c51f36`
Default Java	1.8.0_232
unit	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/4/artifact/out/patch-unit-hadoop-common-project_hadoop-common.txt
Test Results	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/4/testReport/
Max. process+thread count	1650 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client U: .
Console output	https://builds.apache.org/job/hadoop-multibranch/job/PR-1803/4/console
versions	git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1
Powered by	Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

vinayakumarb · 2020-01-16T05:47:53Z

Test failures seems to be unrelated. With same changes (except checkstyle fix) previous build was successfull.

steveloughran · 2020-01-16T11:36:02Z

+1

Already reviewed

vinayakumarb · 2020-01-16T18:10:13Z

Thanks @steveloughran @ayushtkn for reviews.

… Public APIs. Contributed by Vinayakumar B. (apache#1803)

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of…

7cdd02e

… Public APIs

vinayakumarb self-assigned this Jan 13, 2020

vinayakumarb requested review from ayushtkn and steveloughran January 13, 2020 07:00

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of…

e5db427

… Public APIs. Fixed findbugs

steveloughran previously requested changes Jan 13, 2020

View reviewed changes

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of…

f8fc1b8

… Public APIs. Fixed Review comments

ayushtkn approved these changes Jan 13, 2020

View reviewed changes

Fixed checkstyle comments

7e24758

vinayakumarb merged commit edbbc03 into apache:trunk Jan 16, 2020

RogPodge pushed a commit to RogPodge/hadoop that referenced this pull request Mar 25, 2020

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of…

7bd7106

… Public APIs. Contributed by Vinayakumar B. (apache#1803)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of Public APIs. #1803

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of Public APIs. #1803

vinayakumarb commented Jan 13, 2020

hadoop-yetus commented Jan 13, 2020

steveloughran left a comment

steveloughran Jan 13, 2020

vinayakumarb Jan 13, 2020

vinayakumarb Jan 13, 2020

hadoop-yetus commented Jan 13, 2020

ayushtkn left a comment

hadoop-yetus commented Jan 13, 2020

hadoop-yetus commented Jan 14, 2020

vinayakumarb commented Jan 16, 2020

steveloughran commented Jan 16, 2020

vinayakumarb commented Jan 16, 2020

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of Public APIs. #1803

HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of Public APIs. #1803

Conversation

vinayakumarb commented Jan 13, 2020

Removed below APIs from Token.java and replaced usages with helper methods moved from hadoop-hdfs-client module

hadoop-yetus commented Jan 13, 2020

steveloughran left a comment

Choose a reason for hiding this comment

steveloughran Jan 13, 2020

Choose a reason for hiding this comment

vinayakumarb Jan 13, 2020

Choose a reason for hiding this comment

vinayakumarb Jan 13, 2020

Choose a reason for hiding this comment

hadoop-yetus commented Jan 13, 2020

ayushtkn left a comment

Choose a reason for hiding this comment

hadoop-yetus commented Jan 13, 2020

hadoop-yetus commented Jan 14, 2020

vinayakumarb commented Jan 16, 2020

steveloughran commented Jan 16, 2020

vinayakumarb commented Jan 16, 2020