Skip to content

fix(client): MEMORY USAGE must emit SAMPLES when 0 - #3328

Merged
nkaradzhov merged 1 commit into
redis:masterfrom
Develop-KIM:fix/memory-usage-samples-zero
Jul 10, 2026
Merged

nkaradzhov merged 1 commit into
redis:masterfrom
Develop-KIM:fix/memory-usage-samples-zero

Conversation

@Develop-KIM

@Develop-KIM Develop-KIM commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

MEMORY USAGE key SAMPLES 0 was being dropped. The SAMPLES option is guarded by a truthy check:

if (options?.SAMPLES) {
  parser.push('SAMPLES', options.SAMPLES.toString());
}

so an explicit 0 is falsy and never emitted — the command silently degrades to MEMORY USAGE key, which uses the server default of 5 samples.

Per the Redis docs:

By default, this option is set to 5. To sample the all of the nested values, use SAMPLES 0.

So SAMPLES 0 means "scan all nested values" (an exact size) rather than estimating from 5 — observably different for aggregate types (lists/hashes/sets/zsets). Dropping it changes the returned size.

Fix: guard on !== undefined so an explicit 0 is forwarded. This mirrors the recent same-class fix for RESTORE IDLETIME/FREQ 0 (#3323) and XSETID ENTRIESADDED 0 (#3324).

Added a transformArguments test for SAMPLES: 0.


Checklist

  • Does npm test pass with this change (including linting)? — the MEMORY USAGE spec passes (4 tests incl. the new case) and eslint is clean on the changed files.
  • Is the new or changed code fully tested? — added a SAMPLES 0 argument test; it fails on the old truthy guard and passes with !== undefined.
  • Is a documentation update included? — no public API change (existing SAMPLES?: number option); added a JSDoc note documenting the 0 semantics.

Drafted with AI assistance (Claude); reviewed and verified by me. Every line is mine to defend.


Note

Low Risk
Small read-only command argument parsing fix with a regression test; no auth, data mutation, or broad API surface change.

Overview
Fixes MEMORY USAGE so an explicit SAMPLES: 0 is sent to Redis instead of being dropped. The parser used a truthy check on SAMPLES, so 0 never produced SAMPLES 0 and the command fell back to the server default (5 samples), which changes memory estimates for aggregate keys.

The guard is now options?.SAMPLES !== undefined, matching the pattern used for other numeric options that allow 0. MemoryUsageOptions gets a short JSDoc note that 0 means sample all nested values, and a transformArguments test asserts ['MEMORY', 'USAGE', 'key', 'SAMPLES', '0'].

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

`MEMORY USAGE key SAMPLES 0` was guarded by a truthy check
(`if (options?.SAMPLES)`), so an explicit `0` was silently dropped and the
command fell back to the server default of 5 samples.

Per the Redis docs, `SAMPLES 0` samples all nested values (an exact size)
instead of estimating from 5, so the two are observably different for
aggregate types. Guard on `!== undefined` so `0` is forwarded, matching the
recent RESTORE IDLETIME/FREQ 0 fix (redis#3323). Adds a `SAMPLES 0` argument test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nkaradzhov
nkaradzhov merged commit 4875f7b into redis:master Jul 10, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants