[S2.9 / C1] Config plumbing: master-switch + active-sweeper + interval#29
Merged
Conversation
Implements the declarative master-switch model from PR-B (design doc):
replaces the bool `compression-enabled` config with the 3-state enum
`compression-master-switch` (compression / decompression / off, default
off), adds the `compression-active-sweeper` enum (enabled / disabled,
default disabled) and `compression-active-sweeper-interval` time config
(default 0 = no periodic re-runs). Matching INFO fields. Apply hooks
implementing the R2.1.5 transition rules and the R2.1.6 non-functional
state warning. C1 stops at config plumbing; the transient-view drain
mode dispatch is C2, the sweeper engine + COMPRESSION SWEEP FORCE
command are C3.
Master-switch transition table (R2.1.5):
compression -> decompression : auto-retire active dict (drain mode)
compression -> off : NO retire (state frozen)
off -> decompression : auto-retire active dict if any
decompression -> off : no action (already retired)
off -> compression : no action; existing dict reusable
decompression -> compression : no action; falls into R2.1.7 third state
Apply-hook implementation notes:
- The hooks track the prior value via file-static caches (one for
master, one for active-sweeper, one shared for the warning detector
+ a parallel for compression-threads). compressionInit() calls
syncApplyHookCaches() AFTER boot config is applied, which fixes two
bugs:
1. Boot-time non-functional warning (master=compression+threads=0
set at boot) didn't fire because apply hooks don't run during
boot config load.
2. An operator booting with master=compression and later setting
master=decompression would have seen the apply hook log a
misleading "off -> decompression" transition (because the
static was initialized to the default, not the boot value).
- applyCompressionThreads in config.c calls compressionAfterThreadsApplied
to keep the warning detector's threads cache in sync.
- maybeWarnNonFunctional emits LL_WARNING once on transition INTO
master=compression+threads=0; subsequent CONFIG SETs that don't
leave the state are no-ops.
Stale-reference sweep (PR-B vocabulary update):
- src/compression.c: 4 comments and 2 production checks updated from
"compression-enabled" / server.compression_enabled to master-switch
language. INFO field rendering switched from compression_enabled:0
to compression_master_switch:<name> + new compression_active_sweeper
+ compression_active_sweeper_interval fields.
- src/compression_workers.c: 1 comment line.
- src/compression_train.c: gated cron entry on master==compression
instead of !enabled bool.
- src/compression_registry.h: 2 comments referencing
"COMPRESSION SWEEP direction=decompress" updated to the new operator
surface (master=decompression + active-sweeper=enabled).
- src/compression_registry.c: log message at dict-cap-reached.
- src/compression.h: removed unused compressionToggle() declaration
(was a Phase-0 stub, never called); replaced with the apply-hook
declarations.
Test updates:
- src/unit/test_compression_eligibility.cpp: replaced
server.compression_enabled with server.compression_master_switch
using the new enum constants. Added a new test case
MasterSwitchDecompressionBlocksEligibility verifying that the
predicate also rejects in master=decompression (drain mode).
- src/unit/test_compression_train.cpp: same conversion.
- src/unit/test_compression_workers.cpp: 1 comment update.
- tests/unit/type/compression.tcl: rewrote 5 tests that referenced
compression-enabled to use compression-master-switch. Added two
new tests verifying the enum accepts each documented value plus
rejects out-of-set values for both master-switch and active-sweeper.
Updated the 18-knob defaults check (was 16 knobs; the rename from
1 bool to 1 enum + 2 new sweeper configs increases primary-knob
count from 5 to 6, advanced unchanged at 11, hence 17... wait,
the test count is 18 because compression-active-sweeper-interval
is a separate config registration. Confirmed by the test passing.)
Verified:
make -j2 -C src SERVER_CFLAGS=-Werror # clean (BUILD_ZSTD=yes)
make -C src distclean
make -j2 -C src SERVER_CFLAGS=-Werror BUILD_ZSTD=no # clean
./runtest --single unit/type/compression # 13/13 pass
Boot/runtime smoke:
- --compression-master-switch compression --compression-threads 0
emits boot-time LL_WARNING.
- CONFIG SET master compression -> decompression -> off logs the
transitions correctly with the right "from" values.
- INFO compression and COMPRESSION STATUS render the new fields.
Not verified locally:
- gtest unit tests (libgtest-dev not installed). CI validates.
- clang-format-18. CI validates.
Three findings from a thorough re-sweep of the codebase for residual
PR-B-vocabulary references:
1. compressionCommand() still routed `enable` / `disable` subcommands
to a Phase-0 stub (return OK silently), and the HELP text listed
`ENABLE, DISABLE, SWEEP, TRAIN` as future Phase-1 work. Per
PR-B's design §4.5, the ENABLE/DISABLE aliases are NOT part of
v1 (the 3-state enum doesn't map cleanly to enable/disable
verbs). Removed the stub branch and rewrote the HELP text to
reflect the v1 surface (STATUS / HELP land here; SWEEP FORCE /
TRAIN / DICT * land in subsequent S2 PRs).
2. Three code comments cited "R2.1.5" when describing the "no
active dictionary" state. Per the post-PR-B design that's
actually R2.1.7 (master=compression but no active dict yet —
the third state). Updated:
- src/compression.c:1158 (compressionEnqueueCandidate's
no-dict short-circuit)
- src/compression.h:299 (header doc for the same)
- src/compression_workers.c:619 (worker err>0 worker-policy
sentinel comment)
3. .agents/planning/realtime-data-compression/idea-honing.md's
PR-B header note said "Q1-Q16 hold up unchanged" but Q15
(testing strategy — §7.1 transparency-mode harness) describes
a `--compression` flag whose server-config payload uses the
old `--compression-enabled yes` etc. The flag's design intent
is still valid, but the specific config names are superseded.
Tightened the header note to flag Q15 explicitly and revise
the unchanged-Qs list (Q1-Q4, Q6-Q14, Q16).
Verified:
make -j2 -C src SERVER_CFLAGS=-Werror # clean (BUILD_ZSTD=yes)
./runtest --single unit/type/compression # 13/13 pass
…uct comments CI's clang-format-18 wants: 1. src/compression.c masterSwitchName(): switch-case 'return' values should NOT be column-aligned to the longest case label. The default LLVM 18 style collapses each 'case X: return Y;' onto a single line with one space. 2. src/server.h compressionState struct: the new compression_active_sweeper_interval field (35 chars) is longer than the prior longest field (compression_max_value_size at 26 chars), so the trailing-comment alignment column for ALL fields in this block needs to shift right (from column 39 to column 46). Also the continuation lines wrapping the multi-line comments need to align with the new comment column. Both findings reported by the failed clang-format-check job on PR #29 (CI is the source of truth here — clang-format-18 isn't installed locally). Verified the build still passes after the formatting fix.
This was referenced Jun 14, 2026
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.
Summary
PR-C1 of 3 in the implementation of the declarative master-switch model from PR #28 (PR-B). Replaces the bool
compression-enabledconfig with the 3-state enumcompression-master-switchand adds the active-sweeper configs. C1 is config plumbing only — the transient-view drain mode dispatch lands in C2, the sweeper engine +COMPRESSION SWEEP FORCEcommand land in C3.What changes
New configs (R2.1.1 – R2.1.3)
compression-master-switchoff/compression/decompressionoffcompression-enabledboolcompression-active-sweeperdisabled/enableddisabledcompression-active-sweeper-interval0Plus matching INFO fields:
compression_master_switch,compression_active_sweeper,compression_active_sweeper_interval. The legacycompression_enabled:0INFO field is removed.Two new enum constant blocks in
server.h:Apply hooks (R2.1.5 – R2.1.6)
applyCompressionMasterSwitchimplements the auto-retire policy: on any transition INTOdecompression(fromcompressionor fromoff), the active dict is retired viacompressionRegistryRetire(active_id)so it can drain. All other transitions leave the registry untouched. The hook logs the transition atLL_NOTICE.applyCompressionActiveSweeperis minimal in C1 — logs the transition. Engine wiring is C3.maybeWarnNonFunctionalis called from both apply hooks plusapplyCompressionThreads(via the newcompressionAfterThreadsAppliedshim inconfig.c). EmitsLL_WARNINGonce on transition INTOmaster=compression + threads=0(R2.1.6).Static cache initialization
The apply hooks track prior values via file-static caches.
compressionInit()callssyncApplyHookCaches()AFTER boot config is applied, which fixes two boot-time bugs:master=compression + threads=0set at boot) didn't fire because apply hooks don't run during boot config load. Now the sync function explicitly callsmaybeWarnNonFunctional()once at init.master=compressionand later settingmaster=decompressionwould have seen the apply hook log a misleading"off -> decompression"transition (because the static was initialized to the default, not the boot value). Now the static caches reflect the boot values, so the first runtime transition logs correctly.Verified manually with:
Stale-reference sweep (PR-B vocabulary update)
src/compression.ccompressionIsEligible,compressionEnqueueModified) gated onmaster == COMPRESSION_MASTER_COMPRESSION. INFO renderer rewritten.src/compression_workers.csrc/compression_train.cmaster == compression(drain mode and off both block training).src/compression_registry.{h,c}COMPRESSION SWEEP direction=decompresswith the new operator surface (master=decompression + active-sweeper=enabled).src/compression.hcompressionToggle()declaration (Phase-0 stub, never called); replaced with the apply-hook declarations +compressionAfterThreadsAppliedshim.COMPRESSION ENABLE/COMPRESSION DISABLEaliases are referenced exactly once — in a doc comment incompression.hexplicitly noting they are NOT part of v1's surface.Test updates
src/unit/test_compression_eligibility.cpp— replacedserver.compression_enabledwithserver.compression_master_switchusing the new enum constants. AddedMasterSwitchDecompressionBlocksEligibilityverifying the predicate also rejects in master=decompression (drain mode).src/unit/test_compression_train.cpp— same conversion.src/unit/test_compression_workers.cpp— 1 comment update.tests/unit/type/compression.tcl— rewrote tests referencingcompression-enabled. Added two new tests verifying the enum accepts each documented value and rejects out-of-set values for bothcompression-master-switchandcompression-active-sweeper. Updated the knob-default-count test (now 18 knobs including the 3 new ones).What's NOT in this PR (later C-series)
compressionMaterializeTransientViewandcompressionBeforeSleepneed amaster == decompressionbranch that permanent-decompresses instead of restoring.COMPRESSION SWEEP FORCEcommand. Replaces PR [S2.9 + S2.10] Sweep state machine + COMPRESSION SWEEP command #26's content (which will be closed when C3 lands).Verified locally
Boot/runtime smoke:
./src/valkey-server --compression-master-switch compression --compression-threads 0emits the boot-time warning.CONFIG SET compression-master-switch compression -> decompression -> offlogs each transition correctly with the right "from" values.INFO compressionandCOMPRESSION STATUSrender the new fields and stay in lockstep (the meta-test incompression.tclenforces this).Not verified locally (CI gates)
libgtest-devnot installed in this dev environment).clang-format-18.Diff stat