Repository navigation
Conversation
The single-server set_multi path encoded keys in the protocol layer without the b flag and applied the namespace after encoding, so keys with whitespace or non-ASCII characters were stored under their base64 text, where get and get_multi never find them. Non-String keys raised NoMethodError. PipelinedSetter now validates keys with the client's KeyManager (to_s, namespace, truncation) before the request, like PipelinedGetter and PipelinedDeleter, and write_multi_storage_req only encodes the key and sends b. The per-server KeyManager that existed only for this path is removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Single-server
set_multi(thePipelinedSetter→write_multi_storage_reqfast path) built keys differently from every other operation:KeyRegularizer.encode, so keys containing whitespace or non-ASCII characters were stored under their base64 text.getandget_multinever find them there.to_s, so Symbol and Integer keys raisedNoMethodError.Multi-server rings were unaffected because they fall back to
quiet { set }.This moves key validation into
PipelinedSetter, using the client'sKeyManagerin the same way asPipelinedGetterandPipelinedDeleter.write_multi_storage_reqnow only encodes the key and sendsb. The per-serverKeyManageradded toProtocol::Basefor this path had no other users and is removed.Intended behaviour:
set_multiwrites to the same keys thatset,get, andget_multiuse, for any ring size, with or without a namespace. Items already written under base64 text are never read and expire normally.set_multiaccepts the same non-String keys asset.ArgumentErrorbefore any I/O, as withset. PreviouslynilraisedNoMethodError, and a blank key failed inside the request, which closed the connection.Not in scope: keys that need base64 are still limited by
KeyManagertruncating at 250 raw characters, while memcached caps the encoded key at 250 characters. That will be addressed separately.Testing
test_namespace_and_key.rbcallset_multi, thengetandget_multi, on 1- and 2-server rings, with and without a namespace, for plain, space, tab, and non-ASCII keys. They also cover Symbol and Integer keys on both ring sizes. The three single-server cases fail onmain.set_multinamespace tests from Fix set multi namespace on single server #47; the one named "multiple servers" ran on a single server without a namespace. The single-server concurrency test uses a namespace again, and its stale bug note is removed.test_rack_session.rb, which does not load on Ruby 4.0 with or without this change (rack 2.2 requires the removedcgi/cookie).