danny0405 commented on code in PR #20020:
URL: https://github.com/apache/hudi/pull/20020#discussion_r4118537951
##########
packaging/hudi-gcp-bundle/pom.xml:
##########
@@ -114,6 +133,10 @@
<pattern>org.openjdk.jol.</pattern>
<shadedPattern>org.apache.hudi.org.openjdk.jol.</shadedPattern>
</relocation>
+ <relocation>
+ <pattern>com.google.common.</pattern>
Review Comment:
Confirmed this concern with a runtime reproduction against head `0fbfa29`,
using an isolated SDK bundle with the PR's shading rules and Maven Shade 3.5.3.
With host `guava:27.0-jre` before the bundle, this fails with
`NoSuchFieldError: EXACT`:
```java
org.apache.hudi.com.google.common.net.InternetDomainName.from("example.com").publicSuffix();
```
With the bundle before host Guava, the host's unrelocated
`com.google.common.net.InternetDomainName` fails the same way. The bundle alone
succeeds. `PublicSuffixPatterns` stays at its original name, while its
`ImmutableMap` field descriptors are rewritten to the shaded Guava types, so
the host and bundled copies are binary-incompatible even though the class name
is shared.
Please also relocate `com.google.thirdparty.` and cover both classpath
orders in a packaging smoke test.
##########
packaging/hudi-gcp-bundle/pom.xml:
##########
@@ -99,7 +99,24 @@
<include>org.apache.hudi:hudi-hive-sync</include>
<include>org.apache.hudi:hudi-gcp</include>
<include>org.apache.parquet:parquet-avro</include>
- <include>com.google.cloud:google-cloud-bigquery</include>
+ <!-- Google Cloud SDK -->
+ <include>com.google.cloud:*</include>
+ <include>com.google.api:*</include>
+
<include>com.google.apis:google-api-services-storage</include>
+ <include>com.google.api-client:*</include>
+ <include>com.google.api.grpc:*</include>
+ <include>com.google.auth:*</include>
+ <include>com.google.code.gson:gson</include>
+ <include>com.google.http-client:*</include>
+ <include>com.google.oauth-client:*</include>
+ <include>com.google.protobuf:*</include>
+ <include>io.grpc:*</include>
+ <include>io.opencensus:*</include>
Review Comment:
[P2] Include Disruptor or avoid bundling the OpenCensus implementation
This wildcard includes `opencensus-impl` and `opencensus-impl-core` through
the GCS connector dependency tree, but `com.lmax:disruptor` is outside the
bundle allowlist. OpenCensus now finds the implementation and tries to
initialize it instead of using its no-op fallback.
In an isolated SDK bundle using this PR's shading rules, matching Google
dependency versions, and Maven Shade 3.5.3, supplying host
Jackson/logging/codec and calling:
```java
StorageOptions.newBuilder()
.setProjectId("review")
.setCredentials(NoCredentials.getInstance())
.setRetrySettings(StorageOptions.getDefaultRetrySettings().toBuilder().setMaxAttempts(1).build())
.build().getService();
```
fails with `ServiceConfigurationError: Provider
io.opencensus.impl.trace.TraceComponentImpl could not be instantiated`, caused
by `NoClassDefFoundError: com/lmax/disruptor/WaitStrategy`. Adding
`disruptor:3.4.2` makes initialization succeed without accessing GCS.
This is deployment-dependent: Hudi's Spark bundle already includes
Disruptor, and Maven consumers can obtain it from the reduced POM. It affects
direct-jar deployments that do not otherwise supply it. Please include the
implementation's runtime dependency or narrow the OpenCensus inclusion so the
implementation is not activated unintentionally. This was a packaging smoke
test, not a full engine integration test.
--
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]
