sarutak commented on PR #59000:
URL: https://github.com/apache/spark/pull/59000#issuecomment-5808662894
Thanks @ruanwenjun, and thanks @dongjoon-hyun for the ping.
**The diagnosis is correct and I support the fix.** I compared
`software.amazon.awssdk.utils.Logger` in `utils-2.35.4.jar` vs
`bundle-2.35.4.jar` with `javap -s`: the bundle relocates SLF4J, so `logger()`
has a different descriptor (`()Lorg/slf4j/Logger;` vs
`()Lsoftware/amazon/awssdk/thirdparty/org/slf4j/Logger;`). Same version, but
binary-incompatible signatures, which is exactly the `NoSuchMethodError` you
describe. Unifying on `bundle` is the right fix, +1.
But the CI evidence doesn't back "actively failing," so please tighten it:
1. I read the description as offering the linked run (35857041511) as an
existing failure caused by the #58051 mix. If so, that job's failure doesn't
match the claim: its only K8s IT failure is `DecommissionSuite` (a PySpark test
with no S3A I/O, doesn't touch `TransferListenerInvoker`), 9 annotations, none
a `NoSuchMethodError`. (Note: `VolcanoSuite`/ `VolcanoTestsSuite` do exist
under `resource-managers/kubernetes/integration-tests/volcano`, so the "S3
upload tests in KubernetesSuite/VolcanoSuite" claim is plausible in principle;
it just isn't what failed in the linked run.)
2. The PR presents this as a deterministic failure ("the failure seen in S3
upload tests..."), which is the right framing: a classpath-duplication
`NoSuchMethodError` is fixed by jar order, not flaky. But the linked failure is
the opposite, a `DecommissionSuite` timing/flakiness issue, not a
`NoSuchMethodError`. In the same run, a job that does drive S3A read/write
(OIDC E2E) passed, which is mild evidence the mixed classpath isn't uniformly
broken, but a passing job is not proof the error can't occur elsewhere (it
depends on code-path reachability, effective classpath order, and pod logs I
couldn't read). So the linked evidence doesn't establish the failure; a real
reproduction would settle it.
Could you either (a) link the exact job/log where `NoSuchMethodError`
actually reproduced on the pre-fix classpath, or (b) reframe this as preventive
hardening (modular plus bundle are binary-incompatible under relocation; unify
before it bites)? Just so the commit message matches reality.
**Minor:**
- The `credential-aws/pom.xml` comment (and the older #58051 one) frames the
risk as *version divergence*; the real cause is relocation, so same version was
never sufficient. Worth rewording.
- reactive-streams 1.0.4 to 1.0.3 is correct, but 1.0.4 came from the
modular SDK's `netty-nio-client` (removed here), not really an Aliyun choice.
- `hadoop-cloud` excludes all transitives on its `bundle`; `credential-aws`
doesn't. Harmless, but matching avoids future confusion.
--
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]
