Verify flushed data are recovered upon reopen in crash test #12787
Conversation
|
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
6e5f737 to
a4314e7
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
a4314e7 to
4be510e
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
|
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
800b609 to
1dddd4a
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
|
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
1dddd4a to
9f98a86
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
This looks generally right to me.
I want to read through #12152 again to understand it better.
Some questions:
From #12152 (comment)
We should update the expected minimum value of that counter for (1) explicit flushes, and (2) OnFlushCompleted() events
Will a separate PR handle case 1? I only see case 2 being handled
From #12152 (comment)
WAL-disabled, atomic_flush-disabled may be worth testing at some future point but that is still an open topic to discuss. My thoughts are here - #11841 (comment).
Another case that may be worth testing at some point is WAL-disabled, atomic_flush-enabled, and manual flush triggered on a subset of column families (what you suggested we are already doing).
What is the current thinking on how many of these WAL {enabled, disabled}, atomic flush {enabled, disabled} combinations we will try to test?
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
9f98a86 to
18d84d1
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
18d84d1 to
c26ae4d
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
c26ae4d to
a8d9041
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
|
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
hx235
force-pushed
the
verify_flushed_data_recovery
branch
from
a8d9041 to
fd22753
Compare
|
@hx235 has updated the pull request. You must reimport the pull request before landing. |
|
@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Context/Summary:
This is to solve #12152. We persist the largest flushed seqno before crash just like how we persist the ExpectedState. And we verify the db lates seqno after recovery is no smaller than this flushed seqno.
Test: