akashchamp opened a new pull request, #20409:
URL: https://github.com/apache/druid/pull/20409
Fixes #19636.
### Description
When `druid-s3-extensions` and `druid-iceberg-extensions` are both loaded,
an Iceberg batch ingestion task that reads S3/Glue can fail before it starts
running subtasks, while Iceberg constructs its AWS S3 client, with:
```
java.lang.ExceptionInInitializerError
Caused by: java.lang.IllegalArgumentException: No duplicate IdentityProperty
names allowed but both IdentityProperties 5fb0a09e and 536fc38 have the same
namespace (java.lang.String) and name (Bucket). IdentityProperty should be
referenced from a shared static constant to protect against erroneous or
unexpected collisions.
```
**Root cause.** Both extensions declare their own direct dependency on
`software.amazon.awssdk:s3`, so `pull-deps` bundles a private copy of the `s3`
jar into each extension's own directory, and each extension is loaded through
its own `StandardURLClassLoader`
(`druid.extensions.useExtensionClassloaderFirst` defaults to `false`, i.e.
parent-first delegation).
`software.amazon.awssdk.services.s3.internal.s3express.S3ExpressPlugin` (inside
the `s3` jar) registers a static `IdentityProperty` constant into a registry
that lives in `identity-spi`. `identity-spi` is already loaded once, from the
shared parent classpath (`/opt/druid/lib`), because `druid-server` has a
runtime dependency on `druid-aws-common`, which depends on `identity-spi`
transitively. But `s3` itself is *not* on that shared classpath, so each
extension classloader loads its own copy of `S3ExpressPlugin`, and each copy's
static `IdentityProperty` field is a distinct object with the same
name/namespace — which the sha
red, singleton registry rejects as a "duplicate".
**Fix.** This follows the same pattern already used for `sts` (see the
"WebIdentityTokenProvider requires runtime dependency on sts" comment in
`cloud/aws-common/pom.xml`, and `sts` being `provided` in both extensions):
- Add `software.amazon.awssdk:s3` as a `runtime` dependency of
`druid-aws-common`, so it is resolved once, from the shared classpath
(`druid-server` → `druid-aws-common` → `s3`), instead of being extension-scoped.
- Mark `software.amazon.awssdk:s3` as `provided` in both
`extensions-core/s3-extensions/pom.xml` and
`extensions-contrib/druid-iceberg-extensions/pom.xml`, so neither extension
bundles its own copy any more (it's still on the compile/test classpath, just
not packaged).
No `licenses.yaml` change is needed: `software.amazon.awssdk:s3` (like
`sts`, `identity-spi`, `sdk-core`, etc.) is already covered by the existing
"AWS SDK for Java 2" entry, which is `module: java-core` and lists the whole
SDK, independent of which module physically bundles which artifact.
### Verification
- `mvn dependency:tree -Dscope=runtime` on `cloud/aws-common` now shows
`software.amazon.awssdk:s3:jar:2.55.1:runtime`.
- `mvn dependency:tree -Dscope=runtime` on `extensions-core/s3-extensions`
and `extensions-contrib/druid-iceberg-extensions` no longer lists
`software.amazon.awssdk:s3` anywhere in either module's own subtree (confirmed
by grepping the tree output for `awssdk:s3:` and checking the matches are
outside those modules' sections).
- `mvn test-compile` for `cloud/aws-common`,
`extensions-core/s3-extensions`, and
`extensions-contrib/druid-iceberg-extensions` (plus their `-am` prerequisites)
succeeds.
- Full existing unit test suites pass unchanged:
- `druid-s3-extensions`: 247 tests, 0 failures, 0 errors.
- `druid-iceberg-extensions`: 61 tests, 0 failures, 0 errors (including
`GlueIcebergCatalogTest`).
- **Manual reproduction of the actual crash mechanism**, outside the repo,
using the real `software.amazon.awssdk:s3` and `identity-spi` jars resolved by
this build (`2.55.1`) and two `URLClassLoader`s configured exactly like Druid's
default `StandardURLClassLoader` (parent-first delegation, one parent "shared
classpath" loader plus two child "extension" loaders): with a private copy of
`s3-2.55.1.jar` given to *each* child loader (today's pre-fix layout), building
an `S3Client` through the second loader reproduces the exact reported
exception, verbatim:
```
java.lang.IllegalArgumentException: No duplicate IdentityProperty names
allowed but both IdentityProperties 5a021cb9 and 2e52fb3e have the same
namespace (java.lang.String) and name (Bucket). IdentityProperty should be
referenced from a shared static constant to protect against erroneous or
unexpected collisions.
```
With `s3-2.55.1.jar` moved to the shared/parent loader instead (this fix's
layout — one copy, both extensions delegate to it), building an `S3Client`
through both loaders succeeds with no exception. This isn't a Druid integration
test I could reasonably check in (it stands up its own throwaway classloaders
against raw SDK jars rather than a running Druid process), but it directly
confirms the classloading mechanism described above and that this change
removes it.
I did not add real S3/Glue infrastructure to run an actual end-to-end
Iceberg ingestion task; the above reproduces the exact failing code path
(`S3Client.builder().build()` under duplicated extension classloaders) that the
issue's stack trace points to.
#### Release note
Fixed: Iceberg batch ingestion tasks no longer fail with
`IllegalArgumentException: No duplicate IdentityProperty names allowed ... name
(Bucket)` when `druid-s3-extensions` and `druid-iceberg-extensions` are loaded
together.
<hr>
##### Key changed/added classes in this PR
* None (build/packaging change only: `cloud/aws-common/pom.xml`,
`extensions-core/s3-extensions/pom.xml`,
`extensions-contrib/druid-iceberg-extensions/pom.xml`)
<hr>
This PR has:
- [x] been self-reviewed.
- [ ] added documentation for new or modified features or behaviors.
- [x] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
(not needed — see Description above)
- [x] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
---
This PR was prepared by an AI coding agent (Claude, via Claude Code) under
human supervision, from investigation through implementation and verification,
as disclosed above; the reproduction methodology and verification results are
genuine and were actually executed, not simulated.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
