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

SegmentReadTaskScheduler::add took too long. #9024

Closed
SeaRise opened this issue May 7, 2024 · 2 comments · Fixed by #9027
Closed

SegmentReadTaskScheduler::add took too long. #9024

SeaRise opened this issue May 7, 2024 · 2 comments · Fixed by #9027
Assignees
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. component/storage severity/major type/bug The issue is confirmed as a bug.

Comments

@SeaRise
Copy link
Contributor

SeaRise commented May 7, 2024

Bug Report

[2024/05/02 14:11:37.563 +00:00] [INFO] [SegmentReadTaskScheduler.cpp:58] ["Added, pool_id=47879 table_id=7511 block_slots=48 segment_count=4 pool_count=123 cost=120437219.576us do_add_cost=7.705us"] [thread_id=929]

120s

v7.5.1

@SeaRise SeaRise added the type/bug The issue is confirmed as a bug. label May 7, 2024
@JinheLin JinheLin self-assigned this May 7, 2024
@JinheLin JinheLin added affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. labels May 7, 2024
ti-chi-bot bot pushed a commit that referenced this issue May 8, 2024
@JaySon-Huang
Copy link
Contributor

The root cause is since v7.5, we may meet some DeltaMergeStore with only 0 segment in it. When the read task created, it will add some SegmentReadTaskPool without any segment task.

[2024/05/02 14:20:25.054 +00:00] [INFO] [SegmentReadTaskScheduler.cpp:58] ["Added, pool_id=49535 table_id=7514 block_slots=48 **segment_count=0** pool_count=116 **cost=154525318.043us** do_add_cost=3.077us"] [thread_id=3088]

It may make SegmentReadTaskScheduler::scheduleMergedTask return run_next_schedule_immediately=true. And the mtx always required by SegmentReadTaskScheduler::schedule, making SegmentReadTaskScheduler::add starve

std::optional<std::pair<uint64_t, std::vector<uint64_t>>> SegmentReadTaskScheduler::scheduleSegmentUnlock(
const SegmentReadTaskPoolPtr & pool)
{
auto expected_merge_seg_count = std::min(read_pools.size(), 2); // Not accurate.
auto itr = merging_segments.find(pool->physical_table_id);
if (itr == merging_segments.end())
{
// No segment of tableId left.
return std::nullopt;
}

auto segment = scheduleSegmentUnlock(pool);
if (!segment)
{
// The number of active segments reaches the limit.
GET_METRIC(tiflash_storage_read_thread_counter, type_sche_no_segment).Increment();
return {nullptr, true};
}

@JaySon-Huang
Copy link
Contributor

In theory, only LTS v7.5.0/v7.5.1 is affected. Because before #7437, there is at least 1 segment for empty table/partition, which won't cause this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. 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.

3 participants