cshuo commented on code in PR #19330:
URL: https://github.com/apache/hudi/pull/19330#discussion_r3620738058
##########
packaging/hudi-flink-bundle/pom.xml:
##########
@@ -228,6 +233,88 @@
<pattern>com.uber.m3.</pattern>
<shadedPattern>org.apache.hudi.com.uber.m3.</shadedPattern>
</relocation>
+
+ <!-- hive-exec is a fat jar that embeds these deps
un-relocated; under Flink
+ child-first classloading they shadow the versions
Hudi/Flink need. No-op
+ for the default bundle (classes only present when a
shade-hive profile
Review Comment:
These relocations are not scoped to `flink-bundle-shade-hive*`; they run in
the default bundle as well. The default bundle already contains
`parquet-format-structures` and Airlift classes. In particular, excluding
`org.apache.parquet.format.converter.**` only preserves the converter class
name—the remapper still rewrites referenced `org.apache.parquet.format.*` types
in its method descriptors. For example, canonical
`ParquetMetadataConverter.toParquetMetadata(...)` will return relocated
`org.apache.hudi.org.apache.parquet.format.FileMetaData`, so external code
compiled against parquet-hadoop can hit `NoSuchMethodError` under Flink's
child-first loading. Please scope the Hive-only relocations to the shade-Hive
profiles and verify that the default bundle's Parquet ABI remains unchanged.
##########
packaging/hudi-flink-bundle/pom.xml:
##########
@@ -228,6 +233,88 @@
<pattern>com.uber.m3.</pattern>
<shadedPattern>org.apache.hudi.com.uber.m3.</shadedPattern>
</relocation>
+
+ <!-- hive-exec is a fat jar that embeds these deps
un-relocated; under Flink
+ child-first classloading they shadow the versions
Hudi/Flink need. No-op
+ for the default bundle (classes only present when a
shade-hive profile
+ pulls hive-exec into compile scope). -->
+ <relocation>
+ <pattern>com.google.common.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.common.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.google.protobuf.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.protobuf.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang3.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang3.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.thrift.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.orc.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.orc.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.json.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.json.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>io.airlift.compress.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}io.airlift.compress.</shadedPattern>
+ </relocation>
+ <!-- Relocate parquet-format-structures embedded in hive-exec,
but keep
+ org.apache.parquet.format.converter.** (provided by
parquet-hadoop) at its
+ original name: it is used with the un-relocated
ParquetFileReader.readFooter,
+ so relocating it causes a NoSuchMethodError. -->
+ <relocation>
+ <pattern>org.apache.parquet.format.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.parquet.format.</shadedPattern>
+ <excludes>
+ <exclude>org.apache.parquet.format.converter.**</exclude>
+ </excludes>
+ </relocation>
+ <relocation>
+ <pattern>org.codehaus.jackson.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.codehaus.jackson.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.facebook.fb303.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.facebook.fb303.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.iq80.snappy.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.iq80.snappy.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>au.com.bytecode.opencsv.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}au.com.bytecode.opencsv.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>javaewah.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}javaewah.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>javolution.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}javolution.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>jodd.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}jodd.</shadedPattern>
+ </relocation>
+ <!-- DataNucleus (JDO impl behind HMS); its META-INF/services
entries are
+ rewritten by the ServicesResourceTransformer above. -->
+ <relocation>
+ <pattern>org.datanucleus.</pattern>
Review Comment:
`ServicesResourceTransformer` only handles `META-INF/services/*`; it does
not rewrite DataNucleus's root-level `plugin.xml`. Both `datanucleus-core` and
`datanucleus-api-jdo` register implementations there using original
`org.datanucleus.*` class names. After this relocation those classes only exist
under `org.apache.hudi.org.datanucleus.*`, leaving the plugin descriptors
stale. In addition, the current global filter excludes
`META-INF/services/javax.*`, so the JDO service files are not preserved as the
comment implies. This can break embedded/local metastore JDO initialization.
Please either avoid relocating DataNucleus or add proper merging/rewriting for
its plugin descriptors and cover initialization with a test.
##########
packaging/hudi-flink-bundle/pom.xml:
##########
@@ -228,6 +233,88 @@
<pattern>com.uber.m3.</pattern>
<shadedPattern>org.apache.hudi.com.uber.m3.</shadedPattern>
</relocation>
+
+ <!-- hive-exec is a fat jar that embeds these deps
un-relocated; under Flink
+ child-first classloading they shadow the versions
Hudi/Flink need. No-op
+ for the default bundle (classes only present when a
shade-hive profile
+ pulls hive-exec into compile scope). -->
+ <relocation>
+ <pattern>com.google.common.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.common.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.google.protobuf.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.protobuf.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang3.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang3.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.thrift.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.orc.</pattern>
Review Comment:
A package relocation does not distinguish classes embedded in `hive-exec`
from the explicit dependency. Both Hive's embedded ORC 1.3.3 and the declared
`orc-core` 1.5.6 are mapped to the same relocated path, so the duplicate-class
winner is unchanged. In the current Hive-2 shaded build, `OrcFile.class` in the
final bundle is byte-for-byte the copy from `hive-exec` 1.3.3, not the declared
1.5.6 artifact. After this change Hudi's ORC references are also rewritten to
that relocated stale copy. Please filter `org/apache/orc/**` from `hive-exec`
artifact-specifically, as done for Avro, or otherwise ensure the intended
modern ORC artifact exclusively populates the relocated namespace.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
