Enable multi-command parsing for replicated clients#3597
Open
enjoy-binbin wants to merge 2 commits intovalkey-io:unstablefrom
Open
Enable multi-command parsing for replicated clients#3597enjoy-binbin wants to merge 2 commits intovalkey-io:unstablefrom
enjoy-binbin wants to merge 2 commits intovalkey-io:unstablefrom
Conversation
Multi-command parsing in parseMultibulkBuffer (introduced valkey-io#2092) was disabled for replicated clients because the per-command replication offset relied on c->qb_pos as the right boundary of the just-applied command. Replication stream is actually a big pipeline, so if it can be supported, the processing speed of the replica can be improved. Decouple parsing position from application position by recording, for every parsed command, the qb_pos snapshot taken right after that command finished parsing: - parsedCommand.qb_end_pos: snapshot stored in the queue entry, set by parseMultibulkBuffer when a command is pushed into cmd_queue. - client.qb_applied: snapshot of the command currently being processed, set by the parsers (for c->argv) and by consumeCommandQueue (when popping a queued command). commandProcessed() now uses c->qb_applied instead of c->qb_pos to advance reploff by exactly the bytes of the just-applied command. beforeNextClient shifts qb_end_pos of pending queue entries when the replicated client's querybuf is trimmed, keeping subsequent reploff updates consistent. Both fields are populated unconditionally so that a client transitioning to replicated mid-command (e.g. SYNCSLOTS ESTABLISH installs slot_migration_job inside its own handler) still has a valid value when commandProcessed() runs. Signed-off-by: Binbin <binloveplay1314@qq.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #3597 +/- ##
============================================
- Coverage 76.66% 76.63% -0.03%
============================================
Files 160 160
Lines 80472 80483 +11
============================================
- Hits 61690 61678 -12
- Misses 18782 18805 +23
🚀 New features to boost your workflow:
|
| c->parsed_cmd = p->cmd; | ||
| c->slot = p->slot; | ||
| /* Restore qb_pos snapshot so commandProcessed() can update reploff precisely. */ | ||
| c->qb_applied = p->qb_end_pos; |
There was a problem hiding this comment.
we guard the read on isReplicatedClient do we need to guard the write?
Member
Author
There was a problem hiding this comment.
We can't guard the write, we need to maintain the write everytime since we have a CLUSTER SYNCSLOTS ESTABLISH command, this command will turn the client into a replicated client in the mid run.
Signed-off-by: Binbin <binloveplay1314@qq.com>
jjuleslasarte
approved these changes
May 1, 2026
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.
Multi-command parsing in parseMultibulkBuffer (introduced #2092) was
disabled for replicated clients because the per-command replication
offset relied on c->qb_pos as the right boundary of the just-applied
command. Replication stream is actually a big pipeline, so if it can
be supported, the processing speed of the replica can be improved.
Decouple parsing position from application position by recording, for
every parsed command, the qb_pos snapshot taken right after that
command finished parsing:
by parseMultibulkBuffer when a command is pushed into cmd_queue.
processed, set by the parsers (for c->argv) and by
consumeCommandQueue (when popping a queued command).
commandProcessed() now uses c->qb_applied instead of c->qb_pos to
advance reploff by exactly the bytes of the just-applied command.
beforeNextClient shifts qb_end_pos of pending queue entries when the
replicated client's querybuf is trimmed, keeping subsequent reploff
updates consistent.
Both fields are populated unconditionally so that a client transitioning
to replicated mid-command (e.g. SYNCSLOTS ESTABLISH installs slot_migration_job
inside its own handler) still has a valid value when commandProcessed() runs.