Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion packaging/hudi-aws-bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@
<shadedPattern>org.apache.hudi.aws.org.apache.httpcomponents.</shadedPattern>
Comment thread
voonhous marked this conversation as resolved.
</relocation>
</relocations>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

promoteTransitiveDependencies is not set anywhere in the repo, so it defaults to false. That means shade removes the direct dependency entry and everything that only arrived through it, including artifacts that are not in the bundle. Not just "artifacts it actually bundled", as the description puts it.

Here that is hudi-hive-sync, which today supplies hadoop-common, hadoop-client, hadoop-hdfs, hadoop-auth, hive-jdbc, hive-metastore and hive-common at compile scope. None of them are in this bundle's artifactSet, and the jar still contains the org.apache.hudi.hive.* classes that call into them. After this change the published POM advertises neither the classes' host jar nor its dependencies.

That may well be the right end state for a fat bundle meant to land on a Hadoop/Hive classpath, but it is a larger change than the impact section describes. Either set promoteTransitiveDependencies=true, so the reduced POM keeps the non-bundled transitives and preserves the runtime contract exactly minus what was absorbed, or call the drop out explicitly.

(The two kept deps are right: hudi-common and parquet-avro are genuinely not bundled.)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed on the mechanism, and my Impact wording was wrong — "artifacts it actually bundled" is not what promoteTransitiveDependencies=false does. Fixed in ed3ccf5 by taking your first option, promoteTransitiveDependencies=true, on all seven bundles, so the reduced POM keeps the runtime contract minus what was absorbed. Effect is visible on hudi-hadoop-mr-bundle, whose reduced POM goes from 0 to 79 dependencies (orc-core, rocksdbjni, jetty, metrics, simpleclient, lz4-java, disruptor, jaxb-api, …). The other bundles land at 74–191.

One correction on the specific example, though, because it changed what I expected to see. Those seven artifacts are not in hudi-aws-bundle's dependency closure, before or after this PR. They are provided in the root pom's dependencyManagement:

hadoop-client, hadoop-common, hadoop-hdfs, hadoop-auth,
hive-jdbc, hive-metastore, hive-common   ->  <scope>provided</scope>

provided is not transitive, so they never propagate from hudi-hive-sync to this bundle. mvn dependency:list -pl packaging/hudi-aws-bundle returns exactly one org.apache.hive/org.apache.hadoop artifact in the whole tree — hive-storage-api:2.6.0:compile — and no hadoop-* at all. So removing the hudi-hive-sync entry from the published POM was not dropping them; they were never advertised to consumers in the first place.

They are compile scope where you looked, in hudi-sync/hudi-hive-sync/pom.xml, which is what I assume produced the list. The root-level management is what makes them non-transitive one level up.

That does not change the outcome here: your general point was correct, non-bundled transitives were being dropped, and promoteTransitiveDependencies=true now preserves them. It only means the classes in this bundle that call into Hadoop/Hive were relying on a provided classpath before this PR as well, which is the normal contract for a bundle meant to land on a Hadoop/Hive cluster. Say the word if you would rather I also called that out in the Impact section — I have rewritten it, but I did not want to imply this PR changed that part.

And agreed on hudi-common and parquet-avro being correctly kept.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

One more thing I found on a re-check, worth your call before this merges.

