Skip to content

fix(provider-redis): let purges find entries when useRedisSets is off - #245

Merged
derrickmehaffy merged 2 commits into
mainfrom
fix/redis-keys-without-sets
Oct 13, 2026
Merged

fix(provider-redis): let purges find entries when useRedisSets is off#245
derrickmehaffy merged 2 commits into
mainfrom
fix/redis-keys-without-sets

Conversation

@derrickmehaffy

@derrickmehaffy derrickmehaffy commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

What changed and why

With useRedisSets: false in the redis provider's options, purges never removed anything.

  • @keyv/redis 3 keeps no tracking set in that mode and names each entry sets:namespace:keyv:keyv:<key>.
  • keys() still read the tracking set, got an empty list, and returned [].
  • Every pattern purge, which is how the plugin invalidates on content changes, matches against keys(), so it matched nothing. Entries lived until maxAge.

Fix: in that mode, keys() finds entries by the name @keyv/redis gives them, using SCAN, and returns them unqualified, as before.

  • On a cluster it scans every master, because SCAN only sees the node it's sent to.
  • Deleting already worked (delMany → @keyv/redis delete names keys correctly). Only enumeration was broken.

With redis sets on, the default, nothing changes.

Tests

  • New scripts/redis-standalone-live.cjs (pnpm run test:redis:live, runs in e2e_redis): writes entries, lists them with keys(), purges a pattern, and checks what's left, both with and without redis sets.
    • On main, both no-sets checks fail (keys() returns [], and the pattern matched nothing).
    • With the fix, 4 of 4 pass.
  • On a real three-master cluster (local, ad hoc): 20 entries spread 5/10/5 across the masters. keys() found all 20, and the purge left 0.

Locally:

  • test:lint green, test:smoke 3/3, test:cluster 5/5, test:deps ok.
  • test:e2e:redis 135/135.

Checklist

  • Tests added. A spec that fails before the change.
  • pnpm run test:lint passes.
  • Touches a provider: test:e2e:redis, test:smoke, test:cluster run.
  • Conventional commit subjects, branched from main.

Notes for the reviewer

@derrickmehaffy
fix(provider-redis): let purges find entries when useRedisSets is off
3ac936a 
With useRedisSets off, @keyv/redis keeps no tracking set and names every
entry `sets:namespace:keyv:keyv:<key>`. keys() still read the tracking set,
got nothing, and so every pattern purge - the plugin's whole invalidation
path - matched nothing; entries survived until maxAge.

keys() now finds those entries by the name @keyv/redis gives them, with
SCAN, on every master when the client is a cluster, since SCAN only sees
the node it is sent to. Configurations with redis sets on, the default,
are unaffected.

scripts/redis-standalone-live.cjs covers both modes against a real Redis;
without redis sets both checks fail on main. It runs in e2e_redis.
@derrickmehaffy
test(provider-redis): cover purging without redis sets on a live cluster
e27aabf 
Enumeration without redis sets has to SCAN every master. The live cluster
run now asserts entries spread across masters, keys() finds all of them,
and purging what it lists leaves nothing; both checks fail without the fix.
@derrickmehaffy
derrickmehaffy force-pushed the fix/redis-keys-without-sets branch from 3ababc9 to e27aabf Compare October 13, 2026 00:21
@derrickmehaffy
derrickmehaffy merged commit 346272d into main Oct 13, 2026
13 checks passed
@derrickmehaffy
derrickmehaffy deleted the fix/redis-keys-without-sets branch October 13, 2026 00:25
@github-actions github-actions Bot mentioned this pull request Oct 13, 2026
Open
derrickmehaffy added a commit that referenced this pull request Oct 13, 2026
@derrickmehaffy
Merge origin/main into dev/plugin-redis-v3
16f8d48 
Brings in the redis provider fixes from #244, #245 and #249. The plugin-redis
v2 path is main's as merged: makeClusterSafe, the SREM-before-UNLINK cluster
delete, the background tracking-set prune, and keys() scanning every master
when useRedisSets is off.

The constructor now builds @keyv/redis only when there is no node-redis
connection, as this branch did before, so the v3 path never wraps the ioredis
shim. The v3 store itself is unchanged here; porting the same fixes to it
follows separately so each one can be shown failing first. Until then
test:continuity fails its two useRedisSets: false checks, because v2 now
enumerates those entries and v3 does not.

The lockfile is main's plus this branch's two devDependencies.
derrickmehaffy added a commit that referenced this pull request Oct 13, 2026
@derrickmehaffy
test(provider-redis): hold the v3 store to the #244, #245 and #249 ch…
3afb156 
…ecks

The merged fixes have live and recording-client checks for the plugin-redis
v2 path only. The node-redis store this branch adds has the same three
problems, and nothing exercised them:

- on a cluster its delete() and unlinkTracked() UNLINK before they SREM, so a
  purge racing a write leaves a live entry outside the tracking set (#244);
- nothing prunes its tracking set, so expired members are listed by keys()
  and the set grows for as long as the cache runs (#249);
- with useRedisSets off, keys() reads the empty tracking set, so pattern
  purges match nothing (#245).

redis-standalone-live and redis-cluster-live now also drive the provider on a
real node-redis client (createClient, createCluster), constructed the way
init() does on v3. The cluster harness runs every check against both
backends. redis-cluster-check's node-redis recorder gains EXISTS, SSCAN,
per-master clients and an SREM hook, for the command-order, prune and
client-swap assertions. redis-key-continuity now requires keys() to list
every entry the other backend wrote, not only to agree with it, and runs
against a cluster when REDIS_CLUSTER_NODES is set, which CI's redis_cluster
job now does.

The new v3 checks fail at this commit; the fixes follow. The prune-race
checks also assert that a SREM was issued, since a prune that does nothing
passes them otherwise.
derrickmehaffy added a commit that referenced this pull request Oct 13, 2026
@derrickmehaffy
fix(provider-redis): find v3 entries by name when redis sets are off
c28acd8 
With useRedisSets: false there is no tracking set: entries are named
`sets:namespace:keyv:keyv:<key>`, as @keyv/redis 3 names them. #245 made the
v2 path's keys() find them by that name, on every master of a cluster. The
v3 store's keys() still read the empty tracking set, so every pattern purge
matched nothing and entries outlived it until maxAge - and after #245, v2
and v3 listed different keys for the same data, which test:continuity
caught.

NodeRedisStore.namedKeys() SCANs for the `sets:` names. SCAN only sees the
node it is sent to, so on a cluster it walks `masters` through nodeClient(),
the @redis/client 5+ API, and refuses rather than scan one node when a
client lacks them. clear() uses it too, unlinking key by key on a cluster
instead of a multi-key UNLINK, and SCAN instead of KEYS, which blocks the
server for the whole keyspace.
derrickmehaffy added a commit that referenced this pull request Oct 13, 2026
@derrickmehaffy
docs(redis): describe plugin-redis v3, and listing without redis sets
f834063 
The guide still said that with useRedisSets off the provider falls back to a
SCAN that misses entries on a cluster. Since #245 it SCANs every master, so
purges are complete; the reason left to keep the tracking set is cost, and
the warning now says that.

A new section covers plugin-redis v3: how the provider detects it, that it
runs on `.native` and follows a client swap, that entries survive an upgrade
and a rollback, and that v2 - and with it Node 20 - is still supported.
@derrickmehaffy derrickmehaffy mentioned this pull request Oct 13, 2026
Draft
2 of 7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@derrickmehaffy