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

[pipelineX](runtime filter) Fix task timeout caused by runtime filter #33332

Merged
merged 8 commits into from
Apr 8, 2024

Conversation

Gabriel39
Copy link
Contributor

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...

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@Gabriel39 Gabriel39 changed the title [pipelineX](runtime filter) Fix task timeout cuased by runtime filter [pipelineX](runtime filter) Fix task timeout caused by runtime filter Apr 7, 2024
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

be/src/vec/exec/runtime_filter_consumer.h Show resolved Hide resolved
be/src/vec/exec/runtime_filter_consumer.h Show resolved Hide resolved
@Gabriel39
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.64% (8887/24933)
Line Coverage: 27.38% (72982/266550)
Region Coverage: 26.54% (37716/142116)
Branch Coverage: 23.35% (19224/82330)
Coverage Report: http://coverage.selectdb-in.cc/coverage/29f6c816a73363222f3ca335cdaa8a78aa2fff2b_29f6c816a73363222f3ca335cdaa8a78aa2fff2b/report/index.html

@doris-robot
Copy link

TPC-H: Total hot run time: 38698 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 29f6c816a73363222f3ca335cdaa8a78aa2fff2b, data reload: false

------ Round 1 ----------------------------------
q1	17635	4097	4075	4075
q2	2015	194	188	188
q3	10474	1256	1395	1256
q4	10202	795	948	795
q5	7467	3032	2971	2971
q6	219	134	132	132
q7	1121	635	602	602
q8	9416	2013	2052	2013
q9	6834	6214	6164	6164
q10	8485	3545	3517	3517
q11	420	245	236	236
q12	392	212	210	210
q13	17788	2868	2919	2868
q14	268	239	240	239
q15	519	479	476	476
q16	503	386	373	373
q17	967	921	892	892
q18	7353	6485	6410	6410
q19	1608	1555	1549	1549
q20	542	300	296	296
q21	3621	3154	3134	3134
q22	351	302	306	302
Total cold run time: 108200 ms
Total hot run time: 38698 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4068	4052	4106	4052
q2	328	222	220	220
q3	2986	2962	2934	2934
q4	1872	1857	1855	1855
q5	5234	5207	5202	5202
q6	207	127	124	124
q7	2256	1826	1791	1791
q8	3225	3316	3287	3287
q9	8447	8468	8482	8468
q10	3789	4016	4021	4016
q11	560	453	464	453
q12	740	599	634	599
q13	16790	3106	3100	3100
q14	300	279	261	261
q15	526	494	481	481
q16	495	450	470	450
q17	1766	1775	1738	1738
q18	8262	7667	7594	7594
q19	1890	1713	1674	1674
q20	2035	1831	1820	1820
q21	5343	4855	4957	4855
q22	500	444	440	440
Total cold run time: 71619 ms
Total hot run time: 55414 ms

@Gabriel39
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

be/src/pipeline/pipeline_x/dependency.cpp Show resolved Hide resolved
be/src/pipeline/pipeline_x/pipeline_x_task.h Show resolved Hide resolved
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

be/src/pipeline/pipeline_x/dependency.cpp Show resolved Hide resolved
// In pipelineX, runtime filters will be ready or timeout before open phase.
if (expected == RuntimeFilterState::NOT_READY) {
DCHECK(MonotonicMillis() - registration_time_ >= wait_times_ms);
_rf_state_atomic = RuntimeFilterState::TIME_OUT;
Copy link
Contributor

Choose a reason for hiding this comment

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

我们为啥要把not_Ready 状态更改为 TIME_OUT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

执行到这里的时候rf状态要么是timeout要么是ready,ready的话状态是已经被更新过的,所以这个地方是timeout。更新状态是因为要在profile显示

@doris-robot
Copy link

TPC-H: Total hot run time: 38804 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 7f65d65cdb1e625094deee258e479d1bd6a942d6, data reload: false

------ Round 1 ----------------------------------
q1	17606	4506	4442	4442
q2	1995	178	172	172
q3	10478	1183	1198	1183
q4	10186	742	776	742
q5	7502	2734	2689	2689
q6	212	133	137	133
q7	1042	602	596	596
q8	9222	2107	2062	2062
q9	7925	6552	6549	6549
q10	8639	3532	3535	3532
q11	460	241	246	241
q12	457	217	212	212
q13	18923	2943	2950	2943
q14	281	242	245	242
q15	518	473	483	473
q16	500	398	380	380
q17	973	637	664	637
q18	7402	6731	6784	6731
q19	7029	1476	1509	1476
q20	1068	314	305	305
q21	3454	2762	2773	2762
q22	359	302	309	302
Total cold run time: 116231 ms
Total hot run time: 38804 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4404	4172	4191	4172
q2	482	270	257	257
q3	2999	2732	2788	2732
q4	1846	1567	1576	1567
q5	5296	5339	5320	5320
q6	209	122	124	122
q7	2279	1883	1883	1883
q8	3251	3359	3343	3343
q9	8658	8658	8578	8578
q10	3843	3732	3728	3728
q11	586	478	479	478
q12	743	595	582	582
q13	17652	2925	2943	2925
q14	305	275	269	269
q15	517	462	473	462
q16	488	438	421	421
q17	1812	1481	1526	1481
q18	8113	7980	7985	7980
q19	1708	1588	1573	1573
q20	2058	1853	1803	1803
q21	11210	4970	4908	4908
q22	554	471	460	460
Total cold run time: 79013 ms
Total hot run time: 55044 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.61% (8885/24948)
Line Coverage: 27.34% (72961/266864)
Region Coverage: 26.50% (37711/142297)
Branch Coverage: 23.32% (19219/82412)
Coverage Report: http://coverage.selectdb-in.cc/coverage/7f65d65cdb1e625094deee258e479d1bd6a942d6_7f65d65cdb1e625094deee258e479d1bd6a942d6/report/index.html

Copy link
Contributor

@yiguolei yiguolei 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 Apr 8, 2024
Copy link
Contributor

github-actions bot commented Apr 8, 2024

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

Copy link
Contributor

github-actions bot commented Apr 8, 2024

PR approved by anyone and no changes requested.

Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

LGTM

@yiguolei yiguolei merged commit 80526d7 into apache:master Apr 8, 2024
27 of 33 checks passed
Gabriel39 added a commit to Gabriel39/incubator-doris that referenced this pull request Apr 8, 2024
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. dev/2.1.2-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants