-
Notifications
You must be signed in to change notification settings - Fork 411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PageStorage: handle ref and full gc after restart #7931
Conversation
/rebuild |
a6ed009
to
01affc1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-all-tests |
/run-all-tests |
Signed-off-by: JaySon-Huang <[email protected]>
/run-all-tests |
/rebuild |
/run-all-tests |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lidezhu, Lloyd-Pottiger The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
auto field_begin_in_buf | ||
= cur_field_begin <= buffer_begin_in_page ? 0 : cur_field_begin - buffer_begin_in_page; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto field_begin_in_buf | |
= cur_field_begin <= buffer_begin_in_page ? 0 : cur_field_begin - buffer_begin_in_page; | |
auto field_begin_in_buf = cur_field_begin <= buffer_begin_in_page | |
? 0 | |
: cur_field_begin - buffer_begin_in_page; |
/run-integration-test |
/hold |
/unhold |
/run-all-tests |
/hold |
/unhold |
@JaySon-Huang: Your PR was out of date, I have automatically updated it for you. At the same time I will also trigger all tests for you: /run-all-tests
If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
/run-unit-test |
What problem does this PR solve?
Issue Number: close #7915
Problem Summary:
In the previous PR #7920, we can not handle the full GC properly. It will ignore all
UPSERT
entries that is created byblobstore full gc
. Becausefull gc
will use a version=<old_version, old_epoch+1>
for theUPSERT
entries in WAL log files.What is changed and how it works?
As only the REF is not an idempotent operation, when loading edit from disk to restore the PageDirectory, it could re-apply a REF to an non-existing page_id that is already deleted in the dumped snapshot.
All other operation is idempotent operations. So we filter the REF record which is less than or equal to the
filter_seq
For example, in the previous case, we will ignore the
REF 2->1
record when restoring from disk.And after #7668, we could apply
UPSERT
record that the page_id is "deleted" in the dumped snapshot. So we relax the check when aUPSERT
record is applyhint: ignore the commit "Format files": https://github.com/pingcap/tiflash/pull/7931/files/f2167112e883da119e2806e3280f49a01c15e125..01affc15ef42aa43bdcbb5688f73cbe85387f001
Check List
Tests
Side effects
Documentation
Release note