-
Notifications
You must be signed in to change notification settings - Fork 8.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-16557. [pb-upgrade] Upgrade protobuf.version to 3.7.1 #1432
HADOOP-16557. [pb-upgrade] Upgrade protobuf.version to 3.7.1 #1432
Conversation
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
275cc32
to
9220853
Compare
(!) A patch to the testing environment has been detected. |
💔 -1 overall
This message was automatically generated. |
9220853
to
45f331b
Compare
💔 -1 overall
This message was automatically generated. |
All new Javac errors are due to either Generated proto classes are proto dependency itself. Shaded client errors are due to not-using "-Ptest-patch" in the mvn command used to verify the shaded client errors. Used locally and found no errors.
Test "TestLargeBlockReport.testBlockReportExceedsLengthLimit" is related. CodedInputStream's default limit is hardcoded to 2GB from 64MB in 2.5.0. So now excecption thrown before start reading the RPC request itself. |
45f331b
to
433f4f6
Compare
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (non-binding) overall.
From jenkins result:
- There's that last tiny little checkstlye that remained
- Regarding the failed tests:
- TestRouterFaultTolerant.testWriteWithFailedSubcluster is tracked in HDFS-14742
- TestDFSZKFailoverController.testManualFailoverWithDFSHAAdmin is a timeout - I see low risk in it
- TestRestartDfsWithFlush: I don't see an open case for this, but I presume it can be quite similar to HADOOP-16238.
- TestRouterHttpDelegationToken and TestRouterWithSecureStartup is handled in HDFS-14609
- I guess the deprecation warnings should be handled in a follow-up patch as you mentioned in the conversation in the jira.
I'm generally ok with this patch, would be happy to see it committed soon.
...t/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestLargeBlockReport.java
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RpcWritable.java
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java
Show resolved
Hide resolved
...hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/BlockListAsLongs.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanx @vinayakumarb for the work here. I guess you need to update in the Building.txt too the new protobuf version.
Other than that LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Thanks for answers. Sounds like we need to check against dropping compatibility. Should be a quick test I'd say.
Thanks @ayushtkn. updated BUILDING.txt instructions.
Did the quick test, connecting Hadoop-3.2.0 client to Hadoop-3.3.0-SNAPSHOT server with upgraded protobuf. Client was able to communicate successfully to server. |
Thanks @adamantal @saintstack @ayushtkn for reviews. |
Good. Thanks. |
@vinayakumarb - is there a jira etc to track the shadedclient failure and fix? (The missing -Ptest-patch causes protoc-3.7 to not be found which causes the failures?) Also, Thank you for making this change. The protobuf upgrade has been pending for a while. |
I doubled-checked the HDFS fsimage format compatibility. Today's trunk build was able to read a 16GB image written by a 2.8.5 namenode without any problem. |
…1432) HADOOP-16557. [pb-upgrade] Upgrade protobuf.version to 3.7.1. Contributed by Vinayakumar B.
…1432) HADOOP-16557. [pb-upgrade] Upgrade protobuf.version to 3.7.1. Contributed by Vinayakumar B.
No description provided.