[S2.9 / C2] Transient-view drain: master-switch-conditional dispatch (R2.5.7)#30
Merged
Merged
Conversation
…(R2.5.7)
Implements the only intentional cross-mechanism dependency in the
declarative master-switch design (PR-B): the read-path transient-view
drain mode follows the master switch.
master ∈ {compression, off} → RESTORE mode (existing behavior).
Pointer-swap back to compressed.
Cold values stay compressed across
iterations (R2.5.6 preserved).
ZERO recompression cost.
master == decompression → PERMANENT-DECOMPRESS mode.
Release the saved compressed buffer
(decRef dict frame-ref + reverse
install accounting + zfree); the
temp sds stays installed as
val_ptr; encoding stays RAW.
The robj is now permanently
decompressed.
Without the dispatch, every read-touched key would oscillate
between compressed and uncompressed each iteration in master=
decompression — defeating the operator's drain intent. With it, a
single sweep across the keyspace plus normal read traffic fully
decompresses the dataset. Documented in design §2.5.7.
The drain reads `server.compression_master_switch` once per
beforeSleep invocation; mid-iteration master-switch changes resolve
at the next iteration. The "stale slot" branch is mode-independent:
when the kvstore no longer references our pinned robj, we always
discard (free both buffers + drop pin + free entry).
Refactor: the per-entry RESTORE and PERMANENT-DECOMPRESS finalize
logic is extracted into static inline helpers
(restoreTransientEntry / permanentlyDecompressTransientEntry)
alongside the existing discardTransientEntry. Same shape, easy to
read at the call site, and reusable by the new test-only drain
variants.
Test infrastructure additions:
testOnlyCompressionDrainTransientViewAsRestore (new)
testOnlyCompressionDrainTransientViewAsPermanentDecompress (new)
Both follow the existing AsDiscard pattern: iterate the side-map,
apply the per-entry finalize, skip the kvstore lookup. Lets unit
tests verify the drain side effects without setting up a populated
kvstore.
New gtest cases:
RestoreDrainPutsCompressedFormBack
- Materialize, drain in RESTORE mode, verify the robj is back to
OBJ_ENCODING_COMPRESSED with the original compressed buffer
pointer reinstated.
PermanentDecompressDrainReleasesCompressedBuffer
- Materialize, drain in PERMANENT-DECOMPRESS mode, verify the
robj is permanently RAW with the temp sds still installed and
the decompressed bytes round-tripping the original source.
PermanentDecompressDrainHandlesMultipleEntries
- Three entries in the side-map, drain processes all of them;
iteration handles >1 entry without leaks or state confusion.
RestoreDrainResetsPerIterationBudget
- Tight savings cap: materialize → drain (restore) → materialize
again. The second materialize must succeed (would fall back to
permanent-decompress if the budget hadn't reset).
Verified locally:
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
make -j2 -C src test-unit
367 tests pass (was 363 — 4 new for the two-mode drain).
./runtest --single unit/type/compression # 13/13 pass
What's NOT in this PR (PR-C3 follows): sweeper engine + COMPRESSION
SWEEP FORCE command. The transient-view drain in master=decompression
is now correct on its own (one sweep would drain the keyspace via the
read path), but the explicit sweeper is the more direct convergence
mechanism documented in R2.1.2 and is the natural pair to this PR.
1e413a8 to
27b03b0
Compare
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-C2 of 3 in the implementation of the declarative master-switch model from PR #28 (PR-B). Builds on PR-C1 (#29) which landed the config plumbing.
This PR implements R2.5.7 — the only intentional cross-mechanism dependency in the design: the read-path transient-view drain mode follows the master switch.
Why this dispatch is needed
Without it, every read-touched key would oscillate between compressed and uncompressed each iteration in
master=decompression— defeating the operator's drain intent. With it, a single sweep across the keyspace plus normal read traffic fully decompresses the dataset. The design doc §2.5.7 calls this out as the only intentional coupling between the three orthogonal mechanics (master switch, sweeper, transient view).Implementation
compressionBeforeSleepnow readsserver.compression_master_switchonce per invocation and dispatches each "slot matches" entry to the appropriate finalize. The "stale slot" branch is mode-independent — when the kvstore no longer references our pinned robj, we always discard (free both buffers + drop pin + free entry struct).The per-entry finalize logic is extracted into three static-inline helpers:
discardTransientEntryrestoreTransientEntrypermanentlyDecompressTransientEntryTest infrastructure
Two new test-only drain variants (alongside the existing
testOnlyCompressionDrainTransientViewAsDiscard):testOnlyCompressionDrainTransientViewAsRestore— simulates the "slot matches + restore mode" branch for every entry.testOnlyCompressionDrainTransientViewAsPermanentDecompress— simulates the "slot matches + permanent-decompress mode" branch for every entry.Both skip the kvstore lookup so unit tests don't need a populated kvstore. Same pattern as the existing
AsDiscardhelper.Test coverage (4 new gtest cases)
RestoreDrainPutsCompressedFormBackencoding=COMPRESSED,val_ptris the original compressed buffer (not a fresh one), refcount drops 2→1, side-map empties.PermanentDecompressDrainReleasesCompressedBufferencoding=RAW, temp sds stays installed asval_ptr(NOT freed; it IS the value now), decompressed bytes round-trip the source, refcount drops 2→1.PermanentDecompressDrainHandlesMultipleEntriesRestoreDrainResetsPerIterationBudgetWhat's NOT in this PR
COMPRESSION SWEEP FORCEcommand — that's PR-C3.The transient-view drain in
master=decompressionis now correct on its own — normal read traffic across the keyspace will permanently decompress everything that gets touched. The explicit sweeper from C3 is the more direct convergence mechanism documented in R2.1.2 and is the natural pair to this PR.Verified locally
Diff stat