Fix cf_consistency_stress for backup/restore, harmonize #7373
Conversation
This file contains 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
Summary: We can only check key on restored backup if in a stress test configuration locking the key. (Fixes mismatch seen in backup/restore with atomic flush.) TestCheckpoint used a very ugly solution to the same problem: copy-paste dozens of lines of code with some changes and removals. I removed the unnecessary implementation and made the existing one simply adaptive, like TestBackupRestore. Also made TestBackupRestore clean up dead backup/restore directories on success. Test Plan: blackbox_crash_test_with_atomic_flush for a while, blackbox_crash_test for a while, with backup and checkpoint 1 in 5k and only 1k max_keys to stress this area
There was a problem hiding this comment.
@pdillinger has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pdillinger merged this pull request in c4e2066. |
codingrhythm
pushed a commit
to SafetyCulture/rocksdb
that referenced
this pull request
Mar 5, 2021
Summary: We can only check key on restored backup if in a stress test configuration locking the key. (Fixes mismatch seen in backup/restore with atomic flush.) TestCheckpoint used a very ugly solution to the same problem: copy-paste dozens of lines of code with some changes and removals. I removed the unnecessary implementation and made the existing one simply adaptive, like TestBackupRestore. Also made TestBackupRestore clean up dead backup/restore directories on success. Pull Request resolved: facebook#7373 Test Plan: blackbox_crash_test_with_atomic_flush for a while, blackbox_crash_test for a while, with backup and checkpoint 1 in 5k and only 1k max_keys to stress this area Reviewed By: ajkr Differential Revision: D23629057 Pulled By: pdillinger fbshipit-source-id: d7fe7e2be75aaf3cf974be9540a7c5c5de8b371b
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.
Summary: We can only check key on restored backup if in a stress test
configuration locking the key. (Fixes mismatch seen in backup/restore
with atomic flush.)
TestCheckpoint used a very ugly solution to the same problem: copy-paste
dozens of lines of code with some changes and removals. I removed the
unnecessary implementation and made the existing one simply adaptive,
like TestBackupRestore.
Also made TestBackupRestore clean up dead backup/restore directories on
success.
Test Plan: blackbox_crash_test_with_atomic_flush for a while,
blackbox_crash_test for a while, with backup and checkpoint 1 in 5k and
only 1k max_keys to stress this area