Repository navigation
refactor: route single-key commands in one place - #417
Merged
Merged
Conversation
About 90 single-key commands were mapped to shard requests twice: in prepare_command for pipelines and single commands, and in exec/ for transactions and the special connection modes. The two had drifted. The pipelined path never published keyspace events, and the serial path skipped the key and value size limits. route() now maps each command once for both paths, carries its keyspace event, and the size check runs on both paths.
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.
Single-key commands were turned into shard requests in two places:
prepare_commandindispatch.rshandles pipelines and every single command sent on its own, which is the common case. Each command maps to aShardRequestplus aResponseTag.exec/*.rsfunctions called fromexecutehandle transactions, the special connection modes and the fallback. Each command maps to the sameShardRequestplus its own response function.About 90 commands existed in both, and the copies had drifted:
execcopies. Withnotify-keyspace-eventson, a plainSET,SADD,EXPIRE,HSET,ZADD,LPUSHorRPUSHpublished nothing. Only the same command insideMULTIdid.max-key-len,max-value-len) were only checked inprepare_command. ASETinsideMULTIskipped them.Now:
connection/route.rshas oneroute()that maps each single-key command to aRoute: the shard, request and response tag, plus the keyspace event to publish on success. Both paths call it.executesends the request and resolves it with the sameresolve_shard_response.Notifypublishes when the reply is not an error or nil. For SADD, ZADD, EXPIRE and PEXPIRE it also requires a count above zero, so nothing is published when nothing changed. That matches what theexeccopies did. PEXPIRE now publishesexpire, as in Redis. It published nothing before.processrunsvalidate_command_sizeslikeprepare_command.execute's match, returning an internal error, so the match is still exhaustive and a new command can't compile without a handler. The "vector support not compiled" reply for builds without the feature is unchanged.execfunctions and the response helpers only they used are removed: about 1,500 lines.A new integration test checks that a lone
SETpublishes its keyspace event. Clippy is clean without features, withprotobuf, and withvector. The integration suite passes except the CLI tests, which need theember-clibinary built.