Fix db->expires can't be defragged due to incorrect comparison in the expires stage#14092
Merged
sundb merged 6 commits intoredis:unstablefrom Jun 5, 2025
Merged
Fix db->expires can't be defragged due to incorrect comparison in the expires stage#14092sundb merged 6 commits intoredis:unstablefrom
sundb merged 6 commits intoredis:unstablefrom
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
Contributor
There was a problem hiding this comment.
Pull Request Overview
Fixes the incorrect comparison in the expires defragmentation stage and adjusts related test thresholds.
- Corrects the conditional in
defragStageExpiresKvstoreto comparedb->expiresrather thandb->keys - Updates unit test parameters (
active-defrag-threshold-lowerandwait_for_defrag_stopmultiplier) to reflect the new behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/defrag.c | Use db->expires in the resume check for expires defrag |
| tests/unit/memefficiency.tcl | Lower defrag threshold and adjust stop–wait multiplier |
ShooterIT
approved these changes
Jun 5, 2025
sundb
commented
Jun 5, 2025
| defragKeysCtx *defrag_keys_ctx = ctx; | ||
| redisDb *db = &server.db[defrag_keys_ctx->dbid]; | ||
| if (db->keys != defrag_keys_ctx->kvstate.kvs) { | ||
| if (db->expires != defrag_keys_ctx->kvstate.kvs) { |
Collaborator
Author
There was a problem hiding this comment.
@YaacovHazan since the TCL test was added in 8.2, we only need to pick this line of code to 8.0 at that time.
sundb
added a commit
to YaacovHazan/redis
that referenced
this pull request
Jul 2, 2025
… expires stage (redis#14092) This bug was introduced by redis#13814 When defragmenting `db->expires`, if the process exits early and `db->expires` was modified in the meantime (e.g., FLUSHDB), we need to check whether the previously defragmented expires is still the same as the current one when resuming. If they differ, we should abort the current defragmentation of expires. However, in redis#13814, I made a mistake by using `db->keys` and `db->expires`, as expires will never be defragged.
Merged
YaacovHazan
pushed a commit
that referenced
this pull request
Jul 6, 2025
… expires stage (#14092) This bug was introduced by #13814 When defragmenting `db->expires`, if the process exits early and `db->expires` was modified in the meantime (e.g., FLUSHDB), we need to check whether the previously defragmented expires is still the same as the current one when resuming. If they differ, we should abort the current defragmentation of expires. However, in #13814, I made a mistake by using `db->keys` and `db->expires`, as expires will never be defragged.
funny-dog
pushed a commit
to funny-dog/redis
that referenced
this pull request
Sep 17, 2025
… expires stage (redis#14092) This bug was introduced by redis#13814 When defragmenting `db->expires`, if the process exits early and `db->expires` was modified in the meantime (e.g., FLUSHDB), we need to check whether the previously defragmented expires is still the same as the current one when resuming. If they differ, we should abort the current defragmentation of expires. However, in redis#13814, I made a mistake by using `db->keys` and `db->expires`, as expires will never be defragged.
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.
This bug was introduced by #13814
When defragmenting
db->expires, if the process exits early anddb->expireswas modified in the meantime (e.g., FLUSHDB), we need to check whether the previously defragmented expires is still the same as the current one when resuming. If they differ, we should abort the current defragmentation of expires.However, in #13814, I made a mistake by using
db->keysanddb->expires, as expires will never be defragged.