Rework Redis command construction logic. - #2847
Merged
Merged
Conversation
michael-grunder
force-pushed
the
refactor.redis-cmd
branch
4 times, most recently
from
May 28, 2026 20:31
79bc791 to
8911299
Compare
Member
Author
|
@yatsukhnenko I won't merge this before AB testing it for backward compatibility issues but it drastically reduces the boilerplate in building commands. It also fixes a many years long memory leak pathway (failing halfway through a transaction) because previously the wrapper method didn't know how to free the One note, the reason I am passing |
yatsukhnenko
approved these changes
May 31, 2026
* Introduce a new `RedisCmd` struct to dynamically append RESP arguments such that we don't have to precalculate the number of arguments the command will have up front. Additionally the new `RedisCmd allows both a `void *` context pointer but also can attach a `void (*ctx_dtor)(void*)` destructor so we are still able to clean up any allocated context when commands fail. This moves the context cleanup out of every individual reply handler and into the generic processing wrappers. * Create a small group of `resp_str` helper functions for lower level concatination of RESP protocol data over the wire. * Lots of small modernization of the codebase such as using `zend_string*` instead of (`char *`, `size_t`) pairs. * Greatly simplify `crosslot` handling logic
michael-grunder
force-pushed
the
refactor.redis-cmd
branch
from
June 1, 2026 19:06
8911299 to
b793366
Compare
Co-authored-by: Pavlo Yatsukhnenko <yatsukhnenko@users.noreply.github.com>
Co-authored-by: Pavlo Yatsukhnenko <yatsukhnenko@users.noreply.github.com>
Co-authored-by: Pavlo Yatsukhnenko <yatsukhnenko@users.noreply.github.com>
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
3 participants
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.
Introduce a new
RedisCmdstruct to dynamically append RESP arguments such that we don't have to precalculate the number of arguments the command will have up front.Additionally the new
RedisCmd allows both avoid *context pointer but also can attach avoid (ctx_dtor)(void)` destructor so we are still able to clean up any allocated context when commands fail. This moves the context cleanup out of every individual reply handler and into the generic processing wrappers.Create a small group of
resp_strhelper functions for lower level concatination of RESP protocol data over the wire.Lots of small modernization of the codebase such as using
zend_string*instead of (char *,size_t) pairs.Fix: Properly apply any
Redis::OPT_PREFIXto both theSORTkey and theSTOREkey in the sort command.