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

Jump to bottom

optimize ps v3 restore #5163

Merged
merged 8 commits into from
Jun 22, 2022
Merged

optimize ps v3 restore #5163

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
943b0e9
optimize ps v3 restore
hehechen Jun 16, 2022
5447f4f
Update dbms/src/Storages/Page/V3/PageDirectory.h
hehechen Jun 20, 2022
2bac4eb
Update dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp
hehechen Jun 20, 2022
ca66832
fix comments
hehechen Jun 20, 2022
396420c
fix
hehechen Jun 20, 2022
b9c7c3f
Merge branch 'master' into v3restore
JaySon-Huang Jun 21, 2022
818e7df
Merge branch 'master' into v3restore
ti-chi-bot Jun 22, 2022
2fa35e1
Merge branch 'master' into v3restore
ti-chi-bot Jun 22, 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
20 changes: 13 additions & 7 deletions dbms/src/Storages/Page/V3/PageDirectory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ PageSize VersionedPageEntries::getEntriesByBlobIds(
bool VersionedPageEntries::cleanOutdatedEntries(
UInt64 lowest_seq,
std::map<PageIdV3Internal, std::pair<PageVersion, Int64>> * normal_entries_to_deref,
PageEntriesV3 & entries_removed,
PageEntriesV3 * entries_removed,
const PageLock & /*page_lock*/)
{
if (type == EditRecordType::VAR_EXTERNAL)
Expand Down Expand Up @@ -541,7 +541,10 @@ bool VersionedPageEntries::cleanOutdatedEntries(
{
if (iter->second.being_ref_count == 1)
{
entries_removed.emplace_back(iter->second.entry);
if (entries_removed)
{
entries_removed->emplace_back(iter->second.entry);
}
iter = entries.erase(iter);
}
// The `being_ref_count` for this version is valid. While for older versions,
Expand All @@ -551,7 +554,10 @@ bool VersionedPageEntries::cleanOutdatedEntries(
else
{
// else there are newer "entry" in the version list, the outdated entries should be removed
entries_removed.emplace_back(iter->second.entry);
if (entries_removed)
{
entries_removed->emplace_back(iter->second.entry);
}
iter = entries.erase(iter);
}
}
Expand All @@ -564,7 +570,7 @@ bool VersionedPageEntries::cleanOutdatedEntries(
return entries.empty() || (entries.size() == 1 && entries.begin()->second.isDelete());
}

bool VersionedPageEntries::derefAndClean(UInt64 lowest_seq, PageIdV3Internal page_id, const PageVersion & deref_ver, const Int64 deref_count, PageEntriesV3 & entries_removed)
bool VersionedPageEntries::derefAndClean(UInt64 lowest_seq, PageIdV3Internal page_id, const PageVersion & deref_ver, const Int64 deref_count, PageEntriesV3 * entries_removed)
{
auto page_lock = acquireLock();
if (type == EditRecordType::VAR_EXTERNAL)
Expand Down Expand Up @@ -1239,7 +1245,7 @@ bool PageDirectory::tryDumpSnapshot(const ReadLimiterPtr & read_limiter, const W
return done_any_io;
}

PageEntriesV3 PageDirectory::gcInMemEntries()
PageEntriesV3 PageDirectory::gcInMemEntries(bool return_removed_entries)
{
UInt64 lowest_seq = sequence.load();

Expand Down Expand Up @@ -1303,7 +1309,7 @@ PageEntriesV3 PageDirectory::gcInMemEntries()
const bool all_deleted = iter->second->cleanOutdatedEntries(
lowest_seq,
&normal_entries_to_deref,
all_del_entries,
return_removed_entries ? &all_del_entries : nullptr,
iter->second->acquireLock());

{
Expand Down Expand Up @@ -1342,7 +1348,7 @@ PageEntriesV3 PageDirectory::gcInMemEntries()
page_id,
/*deref_ver=*/deref_counter.first,
/*deref_count=*/deref_counter.second,
all_del_entries);
return_removed_entries ? &all_del_entries : nullptr);

if (all_deleted)
{
Expand Down
8 changes: 5 additions & 3 deletions dbms/src/Storages/Page/V3/PageDirectory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +223,14 @@ class VersionedPageEntries
bool cleanOutdatedEntries(
UInt64 lowest_seq,
std::map<PageIdV3Internal, std::pair<PageVersion, Int64>> * normal_entries_to_deref,
PageEntriesV3 & entries_removed,
PageEntriesV3 * entries_removed,
const PageLock & page_lock);
bool derefAndClean(
UInt64 lowest_seq,
PageIdV3Internal page_id,
const PageVersion & deref_ver,
Int64 deref_count,
PageEntriesV3 & entries_removed);
PageEntriesV3 * entries_removed);

void collapseTo(UInt64 seq, PageIdV3Internal page_id, PageEntriesEdit & edit);

Expand Down Expand Up @@ -360,7 +360,9 @@ class PageDirectory

bool tryDumpSnapshot(const ReadLimiterPtr & read_limiter = nullptr, const WriteLimiterPtr & write_limiter = nullptr);

PageEntriesV3 gcInMemEntries();
// Perform a GC for in-memory entries and return the removed entries.
// If `return_removed_entries` is false, then just return an empty set.
PageEntriesV3 gcInMemEntries(bool return_removed_entries = true);
Copy link
Contributor

Choose a reason for hiding this comment

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

return_removed_entries how about change to record_removed_entries ?


std::set<PageId> getAliveExternalIds(NamespaceId ns_id) const;

Expand Down
6 changes: 4 additions & 2 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 @@ -44,7 +44,8 @@ PageDirectoryPtr PageDirectoryFactory::createFromReader(String storage_name, WAL

// 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();
// It's no need to remove the expired entries in BlobStore, so skip filling removed_entries to imporve performance.
dir->gcInMemEntries(/*return_removed_entries=*/false);
LOG_FMT_INFO(DB::Logger::get("PageDirectoryFactory", storage_name), "PageDirectory restored [max_page_id={}] [max_applied_ver={}]", dir->getMaxId(), dir->sequence);

if (blob_stats)
Expand Down Expand Up @@ -84,7 +85,8 @@ PageDirectoryPtr PageDirectoryFactory::createFromEdit(String storage_name, FileP

// 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();
// It's no need to remove the expired entries in BlobStore when restore, so no need to fill removed_entries.
dir->gcInMemEntries(/*return_removed_entries=*/false);

if (blob_stats)
{
Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,14 +644,14 @@ class VersionedEntriesTest : public ::testing::Test
{
DerefCounter deref_counter;
PageEntriesV3 removed_entries;
bool all_removed = entries.cleanOutdatedEntries(seq, &deref_counter, removed_entries, entries.acquireLock());
bool all_removed = entries.cleanOutdatedEntries(seq, &deref_counter, &removed_entries, entries.acquireLock());
return {all_removed, removed_entries, deref_counter};
}

std::tuple<bool, PageEntriesV3> runDeref(UInt64 seq, PageVersion ver, Int64 decrease_num)
{
PageEntriesV3 removed_entries;
bool all_removed = entries.derefAndClean(seq, buildV3Id(TEST_NAMESPACE_ID, page_id), ver, decrease_num, removed_entries);
bool all_removed = entries.derefAndClean(seq, buildV3Id(TEST_NAMESPACE_ID, page_id), ver, decrease_num, &removed_entries);
return {all_removed, removed_entries};
}

Expand Down