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

[enhencement](segcompaction) cancel inflight segcompaction tasks faster when load finish #28901

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

freemandealer
Copy link
Contributor

[Goal]
When building the rowset writer, avoid waiting for inflight segcompaction to elimite long tail latency for load.

[Current situation]

  1. The segcompaction of a rowset is executed serially. During the build phase, we need to wait for the completion of the inflight segcompaction task.

  2. If the rowset writer finishes writing and starts building meta, then segments that have not been compacted will not be submitted to segcompaction worker. We simply ignore them to accelerate the build process.

  3. But this is not enough. If a segcompaction task has already been submitted to the worker thread pool, we will set a cancelled flag for the worker, and nothing will be done during execution to complete the task ASAP.

  4. But this is still not enough. Although the latency of the segcompaction task has been shortened by aforemetioned method, tasks may still be queuing in the thread pool.

[Solution]
We can increase the worker thread pool to avoid queuing congestion, but this is not the best solution.
Segcompaction should be a best effort work, and should not use too many CPU and memory resources. So we adopted the strategy of unbinding build and segcompaction, specifically:

  1. For the segcompaction task that is performing compaction operations, we should not interrupt it, otherwise it may cause file corruption

  2. For those tasks still queued, we no longer care about their results (because these tasks will know they are cancelled and will not perform any actual operations), so we just ignore them and continue with the subsequent rowset build process

Proposed changes

Issue Number: close #xxx

Further comments

If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...

…er when load finish

[Goal]
When building the rowset writer, avoid waiting for inflight segcompaction
to elimite long tail latency for load.

[Current situation]
1. The segcompaction of a rowset is executed serially. During the build phase,
we need to wait for the completion of the inflight segcompaction task.

2. If the rowset writer finishes writing and starts building meta, then segments
that have not been compacted will not be submitted to segcompaction worker.
We simply ignore them to accelerate the build process.

3. But this is not enough. If a segcompaction task has already been submitted to
the worker thread pool, we will set a cancelled flag for the worker,
and nothing will be done during execution to complete the task ASAP.

4. But this is still not enough. Although the latency of the segcompaction task
has been shortened by aforemetioned method, tasks may still be queuing in the
thread pool.

[Solution]
We can increase the worker thread pool to avoid queuing congestion, but this is
not the best solution.
Segcompaction should be a best effort work, and should not use too many CPU and
memory resources. So we adopted the strategy of unbinding build and segcompaction,
specifically:

1. For the segcompaction task that is performing compaction operations, we should
not interrupt it, otherwise it may cause file corruption

2. For those tasks still queued, we no longer care about their results (because
these tasks will know they are cancelled and will not perform any actual operations),
so we just ignore them and continue with the subsequent rowset build process

Signed-off-by: freemandealer <[email protected]>
@freemandealer
Copy link
Contributor Author

run buildall

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -997,17 +997,22 @@ Status StorageEngine::submit_compaction_task(TabletSharedPtr tablet, CompactionT
return _submit_compaction_task(tablet, compaction_type, force);
}

Status StorageEngine::_handle_seg_compaction(SegcompactionWorker* worker,
SegCompactionCandidatesSharedPtr segments) {
Status StorageEngine::_handle_seg_compaction(std::shared_ptr<SegcompactionWorker> worker,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method '_handle_seg_compaction' can be made static [readability-convert-member-functions-to-static]

Suggested change
Status StorageEngine::_handle_seg_compaction(std::shared_ptr<SegcompactionWorker> worker,
static Status StorageEngine::_handle_seg_compaction(std::shared_ptr<SegcompactionWorker> worker,

worker->compact_segments(segments);
// return OK here. error will be reported via BetaRowsetWriter::_segcompaction_status
return Status::OK();
}

Status StorageEngine::submit_seg_compaction_task(SegcompactionWorker* worker,
Status StorageEngine::submit_seg_compaction_task(std::shared_ptr<SegcompactionWorker> worker,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: method 'submit_seg_compaction_task' can be made static [readability-convert-member-functions-to-static]

Suggested change
Status StorageEngine::submit_seg_compaction_task(std::shared_ptr<SegcompactionWorker> worker,
static Status StorageEngine::submit_seg_compaction_task(std::shared_ptr<SegcompactionWorker> worker,

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.53% (8538/23373)
Line Coverage: 28.62% (69422/242589)
Region Coverage: 27.64% (35920/129979)
Branch Coverage: 24.36% (18357/75356)
Coverage Report: http://coverage.selectdb-in.cc/coverage/cef2636805bc62475a90db2efd958c49e2283f13_cef2636805bc62475a90db2efd958c49e2283f13/report/index.html

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.13 seconds
stream load tsv: 567 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 19 seconds loaded 2358488459 Bytes, about 118 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 28.7 seconds inserted 10000000 Rows, about 348K ops/s
storage size: 17184267181 Bytes

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Tpch sf100 test result on commit cef2636805bc62475a90db2efd958c49e2283f13, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4665	4409	4487	4409
q2	369	154	160	154
q3	1467	1275	1201	1201
q4	1097	880	920	880
q5	3196	3192	3164	3164
q6	247	133	129	129
q7	1011	491	513	491
q8	2197	2204	2184	2184
q9	6689	6695	6628	6628
q10	3221	3265	3296	3265
q11	308	187	192	187
q12	354	213	214	213
q13	4567	3808	3794	3794
q14	245	215	214	214
q15	570	522	524	522
q16	437	383	384	383
q17	1017	647	582	582
q18	7037	6793	6787	6787
q19	1548	1447	1419	1419
q20	523	312	308	308
q21	3059	2642	2639	2639
q22	348	275	284	275
Total cold run time: 44172 ms
Total hot run time: 39828 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4358	4360	4355	4355
q2	270	167	173	167
q3	3517	3501	3500	3500
q4	2407	2365	2371	2365
q5	5730	5699	5738	5699
q6	239	125	123	123
q7	2367	1877	1847	1847
q8	3523	3528	3510	3510
q9	9004	9004	8968	8968
q10	3910	4010	4026	4010
q11	495	369	355	355
q12	779	603	610	603
q13	4287	3541	3547	3541
q14	288	263	255	255
q15	573	508	528	508
q16	496	489	457	457
q17	1872	1870	1856	1856
q18	8521	8132	8119	8119
q19	1764	1753	1735	1735
q20	2256	1960	1945	1945
q21	6490	6183	6111	6111
q22	517	422	416	416
Total cold run time: 63663 ms
Total hot run time: 60445 ms

Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 28, 2023
Copy link
Contributor

PR approved by at least one committer and no changes requested.

Copy link
Contributor

PR approved by anyone and no changes requested.

@dataroaring dataroaring merged commit 2e910da into apache:master Dec 28, 2023
27 of 30 checks passed
HappenLee pushed a commit to HappenLee/incubator-doris that referenced this pull request Jan 12, 2024
…er when load finish (apache#28901)

[Goal]
When building the rowset writer, avoid waiting for inflight segcompaction
to elimite long tail latency for load.

[Current situation]
1. The segcompaction of a rowset is executed serially. During the build phase,
we need to wait for the completion of the inflight segcompaction task.

2. If the rowset writer finishes writing and starts building meta, then segments
that have not been compacted will not be submitted to segcompaction worker.
We simply ignore them to accelerate the build process.

3. But this is not enough. If a segcompaction task has already been submitted to
the worker thread pool, we will set a cancelled flag for the worker,
and nothing will be done during execution to complete the task ASAP.

4. But this is still not enough. Although the latency of the segcompaction task
has been shortened by aforemetioned method, tasks may still be queuing in the
thread pool.

[Solution]
We can increase the worker thread pool to avoid queuing congestion, but this is
not the best solution.
Segcompaction should be a best effort work, and should not use too many CPU and
memory resources. So we adopted the strategy of unbinding build and segcompaction,
specifically:

1. For the segcompaction task that is performing compaction operations, we should
not interrupt it, otherwise it may cause file corruption

2. For those tasks still queued, we no longer care about their results (because
these tasks will know they are cancelled and will not perform any actual operations),
so we just ignore them and continue with the subsequent rowset build process

Signed-off-by: freemandealer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants