Add session locking support to RedisCluster#2836
Open
rveznaver wants to merge 1 commit into
Open
Conversation
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
1 participant
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.
Inspired by #2738, I started wondering if it would be possible to add support for session locking into RedisCluster.
I started thinking about distributed locks... mostly if I could avoid them at all if possible. So my idea was to couple the session with its lock and make the write and reads atomic (or a transaction in relational DB terms), and for that I would need to put them both on the same node. Luckily, Redis supports this with hash tags. (Funnily enough, after I started working and digging about it I later found out that someone already had the same idea.)
Now, the last time I have written anything in C was a very long time ago, but I do believe I remember enough to read it. I have utilised both Claude Opus 4.7 extra high thinking and GPT 5.5 extra high thinking to help me in this endeavour, and then proceeded to manually review every single function it added. You may notice that most of the locking functions read as if they are a mirror of the standalone session handling code. This is on purpose as I copied them over before adapting them to work on the cluster, and even went so far as to define the
REDIS_CLUSTER_SPPRINTFmacro inlibrary.hto mirror theREDIS_SPPRINTFone. So if you are used to reading the session handling code, this should be functionally identical.The main deviations from the standalone session flow are:
I have left out the separate docker tests the LLMs have built as I do not see any in this repo and did not really have time to clean up and review those as closely as I have the code.
I am also very sorry for this behemoth of a pull request, I really tried to keep it as minimal and as close to current code as possible for a possible review.