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

[xichen01]

What changes were proposed in this pull request?

shade third dependency for ozone client release package.

refer to
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.

This can effectively reduce dependency conflicts with other projects. Ozone shades all its own dependencies (excluding native provide dependencies).

Before

[~/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$ ls -lh ../ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT.jar
-rw-r--r-- 1 root root 71M Mar  3 17:27 ../ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT.jar
[~/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$ jar xvf ../ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT.jar
[~/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$

[root@rocky9 ~/ozone-2.0.0-SNAPSHOT/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$ tree -d -L 2
.
├── android
│   └── annotation
├── com
│   ├── google
│   └── nimbusds
├── google
│   ├── api
//...
├── jakarta
│   └── annotation
├── javassist
│   ├── bytecode
//...
├── javax
│   ├── activation
│   ├── annotation
│   └── xml
├── META-INF
│   ├── license
//...
├── mozilla
├── net
│   └── jcip
└── org
    ├── apache
//...

47 directories

After

[~/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$
ls -lh ../ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT.jar
-rw-r--r-- 1 root root 61M Mar  5 14:55 ../ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT.jar
[~/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$ jar xvf ../ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT.jar
[~/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$ tree -d -L 2
.
.
├── META-INF
│   ├── maven
│   ├── native
│   ├── native-image
│   ├── proguard
│   └── services
└── org
    └── apache

9 directories

[~/share/ozone/lib/ozone-filesystem-hadoop3-client-2.0.0-SNAPSHOT]$ cd org/apache
└── org
    └── apache
        ├── hadoop
        │   ├── fs
        │   ├── hdds
        │   ├── io
        │   ├── ozone
        │   └── shaded
        └── ozone
            ├── annotations
            └── erasurecode

10 directories

What is the link to the Apache JIRA

HDDS-12462

How was this patch tested?

• CI
  https://github.com/xichen01/ozone/actions/runs/13658629724

• Local manual test
  Test with hadoop client, the test passed.

$ env | grep HADOOP
HADOOP_USER_RPCPASSWORD=1
HADOOP_OPTS= -Djava.library.path=/root/hadoop-3.3.4//lib/
HADOOP_CLASSPATH=/root/ozone/hadoop-ozone/dist/target/ozone-2.0.0-SNAPSHOT/share/ozone/lib/ozone-filesystem-hadoop3-*.jar:
HADOOP_CONF_DIR=/root/ozone/hadoop-ozone/dev-support/intellij/
HADOOP_HOME=/root/hadoop-3.3.4/
HADOOP_COMMON_LIB_NATIVE_DIR=/root/hadoop-3.3.4//lib/native/

$ ozone sh bucket  create s3v/bucket1

$ pwd
/root/hadoop-3.3.4/bin

$ ./hadoop jar ../share/hadoop/mapreduce/hadoop-mapreduce-client-jobclient-*-tests.jar nnbench -baseDir ofs://localhost/s3v/bucket1 -maps 10 -reduces 5 -numberOfFiles 500 -operation create_write

[adoroszlai]

Thanks @xichen01 for the patch.

[adoroszlai]

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

[xichen01]

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?

[adoroszlai]

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.

[adoroszlai]

Or should we expect it to be provided by Hadoop?

[xichen01]

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

[adoroszlai]

org.hadoop.ozone is no longer needed after HDDS-12198 (but your branch is 149 commits behind apache/ozone:master).

[xichen01]

Removed the org.hadoop.ozone.

[xichen01]

PS: we have some package name org.apache.ratis.metrics.xxx, should we rename it to org.apache.ozone.ratis.metrics.xxx

[adoroszlai]

we have some package name org.apache.ratis.metrics.xxx

Is it in Ozone? I don't see it.

[xichen01]

this one:

ozone/hadoop-hdds/framework/src/main/java/org/apache/ratis/metrics/dropwizard3/RatisMetricsUtils.java

Line 18 in dd74eee

package org.apache.ratis.metrics.dropwizard3;

and these is a com.google.protobuf

ozone/hadoop-hdds/common/src/main/java/com/google/protobuf/Proto2Utils.java

Line 18 in dd74eee

package com.google.protobuf;

All the others class package name are org.apache.hadoop or org.apache.ozone

[adoroszlai]

Thanks, I missed it. This one is needed for accessing package-private members.

[xichen01]

OK, got it.

[jojochuang]

@adoroszlai we discussed this issue at last week's APAC community sync. This approach is more future proofing.

[adoroszlai]

Thanks @xichen01 for the patch, LGTM.

Let's wait for @ChenSammi and @jojochuang to also take a look.

[ChenSammi]

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?

[xichen01]

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

[xichen01]

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

[xichen01]

@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 ?

[adoroszlai]

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

[xichen01]

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?

[adoroszlai]

Thanks @xichen01 for updating the patch. Looks like test with Hadoop 2 failed:

hadoop-2.10.2-hadoopfs-o3fs :: Test ozone fs with hadoopfs                    
==============================================================================
Test hadoop dfs                                                       | FAIL |
1 != 0

Marked as draft until CI in fork can pass.

[xichen01]

if we shade ratis on the <pattern>org</pattern> , will cause exception for the ratis file in the META-INF

put ratis shade in a Independent unit can fix this issue

The normal ratis file

[adoroszlai]

Please wait for clean CI run in fork before marking as "ready for review". (we'll still be looking at it, but this avoids unnecessary CI runs.)

[xichen01]

To add this is because the so library in META-INF/native/ is not shaded, and NativeLibraryLoader.load loads the name after shade.

java.lang.IllegalArgumentException: Failed to load any of the given libraries: [netty_tcnative_linux_x86_64_fedora, netty_tcnative_linux_x86_64, netty_tcnative_x86_64, netty_tcnative]
///..
        Caused by: java.io.FileNotFoundException: META-INF/native/liborg_apache_hadoop_ozone_shaded_org_apache_ratis_thirdparty_netty_tcnative_linux_x86_64.so
                at org.apache.hadoop.ozone.shaded.org.apache.ratis.thirdparty.io.netty.util.internal.NativeLibraryLoader.load(NativeLibraryLoader.java:186)
//...

Actual file is:

% ls META-INF/native | grep ratis
liborg_apache_ratis_thirdparty_netty_resolver_dns_native_macos_x86_64.jnilib
liborg_apache_ratis_thirdparty_netty_tcnative_linux_aarch_64.so
liborg_apache_ratis_thirdparty_netty_tcnative_linux_x86_64.so
liborg_apache_ratis_thirdparty_netty_tcnative_osx_aarch_64.jnilib
liborg_apache_ratis_thirdparty_netty_tcnative_osx_x86_64.jnilib
liborg_apache_ratis_thirdparty_netty_transport_native_epoll_aarch_64.so
liborg_apache_ratis_thirdparty_netty_transport_native_epoll_x86_64.so
liborg_apache_ratis_thirdparty_netty_transport_native_kqueue_x86_64.jnilib
org_apache_ratis_thirdparty_netty_tcnative_windows_x86_64.dll

@adoroszlai @szetszwo Is there a better solution?

[adoroszlai]

ratis-thirdparty shades Netty, so it is possible to relocate native libraries.

https://github.com/apache/ratis-thirdparty/blob/650acd3ac2f74cc50161822f7a634df35455be4f/misc/pom.xml#L279

[xichen01]

Netty native libraries have been relocated

[xichen01]

Test has fixed: https://github.com/xichen01/ozone/actions/runs/13737643633

[xichen01]

@adoroszlai PTAL

[adoroszlai]

Thanks @xichen01 for updating the patch.

Duplicating all native dependencies and the relocation logic from ratis-thirdparty seems to be cumbersome and fragile. Is it not possible to "further" relocate all native libraries (i.e. rename .../native/libX to .../native/lib${ozone.shaded.native.prefix}X)?

[adoroszlai]

nit: version can be omitted, already defined in pluginManagement in root pom.xml.

[xichen01]

Done.

[adoroszlai]

nit: please define version in pluginManagement in root pom.xml, similar to other plugins:

ozone/pom.xml

Lines 1331 to 1337 in 8489cc8

	<pluginManagement>
	<plugins>
	<plugin>
	<groupId>com.github.ekryd.sortpom</groupId>
	<artifactId>sortpom-maven-plugin</artifactId>
	<version>${sortpom-maven-plugin.version}</version>
	</plugin>

[xichen01]

Done.

[szetszwo]

@xichen01 , thanks for working on this! The change looks good. I comparing the before and the after. Will post the results soon.

[szetszwo]

• before: https://issues.apache.org/jira/secure/attachment/13075555/before_jar.txt
  72M ozone-filesystem-shaded-2.1.0-SNAPSHOT.jar

• after: https://issues.apache.org/jira/secure/attachment/13075556/after_jar.txt
  66M ozone-filesystem-shaded-2.1.0-SNAPSHOT.jar

[szetszwo]

@xichen01 , thanks a lot for working on this! Found the following so far. Please take a look. Will continue ...

Diffs:

• Removed org/xerial/**
• ⚠️ Shaded org/reflections/** but not META-INF/maven/org.reflections/**
• ⚠️ Shaded org/jetbrains/** but not META-INF/maven/org.jetbrains/**
• Shaded org/intellij/**
• ⚠️ Shaded org/apache/ratis/** but not META-INF/maven/org.apache.ratis/**
• ⚠️ Shaded org/codehaus/ but not META-INF/maven/org.codehaus.mojo/
• Shaded org/checkerframework
• Unchanged org/apache/ozone

The ones with ⚠️ seems to be incorrect.

[szetszwo]

How about removing hadoop, i.e. org_apache_ozone_shaded?

[szetszwo]

Let's also remove hadoop here?

[xichen01]

@szetszwo Thanks for your review.

I don't quite understand what this means, could you clarify it?

⚠️ Shaded org/reflections/** but not META-INF/maven/org.reflections/**
⚠️ Shaded org/jetbrains/** but not META-INF/maven/org.jetbrains/**
//...

[szetszwo]

@xichen01 , I generated the files in #8008 (comment) by

• jar tf ozone-filesystem-shaded-2.1.0-SNAPSHOT.jar

⚠️ Shaded org/reflections/** but not META-INF/maven/org.reflections/**

Before this PR, we have

• org/reflections/
• META-INF/maven/org.reflections

After this PR, we have

• org/apache/hadoop/ozone/shaded/org/reflections/
• META-INF/maven/org.reflections

Should we also shade META-INF/maven/org.reflections?

[xichen01]

@szetszwo

Should we also shade META-INF/maven/org.reflections?

I don't think it's necessary. Even in before jar, the content under META-INF/maven is not shaded.

[root@bogon /tmp]% cat before_jar.txt | grep META-INF/maven | grep shade
META-INF/maven/org.apache.ozone/ozone-filesystem-shaded/
META-INF/maven/org.apache.ozone/ozone-filesystem-shaded/pom.properties
META-INF/maven/org.apache.ozone/ozone-filesystem-shaded/pom.xml
[root@bogon /tmp]% 

META-INF/maven seems to be just an archive file, it is not needed for runtime
https://stackoverflow.com/questions/27382259/why-do-i-need-meta-inf-maven-dir-in-my-jar

[szetszwo]

@xichen01 , you are right that the maven artifacts do not need to be shaded. Thanks for checking!

<shaded.prefix>org.apache.hadoop.ozone.shaded</shaded.prefix>

Please see if we could remove "hadoop" from above. If it is not easy to do, we can do it in the future.

[xichen01]

Please see if we could remove "hadoop" from above. If it is not easy to do, we can do it in the future.

Done.

[szetszwo]

+1 the change look good.