Skip to content

HDDS-12462. Use exclude rules for defining shaded filesystem jar contents #8008

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.

Sign up for GitHub

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

Merged
szetszwo merged 12 commits into from
Mar 21, 2025
Merged

HDDS-12462. Use exclude rules for defining shaded filesystem jar contents #8008

Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0fde989
HDDS-12462. Shade third dependency for ozone client release package
xichen01 Mar 4, 2025
1eadebf
remove org.hadoop.ozone from org exclude
xichen01 Mar 5, 2025
696ecb3
make shade package include org.xerial.snappy
xichen01 Mar 5, 2025
520031e
Shade all the class which JVM or Hadoop can provide
xichen01 Mar 7, 2025
1e45f38
Fix test
xichen01 Mar 7, 2025
efe5471
Fix test
xichen01 Mar 7, 2025
892df45
Fix test
xichen01 Mar 8, 2025
90361b0
shade netty native libraries
xichen01 Mar 14, 2025
7111c6e
Merge branch 'master' into HDDS-12462
xichen01 Mar 14, 2025
b5d92f9
define plugin version in pluginManagement
xichen01 Mar 19, 2025
5c46400
Merge branch 'master' into HDDS-12462
xichen01 Mar 19, 2025
dc217c2
remove hadoop form shaded prefix
xichen01 Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 52 additions & 28 deletions hadoop-ozone/ozonefs-shaded/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,21 @@
<phase>package</phase>
<configuration>
<skip>${maven.shade.skip}</skip>
Copy link
Contributor Author

@xichen01 xichen01 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adoroszlai @ChenSammi
I have an idea
https://github.com/apache/hadoop/blob/d552bb056c3daec8d540f166c41b463e8ef18645/hadoop-client-modules/hadoop-client-api/pom.xml#L131
For these classes that are not shaded in hadoop, should we choose to have Ozone not include these "hadoop client unshaded classes" because this may cause class conflicts? (exclude them in artifactSet and relocation)

We should have two options for Ozone dependencies

  1. Shade dependencies
  2. Do not shade dependencies, and do not include this dependency in the ozone release package

In this way, the ozone release package will only contain shaded dependencies, and for these unshaded dependencies, Ozone will assume that Hadoop env provides these dependencies.

How do you think ?

Copy link
Contributor

@adoroszlai adoroszlai Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. Hadoop is also not included in the fat jar, so it cannot work without Hadoop anyway.

Copy link
Contributor Author

@xichen01 xichen01 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a user directly depend on Ozone and use Ozone's Filesystem without introducing Hadoop's package, can the current Ozone fat-jar provide all necessary dependencies?

<artifactSet>
<excludes>
<!-- Leave snappy that includes native methods which cannot be relocated. -->
<exclude>org.xerial.snappy:*</exclude>
Copy link
Contributor

@adoroszlai adoroszlai Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Classes for org.xerial.snappy are missing from the shaded jars. Is that intentional? Reading the comment I thought it was just not relocated.

Copy link
Contributor Author

@xichen01 xichen01 Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I refer to this https://github.com/viirya/hadoop/blob/7eb09762f38ac825d8614f6ecc42d041efd087bf/hadoop-client-modules/hadoop-client-api/pom.xml#L120 it said the org.xerial.snappy (JNI method ) should not be shaded and excluded it.

But seem we need add it in dependency, let it as transitive dependency to make the downstream?

How do you think?

Copy link
Contributor

@adoroszlai adoroszlai Mar 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hadoop added it as dependency:

https://github.com/apache/hadoop/blob/7eb09762f38ac825d8614f6ecc42d041efd087bf/hadoop-client-modules/hadoop-client-api/pom.xml#L70-L76

But I think the idea is to include everything in the fat jar. So I think we should skip relocation, just like for bouncycastle, instead of completely excluding it.

Copy link
Contributor

@adoroszlai adoroszlai Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or should we expect it to be provided by Hadoop?

Copy link
Contributor Author

@xichen01 xichen01 Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, let the shaded package include the org.xerial.snappy
This artifactSet.exclude has been removed.

