Skip to content

HBASE-28223 Include shaded netty-all in hbase-shaded-mapreduce - #5542

Closed
anmolnar wants to merge 1 commit into
apache:masterfrom
anmolnar:HBASE-28223
Closed

HBASE-28223 Include shaded netty-all in hbase-shaded-mapreduce#5542
anmolnar wants to merge 1 commit into
apache:masterfrom
anmolnar:HBASE-28223

Conversation

@anmolnar

Copy link
Copy Markdown
Contributor

Move netty-all to compile scope in hbase-shaded-mapreduce, because it's a required runtime dependency for MR clients which use TLS connections for HBase and ZK.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 25sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 33smaster passed
+1 💚compile0m 10smaster passed
+1 💚shadedjars5m 4sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 10smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 13sthe patch passed
+1 💚compile0m 11sthe patch passed
+1 💚javac0m 11sthe patch passed
+1 💚shadedjars5m 1spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 8sthe patch passed
_ Other Tests _
+1 💚unit0m 11shbase-shaded-mapreduce in the patch passed.
17m 1s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#5542
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 5732ef221c21 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / dbfb516
Default JavaTemurin-1.8.0_352-b08
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/testReport/
Max. process+thread count67 (vs. ulimit of 30000)
modulesC: hbase-shaded/hbase-shaded-mapreduce U: hbase-shaded/hbase-shaded-mapreduce
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/console
versionsgit=2.34.1 maven=3.8.6
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec3m 17sDocker mode activated.
-0 ⚠️yetus0m 4sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall2m 55smaster passed
+1 💚compile0m 14smaster passed
+1 💚shadedjars4m 57sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 12smaster passed
_ Patch Compile Tests _
+1 💚mvninstall2m 42sthe patch passed
+1 💚compile0m 14sthe patch passed
+1 💚javac0m 14sthe patch passed
+1 💚shadedjars4m 52spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 10sthe patch passed
_ Other Tests _
+1 💚unit0m 14shbase-shaded-mapreduce in the patch passed.
20m 44s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#5542
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 7e7d9e3711b1 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / dbfb516
Default JavaEclipse Adoptium-11.0.17+8
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/testReport/
Max. process+thread count72 (vs. ulimit of 30000)
modulesC: hbase-shaded/hbase-shaded-mapreduce U: hbase-shaded/hbase-shaded-mapreduce
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/console
versionsgit=2.34.1 maven=3.8.6
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 11sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+1 💚mvninstall2m 54smaster passed
+1 💚compile0m 14smaster passed
+1 💚spotless0m 43sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚mvninstall2m 42sthe patch passed
+1 💚compile0m 14sthe patch passed
+1 💚javac0m 14sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚xml0m 1sThe patch has no ill-formed XML file.
+1 💚hadoopcheck9m 25sPatch does not cause any errors with Hadoop 3.2.4 3.3.6.
+1 💚spotless0m 41spatch has no errors when running spotless:check.
_ Other Tests _
+1 💚asflicense0m 10sThe patch does not generate ASF License warnings.
23m 25s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#5542
Optional Testsdupname asflicense javac hadoopcheck spotless xml compile
unameLinux 2bcf5fa4384b 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / dbfb516
Default JavaEclipse Adoptium-11.0.17+8
Max. process+thread count79 (vs. ulimit of 30000)
modulesC: hbase-shaded/hbase-shaded-mapreduce U: hbase-shaded/hbase-shaded-mapreduce
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5542/1/console
versionsgit=2.34.1 maven=3.8.6
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@stoty

Copy link
Copy Markdown
Contributor

Shouldn't netty-all be coming from the Hadoop classpath ?

@stoty

Copy link
Copy Markdown
Contributor

hbase-shaded-client is meant to include Hadoop, but hbase-shaded-mapreduce is not.

@stoty

Copy link
Copy Markdown
Contributor

As discussed offline netty-all is a direct dependency of ZK, so I think this is fine after all.
It would be nice if we wouldn't have to add it explicitly, and instead could take it transitively from zookeeper.

@stoty

Copy link
Copy Markdown
Contributor

-1
We don't need netty-all, only netty-handler.
This problem is caused by overriding the netty4 version, and HBASE-28153 already solves it in a more appropriate way.

@Apache9

Copy link
Copy Markdown
Contributor

Hbase-mapreduce module does not transitively depend on netty-all? Strange...

@stoty

Copy link
Copy Markdown
Contributor

hbase-mapreduce does.
But it is transitive via Hadoop, and we mark Hadoop dependencies as provided in the hbase-mapreduce-shaded uberjar, and only add netty artifacts coming via ZK.
hbase-mapreduce-shaded is like hbase-client-byo-hadoop in this regard, and expects to have Hadoop on the classpath.

@anmolnar
anmolnar marked this pull request as draft November 29, 2023 12:10
@anmolnar

Copy link
Copy Markdown
ContributorAuthor

Thanks @stoty . Converting it to draft, because we're verifying a downstream-only fix.

@anmolnar

anmolnar commented Dec 4, 2023

Copy link
Copy Markdown
ContributorAuthor

Resolved downstream. No need for this fix. Actually the upstream version of hbase does include io.netty in hbase-shaded-mapreduce. Thanks @stoty@Apache9

@anmolnaranmolnar closed this Dec 4, 2023
@anmolnar
anmolnar deleted the HBASE-28223 branch December 4, 2023 12:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@anmolnar@Apache-HBase@stoty@Apache9