Skip to content

feat: add key prefixing (fixes #2982)#3312

Merged
nkaradzhov merged 4 commits into
redis:masterfrom
gajus:gajus/key-prefixing
Jul 1, 2026
Merged

feat: add key prefixing (fixes #2982)#3312
nkaradzhov merged 4 commits into
redis:masterfrom
gajus:gajus/key-prefixing

Conversation

@gajus

@gajus gajus commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Description

Restores key prefixing functionality.

The reasoning is described in #2982


Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

Note

Medium Risk
Prefixing touches almost all command construction and cluster/sentinel routing and WATCH behavior; incorrect edge cases could cause silent wrong-key access or broken optimistic locking, though coverage is broad.

Overview
Adds a keyPrefix client option that prepends a string or Buffer to every key on the wire, with ioredis-style semantics: outbound keys are prefixed, replies are not stripped, SCAN/KEYS MATCH patterns are not auto-prefixed, and Pub/Sub channels (including sharded SPUBLISH) stay unprefixed.

Prefixing is wired through BasicCommandParser (prefixKey / prefixKeys, optional pushKey(..., applyPrefix=false)), and every command path that builds parsers now passes the configured prefix—standalone client, pool, cluster, sentinel, MULTI/pipelines, and WATCH (which previously could watch the wrong key). The pool keeps prefixing at the pool layer and clears keyPrefix on pooled node clients to avoid double-prefixing; cluster multi(routingKey) prefixes the explicit routing key so slot selection matches prefixed command keys.

Several commands were adjusted to use pushKey where keys were previously pushed raw (APPEND, SORT STORE, MIGRATE, CLUSTER KEYSLOT). Docs cover configuration, semantics, and the scanIterator + mGet double-prefix pitfall; integration tests cover transactions, scripts, WATCH, pool/cluster/sentinel, and parser unit tests.

Reviewed by Cursor Bugbot for commit bef1784. Bugbot is set up for automated code reviews on this repo. Configure here.

@jit-ci

jit-ci Bot commented Jun 17, 2026

Copy link
Copy Markdown

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

@gajus

gajus commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Would appreciate guidance @nkaradzhov on the next steps

@nkaradzhov

Copy link
Copy Markdown
Collaborator

@gajus i will try to review soon, as far as I remember though, there were some complications of having this, besides, there is a fairly simple workaround? Anyway, I will need to thoroughly look at this

@gajus

gajus commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

there is a fairly simple workaround

I am not aware of any simple workarounds.

@nkaradzhov nkaradzhov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  1. WATCH doesn't get the prefix

WATCH builds its request differently from normal commands and skips prefixing. So it watches the unprefixed key while the commands inside the transaction use the prefixed key — it guards a key nobody writes to, and optimistic locking silently breaks with no error. Same through Sentinel.

const client = createClient({ keyPrefix: 'app:' });
await client.watch('k');             // watches "k"        (unprefixed)
const m = client.multi().get('k');   // operates on "app:k" (prefixed)
// concurrent writer changes "app:k" -> EXEC does NOT abort; WATCH guarded wrong key
await m.exec();
  1. Cluster multi routes on the unprefixed key

An explicit routing key passed to cluster.multi(routing) isn't prefixed, but the commands inside are — so the cluster picks the node from the unprefixed key while the data lives elsewhere → MOVED/cross-slot. Only with a prefix + explicit routing key, small clean fix:

const cluster = createCluster({ rootNodes, keyPrefix: 'app:' });
const multi = cluster.multi('user');   // routing "user" -> slot from "user"
multi.set('user', '1');                // command key prefixed -> "app:user" (different slot)
await multi.exec();                    // routed by slot('user') -> MOVED / CROSSSLOT

@gajus

gajus commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@nkaradzhov resolved

@nkaradzhov

Copy link
Copy Markdown
Collaborator

@PavelPashov can you please take a look as well please!

@PavelPashov

Copy link
Copy Markdown
Contributor

@gajus Consider using parser.pushKey(key) in CLUSTER KEYSLOT as it reports the slot for the prefixed key, matching where prefixed commands are actually routed.

@PavelPashov PavelPashov 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.

Please document this scanIterator gotcha explicitly: it yields already-prefixed keys, so passing them back to key-prefixing commands will prefix them again unless callers strip the prefix first.

@gajus

gajus commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Both done

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit bef1784. Configure here.

Comment thread packages/client/lib/sentinel/index.ts
@nkaradzhov nkaradzhov dismissed stale reviews from PavelPashov and themself July 1, 2026 08:08

done

@nkaradzhov nkaradzhov merged commit 7ae4652 into redis:master Jul 1, 2026
14 checks passed
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.

3 participants