Check ScheduledCompactKeyName and FinishedCompactKeyName before writing hash #15985
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
CaojiamingAlan
changed the title
jmhbnz
reviewed
May 31, 2023
There was a problem hiding this comment.
Thanks for your work on this @CaojiamingAlan!
Still getting my head around the changes and linked issue, a few tiny nitpicks to consider after an initial read through.
CaojiamingAlan
force-pushed
the
check_revision_before_write_hash
branch
from
6dd5b8e
to
8601eed
Compare
serathius
reviewed
May 31, 2023
serathius
reviewed
May 31, 2023
serathius
reviewed
May 31, 2023
serathius
reviewed
May 31, 2023
serathius
reviewed
May 31, 2023
CaojiamingAlan
force-pushed
the
check_revision_before_write_hash
branch
2 times, most recently
from
24a92a4
to
ed987e5
Compare
jmhbnz
reviewed
Jun 1, 2023
CaojiamingAlan
force-pushed
the
check_revision_before_write_hash
branch
from
ed987e5
to
de65c9e
Compare
|
Please mark conversations as resolved if you have addressed the comment. |
serathius
reviewed
Jun 2, 2023
serathius
reviewed
Jun 2, 2023
CaojiamingAlan
force-pushed
the
check_revision_before_write_hash
branch
from
de65c9e
to
c3edd9c
Compare
jmhbnz
approved these changes
Jun 3, 2023
|
cc @ahrtr |
ahrtr
reviewed
Jun 5, 2023
ahrtr
reviewed
Jun 6, 2023
CaojiamingAlan
force-pushed
the
check_revision_before_write_hash
branch
2 times, most recently
from
c7e1b81
to
1c6eeaf
Compare
chaochn47
approved these changes
Jun 7, 2023
CaojiamingAlan
force-pushed
the
check_revision_before_write_hash
branch
from
1c6eeaf
to
d1c946c
Compare
ahrtr
reviewed
Jun 8, 2023
…esuming scheduled compaction. check ScheduledCompactKeyName and FinishedCompactKeyName before writing hash to hashstore. If they do not match, then it means this compaction has once been interrupted and its hash value is invalid. In such cases, we won't write the hash values to the hashstore, and avoids the incorrect corruption alarm. Signed-off-by: caojiamingalan <alan.c.19971111@gmail.com>
CaojiamingAlan
force-pushed
the
check_revision_before_write_hash
branch
from
d1c946c
to
b9e30bf
Compare
|
@CaojiamingAlan could you backport this PR to 3.5? |
|
@ahrtr Sure, does it mean proposing a PR to merge this change to the release-3.5 branch? |
Yes. Please note that |
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.
Fix #15919
Commit 1: add an e2e test to reproduce the problem. This test won't pass without the fix.
Commit 2: check ScheduledCompactKeyName and FinishedCompactKeyName
before writing hash to hashstore. If they do not match, then it means this compaction has once been interrupted and its hash value is invalid. In such cases, we won't write the hash values to the hashstore, and avoids the incorrect corruption alarm.