[TEST][VL] Forward Hadoop filesystem configuration per runtime#12495
Draft
jackylee-ch wants to merge 1 commit into
Draft
[TEST][VL] Forward Hadoop filesystem configuration per runtime#12495jackylee-ch wants to merge 1 commit into
jackylee-ch wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR forwards user-provided Hadoop filesystem (fs.*) configuration from Spark into each native Velox runtime in a credential-safe way, ensuring native reads/writes use the same filesystem settings as the originating SparkSession.
Changes:
- Add a
HadoopConfContributorSPI andHadoopConfCollectorto snapshot and cache user-provided filesystem settings perSparkSession. - Transport the filesystem snapshot through whole-stage RDD execution and datasource writer/schema paths, and merge it into Velox runtime/connector configuration.
- Improve redaction behavior and runtime identity generation to avoid leaking or fingerprinting credentials; add Scala and C++ tests.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| shims/spark33/src/main/scala/org/apache/spark/sql/hive/execution/HiveFileFormat.scala | Pass SparkSession into nativeConf so writer can collect per-session fs config. |
| shims/spark33/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFileFormat.scala | Same session-aware nativeConf wiring for Parquet writes. |
| shims/spark33/src/main/scala/org/apache/spark/sql/execution/datasources/orc/OrcFileFormat.scala | Same session-aware nativeConf wiring for ORC writes. |
| shims/common/src/main/scala/org/apache/gluten/execution/datasource/GlutenFormatWriterInjects.scala | Add default session-aware nativeConf overload for compatibility. |
| gluten-ut/test/src/test/scala/org/apache/gluten/runtime/RuntimesSuite.scala | Add tests ensuring runtime resource IDs don’t leak/fingerprint credentials. |
| gluten-ut/test/src/test/scala/org/apache/gluten/execution/HadoopConfTransportSuite.scala | Add tests for fsConf transport through RDDs and IteratorApi overload delegation. |
| gluten-ut/test/src/test/scala/org/apache/gluten/execution/HadoopConfCollectorSuite.scala | Add tests for collection/filtering/normalization/caching behavior. |
| gluten-ut/test/src/test/scala/org/apache/gluten/config/GlutenRuntimeConfigSuite.scala | Add tests asserting default redaction regex is injected / user override preserved. |
| gluten-substrait/src/main/scala/org/apache/gluten/execution/WholeStageZippedPartitionsRDD.scala | Add serializable fsConf field and pass it into IteratorApi final-stage iterator creation. |
| gluten-substrait/src/main/scala/org/apache/gluten/execution/WholeStageTransformer.scala | Collect fsConf once per session and thread it into whole-stage RDD constructors. |
| gluten-substrait/src/main/scala/org/apache/gluten/execution/HadoopConfCollector.scala | New collector that filters user sources, normalizes keys, and caches per SparkSession. |
| gluten-substrait/src/main/scala/org/apache/gluten/execution/GlutenWholeStageColumnarRDD.scala | Add serializable fsConf field and pass it into IteratorApi first-stage iterator creation. |
| gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala | Add default native redaction regex to both session and backend native conf defaults. |
| gluten-substrait/src/main/scala/org/apache/gluten/backendsapi/IteratorApi.scala | Add fsConf overloads with default delegation to preserve existing backends. |
| gluten-core/src/main/scala/org/apache/gluten/config/HadoopConfContributor.scala | New SPI for backends/components to declare fs prefixes of interest. |
| gluten-arrow/src/main/scala/org/apache/gluten/runtime/Runtimes.scala | Replace resource key stringification with SHA-256-based credential-safe resourceId. |
| cpp/velox/utils/VeloxWholeStageDumper.h | Expose formatConfigForDump helper for testing and section-specific redaction. |
| cpp/velox/utils/VeloxWholeStageDumper.cc | Redact backend/session conf sections with their own regex; use helper to format dump. |
| cpp/velox/utils/ConfigExtractor.h | Declare filesystem config merge helper and invalid-GCS sentinel constants. |
| cpp/velox/utils/ConfigExtractor.cc | Implement per-runtime filesystem overlay merge, validate GCS runtime config, fail closed. |
| cpp/velox/tests/RuntimeTest.cc | Add tests for config merging, connector overlay behavior, and dump redaction semantics. |
| cpp/velox/operators/writer/VeloxParquetDataSourceGCS.h | Initialize GCS filesystem with per-runtime Hive connector config instead of nullptr. |
| cpp/velox/compute/VeloxRuntime.cc | Register connectors with per-runtime Hive config overlay derived from runtime fsConf. |
| cpp/velox/compute/VeloxBackend.h | Add overloads to create connectors with explicit connector configs (preserve old overloads). |
| cpp/velox/compute/VeloxBackend.cc | Implement new connector factory overloads and delegate old ones to new signatures. |
| cpp/core/tests/GlutenConfigTest.cc | Add tests for redaction search semantics and always-redact UGI identity keys. |
| cpp/core/tests/CMakeLists.txt | Register new gluten_config_test target. |
| cpp/core/config/GlutenConfig.h | Declare redaction helpers and move required includes to header. |
| cpp/core/config/GlutenConfig.cc | Add shouldRedactConfigKey and switch to search semantics in printConfig. |
| backends-velox/src/test/scala/org/apache/spark/sql/execution/datasources/velox/VeloxHadoopConfWriterSuite.scala | Add tests for writer fsConf collection/normalization and schema inspection resource handling. |
| backends-velox/src/test/scala/org/apache/gluten/backendsapi/velox/VeloxHadoopConfTransportSuite.scala | Add tests for Velox Iterator extraConf merging and control-key authority. |
| backends-velox/src/main/scala/org/apache/spark/sql/execution/datasources/velox/VeloxParquetWriterInjects.scala | Override session-aware nativeConf to include per-session collected filesystem config. |
| backends-velox/src/main/scala/org/apache/spark/sql/execution/datasources/velox/VeloxFormatWriterInjects.scala | Pass runtime fsConf into runtime creation; run schema inspection under TaskResources + session. |
| backends-velox/src/main/scala/org/apache/gluten/datasource/VeloxDataSourceUtil.scala | Add fsConf overloads for schema inspection and robust handle/resource cleanup helper. |
| backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxIteratorApi.scala | Add fsConf-aware iterator overloads and build merged extraConf including fsConf. |
| backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala | Implement HadoopConfContributor so Velox declares interested Hadoop fs prefixes. |
| backends-velox/src-delta33/main/scala/org/apache/spark/sql/delta/GlutenParquetFileFormat.scala | Pass SparkSession into nativeConf for Delta Parquet writes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Run Gluten Clickhouse CI on x86 |
jackylee-ch
force-pushed
the
feature/hadoop-conf-forwarding-unified-identity
branch
from
f9fb63b to
e0f4589
Compare
|
Run Gluten Clickhouse CI on x86 |
jackylee-ch
force-pushed
the
feature/hadoop-conf-forwarding-unified-identity
branch
from
e0f4589 to
11724eb
Compare
|
Run Gluten Clickhouse CI on x86 |
1 similar comment
Contributor
Author
|
Run Gluten Clickhouse CI on x86 |
jackylee-ch
force-pushed
the
feature/hadoop-conf-forwarding-unified-identity
branch
from
11724eb to
86ced24
Compare
|
Run Gluten Clickhouse CI on x86 |
jackylee-ch
force-pushed
the
feature/hadoop-conf-forwarding-unified-identity
branch
from
86ced24 to
3e6b778
Compare
|
Run Gluten Clickhouse CI on x86 |
jackylee-ch
force-pushed
the
feature/hadoop-conf-forwarding-unified-identity
branch
from
3e6b778 to
0e27c96
Compare
|
Run Gluten Clickhouse CI on x86 |
jackylee-ch
force-pushed
the
feature/hadoop-conf-forwarding-unified-identity
branch
from
0e27c96 to
8ddd16a
Compare
|
Run Gluten Clickhouse CI on x86 |
Collect user-provided filesystem settings once per Spark session and carry them through whole-stage RDDs and native writer paths without adding credentials to the Substrait plan. Create per-runtime Velox connector configuration, use credential-safe runtime identity, and redact sensitive filesystem and UGI settings. Preserve legacy overloads and constructors for backend compatibility.
jackylee-ch
force-pushed
the
feature/hadoop-conf-forwarding-unified-identity
branch
from
8ddd16a to
cb57a6b
Compare
|
Run Gluten Clickhouse CI on x86 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
This PR forwards user-provided Hadoop filesystem configuration (
fs.*fromspark-defaults.conf,--conf,sc.conf.set,core-site.xml, orsc.hadoopConfiguration.set) from Spark to each native Velox runtime, so filesystem credentials reach the native engine without being embedded in Substrait plans.fs.*settings once perSparkSessionvia aHadoopConfContributorSPI, keeping user sources and dropping built-in*-default.xmldefaults, and normalizing bare /spark.hadoop.key forms.IteratorApioverloads that default to the existing implementations (so backends such as ClickHouse keep working unchanged).Deferred to follow-up PRs (out of scope here): the Delta / Hive-serde writer path, and redaction hardening for sensitive settings. Existing redaction behavior is left unchanged.
How was this patch tested?
Yes. New Scala and C++ unit tests were added:
*-default.xml), bare /spark.hadoop.key normalization with bare-key precedence, variable resolution with a raw-value fallback for substitution cycles, and empty-prefix short-circuiting.IteratorApioverloads delegate to the legacy implementations by default.buildExtraConf— filesystem credentials are kept, unrelated settings dropped, and control keys overridden.ColumnarWriteFilesExecchild RDD carries the forwardedspark.hadoop.fs.*configuration.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude