Fix incorrect kvstore size and BIT accounting after completed migration#2749
Merged
Conversation
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #2749 +/- ##
============================================
+ Coverage 72.59% 72.63% +0.04%
============================================
Files 128 128
Lines 71301 71300 -1
============================================
+ Hits 51759 51791 +32
+ Misses 19542 19509 -33
🚀 New features to boost your workflow:
|
zuiderkwast
approved these changes
Oct 18, 2025
zuiderkwast
left a comment
Contributor
There was a problem hiding this comment.
Looked at the code change but I didn't read the test cases carefully.
We should hurry and include this in 9.0.0, otherwise we're releasing with this known bug. Or should we already plan for 9.0.1?
Contributor
Author
|
I agree - let's get it in to 9.0.0 |
cherukum-Amazon
pushed a commit
to cherukum-Amazon/valkey
that referenced
this pull request
Oct 19, 2025
…on (valkey-io#2749) When working on valkey-io#2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com> (cherry picked from commit 2a914aa)
cherukum-Amazon
pushed a commit
to cherukum-Amazon/valkey
that referenced
this pull request
Oct 19, 2025
…on (valkey-io#2749) When working on valkey-io#2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com> (cherry picked from commit 2a914aa) Signed-off-by: cherukum-amazon <cherukum@amazon.com>
cherukum-Amazon
pushed a commit
to cherukum-Amazon/valkey
that referenced
this pull request
Oct 21, 2025
…on (valkey-io#2749) When working on valkey-io#2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com> (cherry picked from commit 2a914aa) Signed-off-by: cherukum-amazon <cherukum@amazon.com>
diego-ciciani01
pushed a commit
to diego-ciciani01/valkey
that referenced
this pull request
Oct 21, 2025
…on (valkey-io#2749) When working on valkey-io#2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com>
madolson
pushed a commit
that referenced
this pull request
Oct 21, 2025
…on (#2749) When working on #2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com> (cherry picked from commit 2a914aa) Signed-off-by: cherukum-amazon <cherukum@amazon.com>
zhijun42
pushed a commit
to zhijun42/valkey
that referenced
this pull request
Nov 15, 2025
…on (valkey-io#2749) When working on valkey-io#2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com>
zhijun42
pushed a commit
to zhijun42/valkey
that referenced
this pull request
Nov 15, 2025
…on (valkey-io#2749) When working on valkey-io#2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com>
hpatro
pushed a commit
to hpatro/valkey
that referenced
this pull request
Mar 5, 2026
…on (valkey-io#2749) When working on valkey-io#2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash. The solution is: 1. Only call setSlotImportingStateInAllDbs once (in our finishSlotMigrationJob function) 2. Make setSlotImportingStateInAllDbs idempotent by checking if the delete from the kvstore importing hashtable is a no-op This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough. Signed-off-by: Jacob Murphy <jkmurphy@google.com> Signed-off-by: Harkrishn Patro <bunty.hari@gmail.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.
When working on #2635 I errorneously duplicated the setSlotImportingStateInAllDbs call for successful imports. This resulted in us doubling the key count in the kvstore. This results in DBSIZE reporting an incorrect sum, and also causes BIT corruption that can eventually result in a crash.
The solution is:
This also fixes a bug where the number of importing keys is not lowered after the migration, but this is less critical since it is only used when resizing the dictionary on RDB load. However, it could result in un-loadable RDBs if the importing key count gets large enough.