sarutak commented on code in PR #57998:
URL: https://github.com/apache/spark/pull/57998#discussion_r3784187863
##########
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala:
##########
@@ -1089,6 +1089,15 @@ class CoarseGrainedSchedulerBackend(scheduler:
TaskSchedulerImpl, val rpcEnv: Rp
// is processed by DriverEndpoint.
VersionedCredentials.updateIfNewer(
SparkEnv.get.userCredentials, version, initialCredentials)
+
+ // Auto-configure the executor-side S3A credentials provider if the user
has not
+ // explicitly set one. This ensures S3A reads dynamic credentials from
the executor
+ // store without requiring manual Hadoop configuration.
+ val s3aProviderKey = "spark.hadoop.fs.s3a.aws.credentials.provider"
Review Comment:
`CoarseGrainedSchedulerBackend` is responsible for resource management and
task scheduling. It should not have knowledge of specific storage systems (S3A)
or cloud providers (AWS). The SPIP design document explicitly states the design
principle for executor-side injection:
> Core SPI only stores `ServiceCredential`s in the executor credential
store. **No provider-specific injection logic in `core`.**
The current implementation places S3A-specific logic
(`fs.s3a.aws.credentials.provider` key and the
`SparkOidcAwsCredentialsProvider` class name) in `spark-core`'s scheduler
backend, which contradicts this principle. Specific issues:
1. **Layer violation**: A core scheduler component contains a
cloud-vendor-specific configuration key (`fs.s3a.aws.credentials.provider`) and
a class name from an optional module (`credential-aws`). When Azure (ABFS) and
GCS providers are added in the future, this block would need to grow with
additional vendor-specific entries.
2. **Invisible dependency on an optional module**: `spark-core` now
references a class from `credential-aws` (as a string literal). If the module
is not on the classpath, the failure surfaces only at S3A access time as a
`ClassNotFoundException`.
3. **Driver-side effect**: This mutates the driver's `SparkConf`. However,
since `SparkContext._hadoopConfiguration` is already materialized before
`setupUserCredentialManager` runs, the setting only takes effect on executors
(via `SparkAppConfig`). This is probably the correct behavior but is not
documented and could confuse readers.
**Suggestion**: Move this logic out of `CoarseGrainedSchedulerBackend`. Some
possible approaches (not prescriptive):
- The `credential-aws` module itself provides a hook (e.g., via
ServiceLoader or a callback from `UserCredentialManager`) that injects the
necessary Hadoop properties when the module is on the classpath.
- `UserCredentialManager.start()` (which already knows about the registered
providers and their `supportedSchemes()`) applies scheme-to-executor-provider
mappings based on configuration, keeping the mapping in the provider module
rather than in core.
- At minimum, the logic could be moved from the scheduler backend to
`UserCredentialManager`, since it already depends on the provider
infrastructure.
The key constraint from the SPIP is that `core` should not contain
provider-specific injection logic. The "where to configure" decision belongs to
the provider module.
--
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]
