Skip to content

string: avoid storing already-expired MSETEX values#3534

Open
charsyam wants to merge 1 commit intovalkey-io:unstablefrom
charsyam:fix/msetex-expired-fastpath-unstable
Open

string: avoid storing already-expired MSETEX values#3534
charsyam wants to merge 1 commit intovalkey-io:unstablefrom
charsyam:fix/msetex-expired-fastpath-unstable

Conversation

@charsyam
Copy link
Copy Markdown
Contributor

This changes MSETEX to avoid materializing values whose expiration time is already in the past.

The existing msetexCommand code already acknowledged this as a TODO:

"If the milliseconds have expired, we can actually avoid setting the keys
in the database, just like we do when handling the SET command. Like we
could rewrite it to DEL xxx xxx, but it will make the code more complicated
and it seems excessive. Currently we will add it to the database and wait
for either active expire or lazy expire to delete it."

This PR takes the "more complicated" path and mirrors setGenericCommand's
existing fast-path (t_string.c:130). For a large multi-key MSETEX with a
past PXAT, this avoids N × setKey + setExpire allocations, two keyspace
notifications per key (set + expire), and the subsequent active/lazy
expire work.

What changed

  • Add an already-expired fast path to MSETEX in msetexCommand.
  • Preserve NX / XX semantics by evaluating them before the fast path.
  • When any input keys already exist, delete them in place and propagate a
    single consolidated DEL / UNLINK k1 k2 ... kN (not per-key).
  • When no input keys exist (pure no-op), explicitly call
    preventCommandPropagation() instead of relying on dirty == 0, so the
    suppression intent survives future changes to call()'s propagation logic.
  • stat_expiredkeys is incremented once per key that was actually deleted,
    matching how SET's fast-path treats the same counter.

Tests

Under tests/unit/type/string.tcl:

  • Replaces the previous MSETEX with past EXAT/PXAT - key stored but logically expired test, which locked in the pre-fix behavior, with
    ... - keys are not stored.
  • Adjusts the MSETEX lazy/active expire with all expiration options
    tests: key3/key4 (past EXAT/PXAT) now hit the fast path and never
    materialize, so the expected UNLINK / expired_keys counts drop from 4
    to 2.
  • Adds three new tests:
    • MSETEX with past PXAT deletes existing keys and propagates as a single UNLINK — verifies the consolidated multi-key DEL/UNLINK path.
    • MSETEX with past PXAT and no existing keys does not propagate
      uses a sentinel SET to verify the no-op path doesn't leak to
      replicas or the AOF.
    • (Valkey-side only) Tests fix lazyfree-lazy-expire to its default
      so later tests aren't sensitive to ordering.

Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.41%. Comparing base (4a42c95) to head (63941ed).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3534      +/-   ##
============================================
+ Coverage     76.40%   76.41%   +0.01%     
============================================
  Files           159      159              
  Lines         79851    79873      +22     
============================================
+ Hits          61008    61035      +27     
+ Misses        18843    18838       -5     
Files with missing lines Coverage Δ
src/t_string.c 97.46% <100.00%> (+0.29%) ⬆️

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant