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.

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 gc for external pages my block creating tables #5697

Closed
JaySon-Huang opened this issue Aug 25, 2022 · 1 comment · Fixed by #5699
Closed

PageStorage gc for external pages my block creating tables #5697

JaySon-Huang opened this issue Aug 25, 2022 · 1 comment · Fixed by #5699
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 component/storage severity/major type/bug The issue is confirmed as a bug.

Comments

@JaySon-Huang
Copy link
Contributor

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

In PageStorageImpl::gcImpl clean_external_page, we hold the callbacks_mutex until all external page gc done.

auto clean_external_page = [this]() {
std::scoped_lock lock{callbacks_mutex};
if (!callbacks_container.empty())
{
for (const auto & [ns_id, callbacks] : callbacks_container)
{
auto pending_external_pages = callbacks.scanner();
auto alive_external_ids = getAliveExternalPageIds(ns_id);
callbacks.remover(pending_external_pages, alive_external_ids);
}
}
};

while the time cost in clean_external_page is not ignorable and may block new StorageDeltaMerge from being created

void PageStorageImpl::registerExternalPagesCallbacks(const ExternalPageCallbacks & callbacks)
{
std::scoped_lock lock{callbacks_mutex};
assert(callbacks.scanner != nullptr);
assert(callbacks.remover != nullptr);
assert(callbacks.ns_id != MAX_NAMESPACE_ID);
assert(callbacks_container.count(callbacks.ns_id) == 0);
callbacks_container.emplace(callbacks.ns_id, callbacks);
}

// remember to unregister it when shutdown
storage_pool->dataRegisterExternalPagesCallbacks(callbacks);

2. What did you expect to see? (Required)

3. What did you see instead (Required)

4. What is your TiFlash version? (Required)

master

@JaySon-Huang JaySon-Huang added type/bug The issue is confirmed as a bug. affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 labels Aug 25, 2022
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Aug 25, 2022

And calling getAliveExternalPageIds(ns_id) inside the loop makes it cost more time when the number of tables is large

# create 1000 empty tables for testing
for ((i=0;i!=1000;++i)) do; mycli -h 172.16.5.85 -P 8010 --no-warn -D test -e "create table t${i} (c int);"; done

get alive external page for one namespace id inside loop, "get alive" takes about 100ms

[2022/08/25 13:37:26.985 +08:00] [DEBUG] [PageStorageImpl.cpp:330] ["PageStorage:__global__.data GC finished without full gc. [total time=179ms] [dump snapshots=0ms] [gc in mem entries=1ms] [blobstore remove entries=0ms] [blobstore get status=0ms] [get gc entries=0ms] [blobstore full gc=0ms] [gc apply=0ms] [external callbacks=1052] [external gc=178ms] [get alive=92.77ms] [scanner=84.87ms] [remover=0.91ms]"] [thread_id=100]
[2022/08/25 13:38:29.086 +08:00] [DEBUG] [PageStorageImpl.cpp:330] ["PageStorage:__global__.data GC finished without full gc. [total time=197ms] [dump snapshots=0ms] [gc in mem entries=0ms] [blobstore remove entries=0ms] [blobstore get status=0ms] [get gc entries=0ms] [blobstore full gc=0ms] [gc apply=0ms] [external callbacks=1052] [external gc=197ms] [get alive=102.16ms] [scanner=93.88ms] [remover=1.05ms]"] [thread_id=66]
[2022/08/25 13:39:32.365 +08:00] [DEBUG] [PageStorageImpl.cpp:330] ["PageStorage:__global__.data GC finished without full gc. [total time=209ms] [dump snapshots=0ms] [gc in mem entries=0ms] [blobstore remove entries=0ms] [blobstore get status=0ms] [get gc entries=0ms] [blobstore full gc=0ms] [gc apply=0ms] [external callbacks=1052] [external gc=209ms] [get alive=111.65ms] [scanner=96.45ms] [remover=0.92ms]"] [thread_id=160]

get alive external page for all namespace ids, "get alive" takes about 3ms

[2022/08/25 13:33:19.193 +08:00] [DEBUG] [PageStorageImpl.cpp:330] ["PageStorage:__global__.data GC finished without full gc. [total time=88ms] [dump snapshots=0ms] [gc in mem entries=1ms] [blobstore remove entries=0ms] [blobstore get status=0ms] [get gc entries=0ms] [blobstore full gc=0ms] [gc apply=0ms] [external callbacks=1052] [external gc=87ms] [get alive=2.67ms] [scanner=82.25ms] [remover=1.89ms]"] [thread_id=96]
[2022/08/25 13:34:19.614 +08:00] [DEBUG] [PageStorageImpl.cpp:330] ["PageStorage:__global__.data GC finished without full gc. [total time=84ms] [dump snapshots=0ms] [gc in mem entries=0ms] [blobstore remove entries=0ms] [blobstore get status=0ms] [get gc entries=0ms] [blobstore full gc=0ms] [gc apply=0ms] [external callbacks=1052] [external gc=84ms] [get alive=2.49ms] [scanner=79.38ms] [remover=1.81ms]"] [thread_id=130]
[2022/08/25 13:35:23.486 +08:00] [DEBUG] [PageStorageImpl.cpp:330] ["PageStorage:__global__.data GC finished without full gc. [total time=87ms] [dump snapshots=0ms] [gc in mem entries=0ms] [blobstore remove entries=0ms] [blobstore get status=0ms] [get gc entries=0ms] [blobstore full gc=0ms] [gc apply=0ms] [external callbacks=1052] [external gc=86ms] [get alive=2.61ms] [scanner=81.73ms] [remover=1.79ms]"] [thread_id=63]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 component/storage severity/major type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants