-
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
PageDirectory: need do gc twice in restore. #4972
Conversation
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
// | ||
// Second time call gcInMemEntries(): | ||
// - P 1 being_ref_count is 0, it will be clean. | ||
dir->gcInMemEntries(); |
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.
Also add it to PageDirectoryFactory::createFromEdit
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.
done
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.
I mean in these lines, we should also call gcInMemEntries twice
tiflash/dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp
Lines 87 to 100 in 62e7c14
PageDirectoryPtr PageDirectoryFactory::createFromEdit(String storage_name, FileProviderPtr & file_provider, PSDiskDelegatorPtr & delegator, const PageEntriesEdit & edit) | |
{ | |
auto [wal, reader] = WALStore::create(storage_name, file_provider, delegator, WALStore::Config()); | |
(void)reader; | |
PageDirectoryPtr dir = std::make_unique<PageDirectory>(std::move(storage_name), std::move(wal)); | |
loadEdit(dir, edit); | |
// Reset the `sequence` to the maximum of persisted. | |
dir->sequence = max_applied_ver.sequence; | |
// After restoring from the disk, we need cleanup all invalid entries in memory, or it will | |
// try to run GC again on some entries that are already marked as invalid in BlobStore. | |
dir->gcInMemEntries(); | |
if (blob_stats) |
Do not need this change because we have cleaned entries after decreasing the ref count in tiflash/dbms/src/Storages/Page/V3/PageDirectory.cpp Lines 599 to 603 in 64a747a
The test cases will be merged into #4987 |
What problem does this PR solve?
Issue Number: ref #4957
Problem Summary:
entry
if thatentry
be refence by a deletedref
.gcInMemEntries()
:gcInMemEntries()
:restore
. Then BlobStore may not restore success.What is changed and how it works?
gcInMemEntries()
twice to clean up all of deleted entries.Check List
Tests
Side effects
Documentation
Release note