Skip to content

Fix multi-server get_multi stopping at an empty value - #1170

Merged
petergoldstein merged 4 commits into
petergoldstein:mainfrom
radixdev:fix/empty-value-get-multi
Oct 2, 2026
Merged

petergoldstein merged 4 commits into
petergoldstein:mainfrom
radixdev:fix/empty-value-get-multi

Conversation

@radixdev

Copy link
Copy Markdown
Contributor

Summary

On a multi-server ring, a get_multi that includes a key whose value is empty stops reading that key's server at that key. The rest of that server's keys are silently missing from the result. The unread replies stay on the connection, and with larger values they corrupt the next command on it.

This PR parses a zero-length hit (VA 0) as an empty value instead. It's a one-line change in lib/.

Cause

ResponseProcessor#getk_response_from_buffer treats any header whose size is zero as a reply with no body:

return [true, header_len] if body_len.zero?

That covers the terminating MN and error lines, but it also catches VA 0 ... s0. Base#pipeline_next_responses then sees an OK status with no key, which is how it recognizes the MN, and calls finish_pipeline. The VA 0 reply's own terminator and the replies after it are never parsed:

  • Anything already read into the buffer is cleared, so those keys are missing from the result.
  • Anything not read yet stays on the socket and is taken as the reply to the next request.

Reproduction

On main, with 2 memcached servers, 200 keys, and key5 set to '' via set('key5', '', 0, raw: true):

client value size get_multi returned afterwards
raw: true 100B 102 / 200 later gets correct
raw: true 20KB 102 / 200 the next get raises Dalli::DalliError: Response error: 8k18k18k1… (value bytes read as a reply)
default (Marshal) 100B / 20KB 102 / 200 later gets correct
serializer: JSON 100B / 20KB 102 / 200 later gets correct

With this PR, every row returns 200 of 200, with key5 as '', and every later get returns the right value.

A Marshal or JSON client never writes an empty value itself. It still reads one, though, if another client, another language, or a raw: true write stored one. A single-key get and a single-server get_multi already return '' for these.

The logic dates from the meta protocol's pipelined getter in 3.2.0. It was opt-in until 5.0.0, where it became the only protocol.

Change

getk_response_from_buffer keeps the no-body result for headers other than VA. A VA 0 now goes through the normal path: the response size includes the empty body's terminator, it returns [0] until that terminator has arrived, and the value is '' after retrieve.

Testing

  • bundle exec rake: 848 runs, 0 failures, 0 errors.
  • New tests. All three fail on main and pass with this change.
    • ResponseProcessor: a VA 0 hit returns the key, '' and a size that includes the terminator.
    • ResponseProcessor: a VA 0 whose terminator hasn't arrived yet returns [0].
    • Integration, on a real 2-server ring with raw: true: 100 keys of 20KB with one empty value. The hash and block forms of get_multi return every key, and a get of every key afterwards returns the right value.
  • bundle exec rubocop: no offenses.

This is independent of #1169, which leaves zero sizes on the existing token path. Whichever lands second needs a trivial rebase.

This PR was generated with the assistance of Claude Code (Anthropic). The reproduction and test results above come from real runs.

🤖 Generated with Claude Code

radixdev and others added 2 commits September 29, 2026 11:42
getk_response_from_buffer returned the no-body result for any header
with a zero size, which the pipelined getter reads as the terminating
MN. A VA 0 hit therefore ended the server's pipeline early, dropping
its remaining keys and leaving the rest of its replies unread on the
connection. Parse a VA 0 as an empty value instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@radixdev
radixdev marked this pull request as ready for review September 29, 2026 16:02
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@petergoldstein
petergoldstein merged commit c6b32b5 into petergoldstein:main Oct 2, 2026
28 of 29 checks passed
@petergoldstein

Copy link
Copy Markdown
Owner

Thanks, merged. While testing I found the impact goes beyond missing keys: on main, after a truncated get_multi, later single-key gets on the same connection could silently return another key's value (e.g. get('key1') returned key184's). I've expanded the changelog entry to say so.

This is shipping in 5.2.0, plus backports in 5.1.2, 5.0.8, 4.3.5 and 3.2.11 (the meta protocol has had this since 3.2.0). CI was also failing on every PR because the runner images' package index went stale for libevent. That's fixed in #1172.

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