Fix flaky CanDoSortedSetExpireLTM by forcing eviction with FlushAndEvict#1982
Open
tiagonapoli wants to merge 3 commits into
Open
Fix flaky CanDoSortedSetExpireLTM by forcing eviction with FlushAndEvict#1982tiagonapoli wants to merge 3 commits into
tiagonapoli wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes CI flakiness in RespSortedSetTests.CanDoSortedSetExpireLTM by moving ZEXPIRE calls to occur after the larger-than-memory (LTM) fill loop, preventing the TTL budget from being consumed by CPU-dependent setup time.
Changes:
- Removed per-member
ZEXPIREcalls from the initial key-setup loops. - Added a post-LTM-fill block that applies the field expirations immediately before the test’s delay-and-assertion sequence.
- Documented the timing rationale inline (referencing #1981) to prevent regressions.
badrishc
approved these changes
Jul 23, 2026
tiagonapoli
force-pushed
the
tiagonapoli/fix-flaky-sortedset-expire-ltm
branch
from
July 24, 2026 01:05
5c21788 to
e5c70ad
Compare
tiagonapoli
force-pushed
the
tiagonapoli/fix-flaky-sortedset-expire-ltm
branch
3 times, most recently
from
July 24, 2026 01:38
d3224cf to
60847a2
Compare
tiagonapoli
force-pushed
the
tiagonapoli/fix-flaky-sortedset-expire-ltm
branch
3 times, most recently
from
July 24, 2026 04:28
2cacc8c to
09b8c5e
Compare
tiagonapoli
force-pushed
the
tiagonapoli/fix-flaky-sortedset-expire-ltm
branch
from
July 24, 2026 06:40
09b8c5e to
14efe20
Compare
…ict (#1981) Root cause: the test set field TTLs, then ran a 1000-key fill to push the records to disk before asserting expiry. The fill is ~1000 serial round-trips whose duration scales inversely with available CPU, so under CPU pressure it consumed most of the large field's TTL and the field expired before its "still alive" assertion. Fix: replace the fill with a deterministic Store.Log.FlushAndEvict(wait: true), an idiom already used across the test suite. This flushes the records to disk in one synchronous call with no CPU-dependent work between setting the TTLs and the checks, so the only elapsed time is the fixed Task.Delay. FlushAndEvict is called again before each checkpoint so every read is served from disk, still exercising the on-disk TTL deserialization path this test covers. TTLs are lowered to 2s/4s and a dedicated non-expiring key verifies unaffected data survives. Validated with no failures across 37 runs at 0.25 CPU and 29 runs at full CPU. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tiagonapoli
force-pushed
the
tiagonapoli/fix-flaky-sortedset-expire-ltm
branch
from
July 24, 2026 15:35
14efe20 to
d699492
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2742bd39-5499-4d09-a6f3-3469447f77d8
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2742bd39-5499-4d09-a6f3-3469447f77d8
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.
Problem
RespSortedSetTests.CanDoSortedSetExpireLTM (#1981) is flaky under CPU pressure and was observed failing in CI.
Root cause
The test sets per-field TTLs (small/large), then runs a 1000-key fill to force the records to spill to disk, and only afterward asserts that the small field has expired while the large field is still alive.
The fill is ~1000 sequential client→server round-trips, so its wall-clock duration scales inversely with available CPU. The original code added a fixed Task.Delay after the fill, so the elapsed time between
ZEXPIREand the first checkpoint wasfill_time + delay. Under CPU pressure that exceeded the large field's TTL, so the large field legitimately expired before its "still alive" assertion — a test-timing bug, not a product bug.Fix
Replace the fill with a deterministic eviction:
server.Provider.StoreWrapper.store.Log.FlushAndEvict(wait: true)to flush the records to disk in one synchronous call. This is an idiom already used across the test suite (RespRangeIndexTests,CacheSizeTrackerTests,GarnetObjectTests).Task.Delay, so the flakiness is removed at the root rather than papered over with larger timeouts.FlushAndEvictis called again before each checkpoint so every read is served from disk, still exercising the on-disk TTL deserialization path (SortedSetObjectserialize/deserialize) that this test exists to cover.Validation
Run in a CPU-throttled container: no failures across 37 runs at 0.25 CPU and 29 runs at full CPU.
Fixes #1981