Skip to content
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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

PageDirectory: need do gc twice in restore. #4972

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cd77ac3
PageDirectory: need do gc twice in restore.
jiaqizho May 23, 2022
54a3fed
add ut
jiaqizho May 24, 2022
5d51397
merge to master
jiaqizho May 24, 2022
aec9789
add mvcc test
jiaqizho May 24, 2022
62e7c14
merge to master
jiaqizho May 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@ PageDirectoryPtr PageDirectoryFactory::create(String storage_name, FileProviderP
// try to run GC again on some entries that are already marked as invalid in BlobStore.
dir->gcInMemEntries();

// MVCC gc must do twice when restore
// Because the first time call MVCC GC may not clean the deleted entries if being reference.
// But it will clean up the deleted reference.
// ex.
// P 1
// R 2 -> 1
// D 1
// D 2
// First time call gcInMemEntries():
// - P 1 entry 0 being_ref_count is not zero, won't be cleaned.
// - R 2 entry 0 being_ref_count is zero, deref the P 1, make it being_ref_count is 0. Also R2 will be clean up.
//
// Second time call gcInMemEntries():
// - P 1 being_ref_count is 0, it will be clean.
jiaqizho marked this conversation as resolved.
Show resolved Hide resolved
jiaqizho marked this conversation as resolved.
Show resolved Hide resolved
dir->gcInMemEntries();
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

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)


if (blob_stats)
{
// After all entries restored to `mvcc_table_directory`, only apply
Expand Down