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

fix(client): MEMORY USAGE must emit SAMPLES when 0#3328
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants