sarutak commented on PR #58574: URL: https://github.com/apache/spark/pull/58574#issuecomment-5580146902
Thanks for the very thorough review, @dongjoon-hyun. These are all valid, and several point at the same underlying tension you summarized (the selection phase running in situations where no resolution follows). Let me lay out the direction I'd like to take and confirm one design question. **On symmetry with Kerberos / moving resolution earlier.** One option you raised is to have `SparkContext` own the manager lifecycle so the credential store is populated before anything consumes the Hadoop conf. I looked at how Kerberos achieves driver-side correctness, and I'd prefer not to move credential resolution into early `SparkContext` initialization: - `HadoopDelegationTokenManager` and `UserCredentialManager` both start late, from `CoarseGrainedSchedulerBackend.start()` (`setupTokenManager()` / `setupUserCredentialManager()`), after `_hadoopConfiguration` is materialized. Kerberos still works on the driver not because it resolves early, but because it writes tokens into `UserGroupInformation`. A process-global, mutable store that `FileSystem.get()` consults regardless of init order. - OIDC's credential values already follow the same mutable-store model (`SparkOidcAwsCredentialsProvider` reads `SparkEnv.userCredentials` at call time). The only order-dependent part is the static wiring `fs.s3a.aws.credentials.provider=<class>`) we write into the driver's `hadoopConfiguration`. - So pulling `resolve()` (network I/O) into early initialization would make OIDC more asymmetric than Kerberos, not less. Kerberos never resolves that early. It would also move fail-fast and add external I/O during `new SparkContext`. Given that, I'd like to keep the selection/resolution split and instead fix the regressions directly, so that the selection phase is genuinely I/O-free and never wires up a provider in a context where resolution won't follow. Concretely: - Make the selection phase truly I/O-free: obtain `additionalSparkProperties()` without eagerly building provider clients (it's a constant map), so `AwsStsCredentialProvider.init()` no longer triggers IMDS region lookups during `new SparkContext`. - Don't wire providers when no resolution will follow (see the local-mode question below), and log a WARN (not DEBUG) if selection cannot apply a provider's properties, keeping an idempotent fallback application in `start()` for providers that do resolve. - Fix the shutdown ordering / single-owner issues for the loader. - Update the scaladoc and the PR description to reflect the selected-vs-resolved behavior change. - Add `SparkContext`-level tests (not just the Minikube E2E) and move the call earlier. **One question I'd like your steer on.** For local mode: Kerberos does run in `LocalSchedulerBackend` (it overrides `createTokenManager()`), whereas `UserCredentialManager` is only started by `CoarseGrainedSchedulerBackend`, so OIDC never resolves in local mode today. Two ways to make selection consistent with that: (a) skip the selection phase when `isLocal` (resolution won't follow, so don't wire), or (b) also run `UserCredentialManager` in `LocalSchedulerBackend` (true parity with Kerberos; still resolving late, so no early-I/O asymmetry). (a) is minimal and matches today's "OIDC is unsupported in local mode" reality; (b) is a small scope increase but gives real Kerberos parity. Do you have a preference? I lean toward (a) for this PR and filing (b) separately, unless you'd rather have parity now. For the constructor-window case (`spark.jars`/`spark.files=s3a://...` fetched before `_taskScheduler.start()`), that one isn't fully solvable without early resolution; I plan to document it as a limitation (prefer `local://` resources, or `fs.s3a.impl.disable.cache=true`), which is the same class of early-driver-access caveat Kerberos-based setups have. Let me know if you'd want more than documentation there. -- 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]
