Skip to content

feat(csc): metadata-driven command eligibility (CAE-3353) - #3960

Open
ofekshenawa wants to merge 2 commits into
masterfrom
update-cacheable-commands-for-csc
Open

feat(csc): metadata-driven command eligibility (CAE-3353)#3960
ofekshenawa wants to merge 2 commits into
masterfrom
update-cacheable-commands-for-csc

Conversation

@ofekshenawa

@ofekshenawa ofekshenawa commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Note

Medium Risk
Changes which commands are cached and which keys are tracked for invalidation; incorrect metadata or overrides could serve stale reads, though the design fails closed and adds regression tests for known server gaps.

Overview
Replaces the hand-maintained CSC command allow-list with Redis COMMAND metadata: a checked-in generated cscCommandTable (via internal/csccmdgen and //go:generate) drives which read-only commands may be cached, using flags/tips (readonly, script_runner, blocking, nondeterministic_output, dont_cache) and key-spec extraction instead of per-command switches.

cscCommandOverrides layers probe-verified exclusions on top of the generated table (e.g. json.mget, ts.nrange/nrevrange, xinfo|stream/groups, touch, cf.compact) so broken server invalidation cannot re-enable caching. Lookups resolve container subcommands (memory|usage, ft.config|get) and drop bare parent entries that would shadow children.

Key listing for invalidations is generalized from firstKey/lastKey/step and numkeys patterns in metadata, with wire-faithful argument checks so misclassified commands fail closed. Eligibility tests and new e2e cases cover multi-key MGET invalidation and the override guards.

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

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7117e9def0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread csc_command_table.go
"lolwut": {bits: cscFlagReadonly},
"lpos": {bits: cscFlagReadonly | cscHasKeySpec, extract: cscKeyExtractRange, firstKey: 1, lastKey: 1, step: 1},
"lrange": {bits: cscFlagReadonly | cscHasKeySpec, extract: cscKeyExtractRange, firstKey: 1, lastKey: 1, step: 1},
"memory|usage": {bits: cscFlagReadonly | cscHasKeySpec, extract: cscKeyExtractRange, firstKey: 2, lastKey: 2, step: 1},

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Exclude sampled MEMORY USAGE from the cache

MEMORY USAGE samples nested values by default, and any invocation using a nonzero SAMPLES count can return a different estimate for the same unchanged key. Marking it cacheable freezes the first sampled estimate and serves it for every subsequent call until an unrelated key invalidation occurs, changing the command's observable semantics for clients with CSC enabled; treat this command as nondeterministic or only cache the SAMPLES 0 form.

Useful? React with 👍 / 👎.

@ofekshenawa ofekshenawa changed the title feat(csc): metadata-driven command eligibility feat(csc): metadata-driven command eligibility (CAE-3353) Aug 10, 2026
@ofekshenawa
ofekshenawa requested a review from ndyakov August 10, 2026 14:17
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