Implement defragmentation for pubsub kvstore#13058
Merged
oranagra merged 33 commits intoredis:unstablefrom Mar 4, 2024
Merged
Conversation
oranagra
reviewed
Feb 18, 2024
shahsb
approved these changes
Feb 18, 2024
oranagra
reviewed
Feb 21, 2024
the reason for returning NULL is that we don't need to update the value of clients dictionary when `dictDefragTables(clients)` is NULL.
oranagra
reviewed
Feb 25, 2024
Member
oranagra
left a comment
There was a problem hiding this comment.
i wonder if we now need a better test? (one that grantees we really did re-locate something in pubsub) or do you think the one we have is sufficient?
Co-authored-by: oranagra <[email protected]>
sundb
commented
Feb 26, 2024
without timeout Fix an overlook in redis#11695, if defragment doesn't reach the end time, we should wait for the current db's keys and expires to finish before leaving, now it's possible to exit early when the keys are defragmented.
sundb
commented
Feb 27, 2024
Before this PR we are always interating from stage 0 to 3, but now we're starting from the last stage, so when defrag_stage is not equal i, it means that the last stage isn't finished, so there is no point in iterating further.
Collaborator
Author
|
@oranagra recent changes from last review:
|
oranagra
reviewed
Mar 2, 2024
sundb
commented
Mar 3, 2024
Co-authored-by: oranagra <[email protected]>
oranagra
approved these changes
Mar 4, 2024
Collaborator
Author
|
@oranagra It's ready. |
sundb
added a commit
that referenced
this pull request
Jun 18, 2024
… reset (#13315) this PR fixes two crashes: 1. Fix missing slotToKeyInit() when using `flushdb async` under cluster mode. #13205 2. Fix missing expires_cursor reset when stopping active defrag in the middle of defragment. #13307 If we stop active defrag in the middle of defragging db->expires, if `expires_cursor` is not reset to 0, the next time we enable active defrag again, defragLaterStep(db, ...) will be entered. However, at this time, `db` has been reset to NULL, which results in crash. The affected code were removed by #11695 and #13058 in usntable, so we just need backport this to 7.2.
This was referenced Aug 7, 2025
sundb
added a commit
that referenced
this pull request
Aug 14, 2025
Fix #13612 This bug was introduced by #11465 This PR added the incremental defragmentation for db->expires. If the time for the defragmentation cycle has not arrived, it will exit unless both cursor and expires_cursor are 0, meaning both keys and expires have been fragmented. However, the expires_cursor check has now been overlooked, which leads to we will exit immediately even if the defragmentation doesn't reach the end time, which will cause the efficiency of defragmentation to become very low. Note that this bug was already fixed in version 7.4(#13058)
sundb
added a commit
that referenced
this pull request
Sep 7, 2025
This PR fixes two defrag issues. 1. Fix a use-after-free issue caused by updating dictionary keys after a pubsub channel is reallocated. This issue was introduced by #13058 1. Fix potential use-after-free for lua during AOF loading with defrag This issue was introduced by #13058 This fix follows #14319 This PR updates the LuaScript LRU list before script execution to prevent accessing a potentially invalidated pointer after long-running scripts.
funny-dog
pushed a commit
to funny-dog/redis
that referenced
this pull request
Sep 17, 2025
After redis#13013 ### This PR make effort to defrag the pubsub kvstore in the following ways: 1. Till now server.pubsub(shard)_channels only share channel name obj with the first subscribed client, now change it so that the clients and the pubsub kvstore share the channel name robj. This would save a lot of memory when there are many subscribers to the same channel. It also means that we only need to defrag the channel name robj in the pubsub kvstore, and then update all client references for the current channel, avoiding the need to iterate through all the clients to do the same things. 2. Refactor the code to defragment pubsub(shard) in the same way as defragment of keys and EXPIRES, with the exception that we only defragment pubsub(without shard) when slot is zero. ### Other Fix an overlook in redis#11695, if defragment doesn't reach the end time, we should wait for the current db's keys and expires, pubsub and pubsubshard to finish before leaving, now it's possible to exit early when the keys are defragmented. --------- Co-authored-by: oranagra <[email protected]>
YaacovHazan
pushed a commit
to YaacovHazan/redis
that referenced
this pull request
Sep 28, 2025
This PR fixes two defrag issues. 1. Fix a use-after-free issue caused by updating dictionary keys after a pubsub channel is reallocated. This issue was introduced by redis#13058 1. Fix potential use-after-free for lua during AOF loading with defrag This issue was introduced by redis#13058 This fix follows redis#14319 This PR updates the LuaScript LRU list before script execution to prevent accessing a potentially invalidated pointer after long-running scripts.
sundb
added a commit
to YaacovHazan/redis
that referenced
this pull request
Sep 30, 2025
This PR fixes two defrag issues. 1. Fix a use-after-free issue caused by updating dictionary keys after a pubsub channel is reallocated. This issue was introduced by redis#13058 1. Fix potential use-after-free for lua during AOF loading with defrag This issue was introduced by redis#13058 This fix follows redis#14319 This PR updates the LuaScript LRU list before script execution to prevent accessing a potentially invalidated pointer after long-running scripts.
sundb
added a commit
to YaacovHazan/redis
that referenced
this pull request
Sep 30, 2025
This PR fixes two defrag issues. 1. Fix a use-after-free issue caused by updating dictionary keys after a pubsub channel is reallocated. This issue was introduced by redis#13058 2. Fix potential use-after-free for lua during AOF loading with defrag This issue was introduced by redis#13058 This fix follows redis#14319 This PR updates the LuaScript LRU list before script execution to prevent accessing a potentially invalidated pointer after long-running scripts.
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.
After #13013
This PR make effort to defrag the pubsub kvstore in the following ways:
Till now server.pubsub(shard)_channels only share channel name obj with the first subscribed client, now change it so that the clients and the pubsub kvstore share the channel name robj.
This would save a lot of memory when there are many subscribers to the same channel.
It also means that we only need to defrag the channel name robj in the pubsub kvstore, and then update
all client references for the current channel, avoiding the need to iterate through all the clients to do the same things.
Refactor the code to defragment pubsub(shard) in the same way as defragment of keys and EXPIRES, with the exception that we only defragment pubsub(without shard) when slot is zero.
Other
Fix an overlook in #11695, if defragment doesn't reach the end time, we should wait for the current
db's keys and expires, pubsub and pubsubshard to finish before leaving, now it's possible to exit
early when the keys are defragmented.