[DNM][CORE] Replace string-based native conf key lists with declarative NativeConfRegistry#12549
Draft
jackylee-ch wants to merge 8 commits into
Draft
[DNM][CORE] Replace string-based native conf key lists with declarative NativeConfRegistry#12549jackylee-ch wants to merge 8 commits into
jackylee-ch wants to merge 8 commits into
Conversation
…ister native confs Currently, conf keys passed to native side are maintained in hard-coded string lists (`nativeKeys` in GlutenConfig, `extraNative*ConfKeys` in backend settings) that are decoupled from where the configs are defined. This introduces an additive registration mechanism: - `ConfigBuilder.passToNative(scope)` marks a Gluten config entry to be passed to native side, registered automatically on entry creation. - `NativeConfScope` (SESSION/BACKEND/BOTH) declares whether the conf is passed on each native runtime creation, once at backend init, or both. - `NativeConfRegistry` collects registrations; raw Spark/Hadoop keys can be registered via `registerRaw`, optionally with a default value that is always passed. - `getNativeSessionConf`/`getNativeBackendConf` additionally include registered confs, without changing existing key lists or prefix rules. This enables incremental migration of the hard-coded lists and allows each module to register its own native confs on demand. Generated-by: Claude (Cursor Agent)
|
Run Gluten Clickhouse CI on x86 |
…iveConfRegistry Complete the migration from hard-coded native conf key lists to the declarative passToNative mechanism: - NativeConfRegistry now tracks session/backend scopes separately, and supports per-entry defaultToPass (always-send with default) and transform (value normalization such as byte-unit conversion and upper-casing). - ConfigBuilder gains passToNative(scope) and passToNativeWithDefault(scope, default) that auto-register entries on creation, including parsed default values (e.g. bytes confs pass numeric bytes). - GlutenConfig: remove the nativeKeys set, the hard-coded default value lists and per-key special cases in getNativeSessionConf/getNativeBackendConf; both methods now select confs from NativeConfRegistry plus the existing prefix rules. Non-Gluten keys (SQLConf/Spark core/Hadoop/S3/GCS) are registered via NativeConfRegistry.registerRaw in one place. - GlutenCoreConfig/VeloxConfig: declare native passing at conf definitions; velox-only raw keys are registered in backends-velox instead of common code. - Remove BackendSettingsApi.extraNativeSessionConfKeys/extraNativeBackendConfKeys (no overrides existed) and the now-unused GlutenConfigUtil.mapByteConfValue. - Extend NativeConfRegistrySuite to cover scopes, defaults, transforms and duplicate registration. Generated-by: Cursor 2.4.28 (Fable 5)
|
Run Gluten Clickhouse CI on x86 |
…ope enum to NativeScope Address review feedback on API ergonomics: - Merge passToNative/passToNativeWithDefault into one method: passToNative(scope, alwaysPass, default). alwaysPass = true means the conf is always delivered to native with its (parsed) default when unset by user. - Rename the scope enum NativeConfScope -> NativeScope with clearer values: SESSION -> RUNTIME (passed on each native runtime creation), BOTH -> ALL; BACKEND unchanged (passed once at native backend initialization). - Rename NativeConfRegistry.registerRaw -> register, selectSessionConf -> selectRuntimeConf, isSessionKey -> isRuntimeKey accordingly. Generated-by: Cursor 2.4.28 (Fable 5)
|
Run Gluten Clickhouse CI on x86 |
…PassToNative property Address review feedback: - passToNative(scope) now takes only the scope, defaulting to ALL; users may narrow it to BACKEND or RUNTIME. - Remove the default parameter: the entry's own (parsed) default value is used. - Split always-pass semantics into a separate chainable alwaysPassToNative() property instead of a passToNative parameter. It requires passToNative() and an entry default value. - COLUMNAR_SHUFFLE_CODEC(_BACKEND) are optional entries without defaults, so their always-pass-empty-string behavior moves to raw registrations in registerNativeConfs(). Generated-by: Cursor 2.4.28 (Fable 5)
|
Run Gluten Clickhouse CI on x86 |
…ssToNative to passDefault Address review feedback: - passToNative() no longer takes a scope. The native scope is derived from the conf's staticness: static confs (buildStaticConf) are passed to native backend initialization; dynamic confs (buildConf) are passed on each native runtime creation. buildStaticConf marks the builder via markStatic(). - Confs whose consumption scope differs from the derived one (e.g. static velox cachePrefetchMinPct read at runtime creation, dynamic memory sizing confs consumed at backend init) drop the builder marker and register explicitly through NativeConfRegistry.register at the end of their conf object body, with comments explaining the mismatch. - Rename alwaysPassToNative() to the shorter passDefault(), conveying that the entry's default value is passed even when the conf is not set by user. Generated-by: Cursor 2.4.28 (Fable 5)
|
Run Gluten Clickhouse CI on x86 |
Follow the rule that static confs are for native backend initialization
(immutable afterwards) and non-static confs are for native session:
1. Static but consumed per native runtime -> make it non-static:
- velox cudf.enableTableScan
2. Non-static but consumed only at native backend init -> make it static:
- velox awsSdkLogLevel, fs.s3a.retry.mode (HiveConnector built once)
- core memoryOverhead.size.in.bytes (set on SparkConf at driver start only)
Also promote raw-registered s3UseProxyFromEnv / s3PayloadSigningPolicy
to static ConfigEntries.
3. Non-static and consumed by both backend init and session -> keep
dual registration and document the special semantics:
- debug enabled, cudf enabled, task offheap size
- offheap size / task slots stay non-static solely because
GlutenAutoAdjustStageResourceProfile mutates them at runtime.
Generated-by: Cursor 2.4.28 (Fable 5)
|
Run Gluten Clickhouse CI on x86 |
Documents the NativeConfRegistry design: the two delivery channels (BACKEND at backend init, RUNTIME at native runtime creation), scope derivation from conf staticness, the passToNative/passDefault builder API, explicit registration for non-Gluten keys, and the enumerated dual-scope confs with their split semantics. Generated-by: Cursor 2.4.28 (Fable 5)
|
Run Gluten Clickhouse CI on x86 |
…ass lazily evaluated - NativeConfRegistrySuite: filter select results to the keys under test so assertions are not affected by real registrations from conf objects (e.g. GlutenCoreConfig) loaded by other suites in the same JVM. - NativeConfRegistry: evaluate defaultToPass on each selection instead of snapshotting it at registration. spark.sql.session.timeZone's default follows the current JVM default time zone, so a snapshot taken at conf object initialization passes a stale time zone to native.
|
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 were proposed in this pull request?
[DNM] Do Not Merge — for design discussion.
This PR fully replaces the hard-coded, string-based native conf key lists in
GlutenConfig.getNativeSessionConf/getNativeBackendConfwith a declarative registration mechanism, so each module declares its own native confs on demand instead of common code maintaining giant key lists.The mechanism
NativeConfRegistry(gluten-core): a central registry of conf keys to pass to native side, keyed byNativeScope:RUNTIME: dynamic, passed each time a native runtime instance is created;BACKEND: static, passed once during native backend initialization;ALL: both.Runtime and backend scopes are tracked separately, so the same key can carry different semantics per scope. Each entry supports:
defaultToPass: if defined, the key is always passed, using the default value when unset by user (replaces the old "configs having default values" lists);transform: applied to user-set values before passing (replaces the old per-key special cases such as byte-unit conversion forspark.shuffle.file.bufferand upper-casing forspark.sql.legacy.timeParserPolicy).ConfigBuilder.passToNative(): Gluten config entries declare native passing right at their definitions. No scope argument — the native scope is derived from the conf's staticness: static confs (buildStaticConf) go toBACKEND, dynamic confs (buildConf) go toRUNTIME:passDefault()marks that the entry's own default value (parsed form, e.g. abytesConfdefault"1KB"is delivered as"1024") is passed even when the conf is not set by user.Confs whose native consumption scope differs from the derived one (e.g. static velox
cachePrefetchMinPctread on runtime creation; dynamic memory-sizing confs consumed at backend init) register explicitly viaNativeConfRegistry.registerat the end of their conf object body, with comments explaining the mismatch.NativeConfRegistry.register: for non-Gluten keys (SQLConf / Spark core / Hadoop / S3 / GCS) that have no GlutenConfigEntry, and for the scope-mismatch cases above. Common registrations live inregisterNativeConfs()inGlutenConfig; velox-only keys are registered fromVeloxConfigin backends-velox instead of common code. Registrations are naturally modular: a backend's registrations only take effect when its conf object loads, so backend-specific keys never leak across deployments.What is removed
nativeKeysset (40+ hard-coded strings, including velox-specific keys living in common code) inGlutenConfig.Seqs and all per-key special-case code ingetNativeSessionConf/getNativeBackendConf. Both methods now do: registry selection + existing prefix rules + UGI tokens (session only).BackendSettingsApi.extraNativeSessionConfKeys/extraNativeBackendConfKeys(no backend ever overrode them).GlutenConfigUtil.mapByteConfValue(superseded by entry-leveltransform).Behavior compatibility
The selected key/value results of
getNativeSessionConf/getNativeBackendConfare intended to be identical to before. Notable equivalence mappings:nativeKeysfilter.passToNative()on dynamic confs /register(key, RUNTIME)Seq.passToNative().passDefault()/register(..., defaultToPass)mapByteConfValuespecial casesregister(..., transform = byte conversion)timeParserPolicy.toUpperCaseregister(..., transform = upper-case)Seq.passToNative().passDefault()on static confs /register(..., BACKEND, defaultToPass)keysfilter.passToNative()on static confs /register(key, BACKEND)VeloxConfig(backends-velox)Prefix-based rules (
spark.gluten.sql.columnar.backend.<backend>,spark.hadoop.fs.s3a.etc.) are kept as-is, since they are pattern rules rather than string key registrations.Staticness alignment with native consumption scope
Static confs are for native backend initialization (immutable afterwards); non-static confs are for native session. Confs whose declared staticness did not match their actual native consumption were fixed:
Static but consumed per native runtime -> made non-static
spark.gluten.sql.columnar.backend.velox.cudf.enableTableScan: read by native inWholeStageResultIterator/SubstraitToVeloxPlanon every query, not at backend init, so it is session-tunable now.Non-static but consumed only at native backend init -> made static
spark.gluten.velox.awsSdkLogLevel,spark.gluten.velox.fs.s3a.retry.mode: only read when building the reused native HiveConnector at backend init; session changes never took effect.spark.gluten.memoryOverhead.size.in.bytes: only set on SparkConf at driver start (VeloxListenerApi), consumed by Velox backend init for global memory sizing.spark.gluten.velox.s3UseProxyFromEnv,spark.gluten.velox.s3PayloadSigningPolicy: promoted from raw string registrations to staticConfigEntrys inVeloxConfig.Non-static and consumed by both backend init and session -> allowed, dual-registered and documented in code
spark.gluten.sql.debugspark.gluten.sql.columnar.cudfspark.gluten.memory.task.offHeap.size.in.bytesGlutenAutoAdjustStageResourceProfilespark.gluten.memory.offHeap.size.in.bytesspark.gluten.numTaskSlotsPerExecutorDesign document
docs/developers/NativeConfPassing.mddocuments the full design: the two delivery channels (BACKEND at native backend init, RUNTIME at native runtime creation), scope derivation from conf staticness, thepassToNative()/passDefault()builder API, explicitNativeConfRegistry.registerfor non-Gluten keys, prefix rules, and the enumerated dual-scope confs with their split semantics.How was this patch tested?
NativeConfRegistrySuitecovering: scope derivation from conf staticness, parsed-default registration for bytes confs, passDefault constraint checks, per-scope mixed semantics, duplicate registration rejection, defaultToPass and transform semantics.-Pspark-3.5,backends-velox(checkstyle/scalastyle/spotless clean). backends-clickhouse Java code compiles; the delta33 Scala compile failure on this machine is pre-existing and unrelated.AI disclosure: this PR was developed with the assistance of Cursor (Fable 5), with human review of all changes.