TEZ-4715: Migrate Tez PB Protocols from ProtobufRPCEngine to ProtobufRPCEngine2#493
TEZ-4715: Migrate Tez PB Protocols from ProtobufRPCEngine to ProtobufRPCEngine2#493Aggarwal-Raghav wants to merge 1 commit into
Conversation
|
(!) A patch to the testing environment has been detected. |
Aggarwal-Raghav
force-pushed
the
TEZ-4715
branch
from
580e597 to
b459b5d
Compare
|
Jenkins is not updating the final status 😔 |
|
The UT failure debugging in #483 (comment) requires hadoop thirdparty making this PR dependent. TEZ-4715 + TEZ-4708 will completely migrate tez from |
|
@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 |
|
@abstractdog @ayushtkn , can you help on the CI for this? |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
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. |
This comment was marked as outdated.
This comment was marked as outdated.
|
(!) A patch to the testing environment has been detected. |
This comment was marked as outdated.
This comment was marked as outdated.
…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.
|
@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). |
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) |
|
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 
|
This comment was marked as outdated.
This comment was marked as outdated.
|
💔 -1 overall
This message was automatically generated. |
|
This is ready for review @ayushtkn @abstractdog , the spotbugs are unrelated and |
4 participants
com.google.protobufto shadedorg.apache.hadoop.thirdparty.protobuf