TEZ-4715: Migrate Tez PB Protocols from ProtobufRPCEngine to ProtobufRPCEngine2#493
Open
Aggarwal-Raghav wants to merge 2 commits intoapache:masterfrom
Open
TEZ-4715: Migrate Tez PB Protocols from ProtobufRPCEngine to ProtobufRPCEngine2#493Aggarwal-Raghav wants to merge 2 commits intoapache:masterfrom
Aggarwal-Raghav wants to merge 2 commits intoapache:masterfrom
Conversation
|
(!) A patch to the testing environment has been detected. |
Aggarwal-Raghav
force-pushed
the
TEZ-4715
branch
from
580e597 to
b459b5d
Compare
Contributor
Author
|
Jenkins is not updating the final status 😔 |
Contributor
Author
|
The UT failure debugging in #483 (comment) requires hadoop thirdparty making this PR dependent. TEZ-4715 + TEZ-4708 will completely migrate tez from |
Member
|
@abstractdog From the code point of view, this seems right, We had to just use the RPCEngine2 & migrate to the Hadoop Thirdparty. That is rightly done here. But I think this would be an incompatible change, beyond this point I believe Hive master won't build nor work with Tez master. Hive has to do the same activity when moving to Tez-1.0.0 & that would be incompatible for Hive as well & believe the older Hadoop versions won't work post this activity in Hive or Tez, unless there was something done later to fix it, which I don't think... There were compat issues that is why I believe HADOOP-17046 was done later to make sure older client can work with them. Means protobuf is indeed a very known heavy Incompat change, a lot of discussion at HADOOP-13363, in case interested. These are the tradeoffs, I just wanted to highlight. Rest all cool |
Contributor
Author
|
@abstractdog @ayushtkn , can you help on the CI for this? |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
I verified that the JAVA_HOME exists in the docker image but it the jenkins the JAVA_HOME is explicitly set: |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
Aggarwal-Raghav
added a commit
to Aggarwal-Raghav/tez
that referenced
this pull request
May 10, 2026
…erfile (apache#493) - Resolve JAVA_HOME mismatch by creating a symlink from the host's expected path (/usr/lib/jvm/java-XX-openjdk-amd64) to the container's internal path (/opt/java/openjdk). - Parameterize JAVA_VERSION using a build argument to simplify future JDK upgrades. - Add descriptive comments regarding environment variable propagation from Yetus. - Fix ShellCheck SC2086 by adding quotes to the symlink path.
Contributor
Author
|
@abstractdog , can you please remove the JAVA_HOME env varaible setting in docke run command from Jenkins? Let me know if i need to revert #494. CI for tez is not a good experiece at all, this issue should have been caught in #494 itself. Moreover, not sure why mvninstall is failing in run 4. This diff patch running is not helping as it is trying to dowload the SNAPSHOT tez modules jars (i believe). |
Contributor
this is very weird, I guess your Dockerfile change didn't trigger a precommit test your improvement in #494 was great, so I would not revert it if we can fix the problem in 1-2 days (considering the project's pace, it's fine) |
Contributor
Author
|
OK, pipeline seems to be progressing now. I have updated the JAVA_HOME in Jenkinsfile to match the eclipse temurin JAVA_HOME. Internally we use cloudbees Jenkins and some additional tools that passes the env variables, so I thought that JAVA_HOME was coming from Jenkins UI but it was not the case. That's why I asked:
If the run#8 works then I'll raise an addendum PR for TEZ-4717 incorporating Jenkins file change in it. I don't see any other place in project where it need updation for JAVA_HOME 
|
|
💔 -1 overall
This message was automatically generated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
com.google.protobufto shadedorg.apache.hadoop.thirdparty.protobuf