Fix issue that cause false alarm corruption report #12626
Conversation
db/compaction/compaction_iterator.cc
Outdated
| // saved_seq_for_penul_check_ is populated in `NextFromInput` when the | ||
| // entry's sequence number is non zero and validity context for output this | ||
| // entry is kSwapPreferredSeqno for use in `DecideOutputLevel`. It should be | ||
| // cleared out here unconditionally. |
There was a problem hiding this comment.
It wasn't immediately clear to me that the field is cleared because this is where the value is consumed.
There was a problem hiding this comment.
Yeah, I added this loud comment to emphasize this.
There was a problem hiding this comment.
I mean reading your comment didn't make it clear. I went searching for other uses of saved_seq_for_penul_check_ to get more context on why "it should be cleared out here."
There was a problem hiding this comment.
I see, I added some clarification, specifically it's this: "Otherwise, it may end up getting consumed
incorrectly by a different entry"
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jowlyzhang has updated the pull request. You must reimport the pull request before landing. |
|
@jowlyzhang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@jowlyzhang merged this pull request in 9dc171e. |
Summary: The state of `saved_seq_for_penul_check_` is not correctly maintained with the current flow. It's supposed to store the original sequence number for a `kTypeValuePreferredSeqno` entry for use in the `DecideOutputLevel` function. However, it's not always properly cleared. Pull Request resolved: #12626 Test Plan: Added unit test that would fail before the fix ./tiered_compaction_test --gtest_filter="*InterleavedTimedPutAndPut*" Reviewed By: pdillinger Differential Revision: D57123469 Pulled By: jowlyzhang fbshipit-source-id: 8d73214b3b6dc152daf19b6bd6ee9063581dc277
The state of
saved_seq_for_penul_check_is not correctly maintained with the current flow. It's supposed to store the original sequence number for akTypeValuePreferredSeqnoentry for use in theDecideOutputLevelfunction. However, it's not always properly cleared.Test Plan:
Added unit test that would fail before the fix
./tiered_compaction_test --gtest_filter="InterleavedTimedPutAndPut"