[DNM][CORE] Replace string-based native conf key lists with declarative NativeConfRegistry#12549
Draft
jackylee-ch wants to merge 2 commits into
Draft
[DNM][CORE] Replace string-based native conf key lists with declarative NativeConfRegistry#12549jackylee-ch wants to merge 2 commits into
jackylee-ch wants to merge 2 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 |
jackylee-ch
changed the title
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
1 participant
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. Session and backend(static) 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(scope)/passToNativeWithDefault(scope, default): Gluten config entries declare native passing right at their definitions, e.g.For
passToNativeWithDefault, the parsed default is registered (e.g. abytesConfdefault"1KB"is passed to native as"1024"), keeping JVM/native value formats consistent.NativeConfRegistry.registerRaw: for non-Gluten keys (SQLConf / Spark core / Hadoop / S3 / GCS) that have no GlutenConfigEntry. All such registrations now live in a singleregisterNativeConfs()inGlutenConfig, and velox-only raw keys are registered fromVeloxConfigin backends-velox instead of common code.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(SESSION)/registerRaw(key, SESSION)Seq.passToNativeWithDefault(SESSION)/registerRaw(..., defaultToPass)mapByteConfValuespecial casesregisterRaw(..., transform = byte conversion)timeParserPolicy.toUpperCaseregisterRaw(..., transform = upper-case)Seq.passToNativeWithDefault(BACKEND, ...)/registerRaw(..., BACKEND, defaultToPass)keysfilter.passToNative(BACKEND)/registerRaw(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.How was this patch tested?
NativeConfRegistrySuitecovering: scope registration via builder, per-scope mixed semantics, parsed-default registration for bytes confs, 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.