</excludes>
</artifactSet>
<filters>
<filter>
<artifact>*:*</artifact>
<excludes>
<!-- Exclude android (dependency from io.grpc:grpc-core) to reduce unnecessary dependencies and avoid potential conflicts -->
<exclude>android/**</exclude>
</excludes>
</filter>
</filters>
<transformers>
<transformer implementation="org.apache.maven.plugins.shade.resource.DontIncludeResourceTransformer">
<resources>
Expand All @@ -120,38 +135,47 @@
<relocation>
<pattern>org</pattern>
<shadedPattern>${shaded.prefix}.org</shadedPattern>
<includes>
<include>org.yaml.**.*</include>
<include>org.sqlite.**.*</include>
<include>org.tukaani.**.*</include>
<include>org.bouncycastle.**.*</include>
<include>org.rocksdb.**.*</include>
<include>org.apache.commons.cli.**.*</include>
<include>org.apache.commons.compress.**.*</include>
<include>org.apache.commons.codec.**.*</include>
<include>org.apache.commons.beanutils.**.*</include>
<include>org.apache.commons.collections.**.*</include>
<include>org.apache.commons.digester.**.*</include>
<include>org.apache.commons.io.**.*</include>
<include>org.apache.commons.logging.**.*</include>
<include>org.apache.commons.validator.**.*</include>
<include>org.apache.commons.lang3.**.*</include>
<include>org.sqlite.**.*</include>
<include>org.apache.thrift.**.*</include>
</includes>
<excludes>
<exclude>org.apache.commons.logging.**</exclude>
Copy link
Contributor

@ChenSammi ChenSammi Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

org.apache.commons.logging.**
org.bouncycastle.**

These two are included to shade currently, but excluded in the new change. Is there any impact if not shaded?

Copy link
Contributor Author

@xichen01 xichen01 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

org.apache.commons.logging.** this can be removed from the exclude list, i will remove it.

Copy link
Contributor Author

@xichen01 xichen01 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

org.bouncycastle.** I am considering whether we should exclude this package from Ozone client release package.

<exclude>org.apache.hadoop.**</exclude>
<exclude>org.apache.log4j.**</exclude>
<exclude>org.apache.ozone.**</exclude>
<exclude>org.apache.ratis.**</exclude>
<exclude>org.bouncycastle.**</exclude>
<exclude>org.ietf.jgss.*</exclude>
<exclude>org.junit.**</exclude>
<exclude>org.omg.**</exclude>
<exclude>org.slf4j.**</exclude>
<exclude>org.w3c.dom.**</exclude>
<exclude>org.xerial.snappy.**</exclude>
<exclude>org.xml.sax.**</exclude>
<exclude>**.pom.xml</exclude>
</excludes>
</relocation>
<relocation>
<pattern>com</pattern>
<shadedPattern>${shaded.prefix}.com</shadedPattern>
<includes>
<include>com.google.common.**.*</include>
<include>com.google.gson.**.*</include>
<include>com.codahale.**.*</include>
<include>com.fasterxml.**.*</include>
<include>com.lmax.**.*</include>
<include>com.github.joshelser.**.*</include>
<include>com.twitter.**.*</include>
</includes>
<excludes>
<exclude>com.google.protobuf.**</exclude>
<exclude>com.sun.javadoc.**</exclude>
<exclude>com.sun.jndi.**</exclude>
<exclude>com.sun.management.**</exclude>
<exclude>com.sun.security.**</exclude>
<exclude>com.sun.tools.**</exclude>
<exclude>**.pom.xml</exclude>
</excludes>
</relocation>
<relocation>
<pattern>google</pattern>
<shadedPattern>${shaded.prefix}.google</shadedPattern>
</relocation>
<relocation>
<pattern>net.jcip</pattern>
<shadedPattern>${shaded.prefix}.net.jcip</shadedPattern>
</relocation>
<relocation>
<pattern>javassist</pattern>
<shadedPattern>${shaded.prefix}.javassist</shadedPattern>
</relocation>
<relocation>
<pattern>kotlin</pattern>
Expand Down
Loading