Nine other bundles already publish a reduced POM and none of them sets promoteTransitiveDependencies: hudi-spark-bundle, hudi-utilities-bundle, hudi-utilities-slim-bundle, hudi-flink-bundle, hudi-cli-bundle, hudi-kafka-connect-bundle, hudi-integ-test-bundle, hudi-metaserver-server-bundle (shade's default) and hudi-timeline-server-bundle (explicit true).

hudi-spark3.5-bundle_2.12's published POM has carried 8 dependencies from 0.15.0 through current master, so dropping non-bundled transitives is the shipped behaviour for those nine. With promotion the seven here publish 73-191 instead. So the real choice is promotion everywhere or promotion nowhere, and right now this PR makes seven differ from nine.

I have kept promotion because it preserves the runtime contract you raised and is the conservative direction for consumers, but I did not want to decide house style silently. Either is a small change from here: drop it and document the transitive drop instead, or keep it and align the other nine in a follow-up. I left those nine alone deliberately, since touching them changes published metadata for nine more artifacts including the Spark and Flink bundles.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Decision: keep promoteTransitiveDependencies=true on these seven. Not blocking on aligning the other nine.

The three options are not symmetric:

POM state what a consumer resolves
today (createDependencyReducedPom=false) full closure, including unrelocated copies of what the jar already absorbed
reduced, promotion off direct deps minus absorbed -> loses real runtime deps
reduced, promotion on (this PR) full closure minus absorbed

Only the third preserves the runtime contract, so this is not really "house style vs correctness". The nine get away with promotion-off because the Spark/Flink bundles land on a classpath that already supplies everything; these seven land on Hive/Presto/MR classpaths that do not ship orc-core, rocksdbjni, jetty-server or dropwizard metrics. Leaving the nine alone here is the right scope.

Verified on a clean build of ed3ccf5 vs 633d142:

  • all seven emit a reduced POM (74 / 80 / 82 / 105 / 112 / 190 / 74 deps)
  • cross-checked every entry against each bundle's effective artifactSet (own includes plus the inherited root includes): zero overlap, nothing that is inside a jar is still declared
  • every promoted entry carries an explicit <version>, <parent> is retained, no test/optional/classifier artifacts leaked in
  • hudi-presto-bundle, hudi-hadoop-mr-bundle and hudi-datahub-sync-bundle jar listings are identical to master, and presto keeps its 109 org/apache/hudi/hadoop/** entries

Action for this PR (description only): add a line to Impact noting that promotion flattens the graph, so those 80-190 artifacts become depth-1 for consumers with pinned versions and baked-in exclusions, and therefore win mediation against the consumer's own deeper transitives. That differs from both today's behaviour and the other nine.

Follow-up, not here: align the nine, or record an explicit decision that they stay as they are.

<!-- Keep dependencies that are not absorbed into the shaded jar, so the reduced POM still
declares what consumers need at runtime. -->
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
<filters>
<filter>
<artifact>*:*</artifact>
Expand Down
5 changes: 4 additions & 1 deletion packaging/hudi-azure-bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,10 @@
<shadedPattern>org.apache.hudi.org.reactivestreams.</shadedPattern>
</relocation>
</relocations>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>
Comment thread
voonhous marked this conversation as resolved.
<!-- Keep dependencies that are not absorbed into the shaded jar, so the reduced POM still
declares what consumers need at runtime. -->
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
<filters>
<filter>
<artifact>*:*</artifact>
Expand Down
5 changes: 4 additions & 1 deletion packaging/hudi-datahub-sync-bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@
<shadedPattern>org.apache.hudi.org.openjdk.jol.</shadedPattern>
</relocation>
</relocations>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Least surprising of the seven: parquet-avro and avro are already provided so they survive the reduction, and the three removed Hudi deps are all in the artifactSet. The transitive-drop caveat from the aws-bundle comment still applies to whatever hudi-common was supplying that is not bundled here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks — the transitive-drop point is addressed for this bundle too: promoteTransitiveDependencies=true is now set on all seven, so hudi-datahub-sync-bundle's reduced POM keeps the dependencies that are not absorbed into the shaded jar rather than dropping them with the removed entry. Details and the numbers are in my reply on the hudi-aws-bundle thread, including a correction about which hadoop/hive artifacts were actually in the closure.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two corrections after rebuilding ed3ccf5, neither blocking.

1. parquet-avro and avro no longer survive the reduction. That held for 73d9d06, but not after promotion was added. Shade runs at requiresDependencyResolution=runtime, so project.getArtifacts() is compile+runtime only, and with promoteTransitiveDependencies=true that set replaces the original direct-dependency list rather than filtering it, so provided entries are dropped. On the built POM:

packaging/hudi-datahub-sync-bundle/target/dependency-reduced-pom.xml
  parquet-avro : absent
  avro         : absent

Consumer impact is nil, since provided is not transitive, and this is the only one of the seven that declares any provided deps. So it is a description fix, not a code fix.

2. The reduced POM declares hudi-hadoop-mr:compile, and that artifact depends on hudi-common and hudi-hadoop-common, both of which are in this bundle's artifactSet. The absorbed artifacts come back one hop down. Not a regression, since today's POM declares hudi-common directly anyway, just an incomplete win worth knowing about.

Action: description only. Drop the "already provided so they survive" reasoning and note the provided drop instead. No POM change needed.

<!-- Keep dependencies that are not absorbed into the shaded jar, so the reduced POM still
declares what consumers need at runtime. -->
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
<filters>
<filter>
<artifact>*:*</artifact>
Expand Down
5 changes: 4 additions & 1 deletion packaging/hudi-gcp-bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@
<shadedPattern>org.apache.hudi.org.openjdk.jol.</shadedPattern>
</relocation>
</relocations>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>
Comment thread
voonhous marked this conversation as resolved.
<!-- Keep dependencies that are not absorbed into the shaded jar, so the reduced POM still
declares what consumers need at runtime. -->
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
<filters>
<filter>
<artifact>*:*</artifact>
Expand Down
5 changes: 4 additions & 1 deletion packaging/hudi-hadoop-mr-bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@
<shadedPattern>org.apache.hudi.com.fasterxml.jackson.</shadedPattern>
</relocation>
</relocations>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>
Comment thread
voonhous marked this conversation as resolved.
<!-- Keep dependencies that are not absorbed into the shaded jar, so the reduced POM still
declares what consumers need at runtime. -->
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
<filters>
<filter>
<artifact>*:*</artifact>
Expand Down
5 changes: 4 additions & 1 deletion packaging/hudi-hive-sync-bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@
<shadedPattern>org.apache.hudi.org.objenesis.</shadedPattern>
</relocation>
</relocations>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>
Comment thread
voonhous marked this conversation as resolved.
<!-- Keep dependencies that are not absorbed into the shaded jar, so the reduced POM still
declares what consumers need at runtime. -->
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
<filters>
<filter>
<artifact>*:*</artifact>
Expand Down
18 changes: 17 additions & 1 deletion packaging/hudi-presto-bundle/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@
<shadedPattern>org.apache.hudi.org.openjdk.jol.</shadedPattern>
</relocation>
</relocations>
<createDependencyReducedPom>false</createDependencyReducedPom>
<createDependencyReducedPom>true</createDependencyReducedPom>
Comment thread
voonhous marked this conversation as resolved.
<!-- Keep dependencies that are not absorbed into the shaded jar, so the reduced POM still
declares what consumers need at runtime. -->
<promoteTransitiveDependencies>true</promoteTransitiveDependencies>
<filters>
<filter>
<artifact>*:*</artifact>
Expand Down Expand Up @@ -193,6 +196,19 @@
<artifactId>hudi-hadoop-mr-bundle</artifactId>
<version>${project.version}</version>
</dependency>
<!-- Both are shaded in by the artifactSet above. Declared directly rather than relying on
hudi-hadoop-mr-bundle to supply them transitively, so this bundle's shade inputs do not depend
on another bundle's published dependency list. -->
<dependency>
<groupId>org.apache.hudi</groupId>
<artifactId>hudi-hadoop-mr</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.apache.hudi</groupId>
<artifactId>hudi-hadoop-common</artifactId>
<version>${project.version}</version>
</dependency>

<!-- Kryo -->
<dependency>
Expand Down
Loading