Skip to content

Fix flaky CanDoSortedSetExpireLTM by forcing eviction with FlushAndEvict#1982

Open
tiagonapoli wants to merge 3 commits into
mainfrom
tiagonapoli/fix-flaky-sortedset-expire-ltm
Open

Fix flaky CanDoSortedSetExpireLTM by forcing eviction with FlushAndEvict#1982
tiagonapoli wants to merge 3 commits into
mainfrom
tiagonapoli/fix-flaky-sortedset-expire-ltm

Conversation

@tiagonapoli

@tiagonapoli tiagonapoli commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

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 ZEXPIRE and the first checkpoint was fill_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:

  • Call 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).
  • With no fill, there is no CPU-dependent work between setting the TTLs and the checks — the only elapsed time is the fixed Task.Delay, so the flakiness is removed at the root rather than papered over with larger timeouts.
  • FlushAndEvict is called again before each checkpoint so every read is served from disk, still exercising the on-disk TTL deserialization path (SortedSetObject serialize/deserialize) that this test exists to cover.
  • TTLs lowered to 2s / 4s (the fill is gone, so no time budget is needed to absorb it), and a dedicated non-expiring key verifies that unaffected data survives intact.

Note: adding more members to a single sorted set would not work — that grows one record in place and never evicts the TTL-bearing records. Eviction requires pushing those records below HeadAddress, which FlushAndEvict does directly.

Validation

Run in a CPU-throttled container: no failures across 37 runs at 0.25 CPU and 29 runs at full CPU.

Fixes #1981

Copilot AI review requested due to automatic review settings July 23, 2026 22:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ZEXPIRE calls 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.

@tiagonapoli
tiagonapoli force-pushed the tiagonapoli/fix-flaky-sortedset-expire-ltm branch from 5c21788 to e5c70ad Compare July 24, 2026 01:05
@tiagonapoli tiagonapoli changed the title Fix flaky CanDoSortedSetExpireLTM by setting field expirations after the LTM fill Fix flaky CanDoSortedSetExpireLTM by anchoring waits to expiry instants Jul 24, 2026
@tiagonapoli
tiagonapoli force-pushed the tiagonapoli/fix-flaky-sortedset-expire-ltm branch 3 times, most recently from d3224cf to 60847a2 Compare July 24, 2026 01:38
@tiagonapoli tiagonapoli changed the title Fix flaky CanDoSortedSetExpireLTM by anchoring waits to expiry instants Fix flaky CanDoSortedSetExpireLTM by polling for expiry instead of fixed delays Jul 24, 2026
@tiagonapoli
tiagonapoli force-pushed the tiagonapoli/fix-flaky-sortedset-expire-ltm branch 3 times, most recently from 2cacc8c to 09b8c5e Compare July 24, 2026 04:28
@tiagonapoli tiagonapoli changed the title Fix flaky CanDoSortedSetExpireLTM by polling for expiry instead of fixed delays Fix flaky CanDoSortedSetExpireLTM via fill-anchored expiry waits Jul 24, 2026
@tiagonapoli
tiagonapoli force-pushed the tiagonapoli/fix-flaky-sortedset-expire-ltm branch from 09b8c5e to 14efe20 Compare July 24, 2026 06:40
@tiagonapoli tiagonapoli changed the title Fix flaky CanDoSortedSetExpireLTM via fill-anchored expiry waits Fix flaky CanDoSortedSetExpireLTM by forcing eviction with FlushAndEvict Jul 24, 2026
…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
tiagonapoli force-pushed the tiagonapoli/fix-flaky-sortedset-expire-ltm branch from 14efe20 to d699492 Compare July 24, 2026 15:35
Tiago Martins Napoli and others added 2 commits July 24, 2026 09:41
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: RespSortedSetTests.CanDoSortedSetExpireLTM races TTL against CPU-dependent LTM fill

3 participants