Skip to content

[fix](move-memtable) fix initial use count of streams for auto partition #33165

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
Merged
liaoxin01 merged 3 commits into from
Apr 3, 2024
Merged

[fix](move-memtable) fix initial use count of streams for auto partition #33165

liaoxin01 merged 3 commits into from
Apr 3, 2024

Conversation

@kaijchen
Copy link
Member

@kaijchen kaijchen commented Apr 2, 2024 •
edited
Loading

Proposed changes

In auto partition, new streams may be created after some sinks closed.
But the initial use_cnt of those streams are still num_local_sinks,
which causes use_cnt cannot reach 0 and CLOSE_LOAD will never be sent.

This PR fixes this problem by removing use_cnt from individual streams.
Sinks will now rely on the use_cnt in each LoadStreamMap to determine whether this is the last sink.
Only the last sink will handle the close_load, close_wait and commit_info.

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@doris-robot
Copy link

doris-robot commented Apr 2, 2024

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.

@kaijchen kaijchen marked this pull request as draft April 2, 2024 09:14
github-actions[bot]
github-actions bot reviewed Apr 2, 2024
View reviewed changes
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

@kaijchen kaijchen changed the title [fix](move-memtable)(auto partition) fix use_cnt when incremental open streams after some sink closed [fix](move-memtable) fix load stream stub initial use_cnt for auto partition Apr 3, 2024
github-actions[bot]
github-actions bot reviewed Apr 3, 2024
View reviewed changes
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

@kaijchen kaijchen changed the title [fix](move-memtable) fix load stream stub initial use_cnt for auto partition [fix](move-memtable) fix initial use count of streams for auto partition Apr 3, 2024
@kaijchen
Copy link
Member Author

kaijchen commented Apr 3, 2024

run buildall

1 similar comment
@kaijchen
Copy link
Member Author

kaijchen commented Apr 3, 2024

run buildall

@kaijchen kaijchen marked this pull request as ready for review April 3, 2024 08:19
@doris-robot
Copy link

doris-robot commented Apr 3, 2024

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

------ Round 1 ----------------------------------
q1	17617	4101	4074	4074
q2	2020	191	183	183
q3	10482	1251	1424	1251
q4	10214	900	1041	900
q5	7473	2991	2914	2914
q6	222	144	136	136
q7	1127	614	597	597
q8	9401	2040	2039	2039
q9	6931	6207	6185	6185
q10	8487	3523	3503	3503
q11	417	246	233	233
q12	380	216	209	209
q13	17792	2893	2887	2887
q14	282	239	248	239
q15	535	489	485	485
q16	506	383	383	383
q17	969	939	881	881
q18	7293	6503	6572	6503
q19	1594	1549	1548	1548
q20	610	322	298	298
q21	3555	3178	3123	3123
q22	347	330	306	306
Total cold run time: 108254 ms
Total hot run time: 38877 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4041	4045	4051	4045
q2	333	219	221	219
q3	2969	2984	2970	2970
q4	1874	1866	1848	1848
q5	5260	5230	5260	5230
q6	212	124	123	123
q7	2274	1838	1833	1833
q8	3228	3321	3308	3308
q9	8515	8483	8472	8472
q10	3756	3878	3835	3835
q11	544	461	475	461
q12	691	544	540	540
q13	9387	2890	2863	2863
q14	307	263	275	263
q15	518	472	479	472
q16	449	399	395	395
q17	1749	1706	1677	1677
q18	7648	7282	7224	7224
q19	1643	1648	1643	1643
q20	1943	1719	1715	1715
q21	4995	4709	4723	4709
q22	482	423	425	423
Total cold run time: 62818 ms
Total hot run time: 54268 ms

@doris-robot
Copy link

doris-robot commented Apr 3, 2024

TeamCity be ut coverage result:
Function Coverage: 35.64% (8882/24922)
Line Coverage: 27.37% (72915/266393)
Region Coverage: 26.55% (37703/142011)
Branch Coverage: 23.35% (19214/82274)
Coverage Report: http://coverage.selectdb-in.cc/coverage/e4e88caa1e4ac1c4d0d8ef3525546c2cc81baca9_e4e88caa1e4ac1c4d0d8ef3525546c2cc81baca9/report/index.html

@doris-robot
Copy link

doris-robot commented Apr 3, 2024

TPC-DS: Total hot run time: 180866 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit e4e88caa1e4ac1c4d0d8ef3525546c2cc81baca9, data reload: false

query1	1219	1112	1112	1112
query2	6434	1854	1750	1750
query3	6695	219	230	219
query4	24757	21475	21517	21475
query5	4258	405	414	405
query6	273	185	185	185
query7	4615	312	298	298
query8	231	181	189	181
query9	8476	2250	2230	2230
query10	590	257	266	257
query11	14936	14400	14443	14400
query12	145	105	98	98
query13	1646	381	380	380
query14	8702	6936	6827	6827
query15	210	182	182	182
query16	6711	275	277	275
query17	997	617	575	575
query18	1577	297	280	280
query19	206	163	158	158
query20	102	96	93	93
query21	200	125	126	125
query22	4920	4776	4745	4745
query23	33553	32833	32801	32801
query24	12513	3199	3171	3171
query25	657	398	396	396
query26	1822	159	155	155
query27	3015	336	328	328
query28	6680	1836	1814	1814
query29	1146	575	574	574
query30	290	165	167	165
query31	1041	731	723	723
query32	82	60	62	60
query33	710	246	234	234
query34	1065	480	506	480
query35	845	701	704	701
query36	987	866	880	866
query37	235	80	76	76
query38	3521	3404	3395	3395
query39	1572	1531	1545	1531
query40	290	136	131	131
query41	47	43	45	43
query42	106	113	101	101
query43	429	392	406	392
query44	1066	700	697	697
query45	269	260	254	254
query46	1061	801	762	762
query47	1920	1816	1804	1804
query48	373	301	299	299
query49	1110	357	355	355
query50	794	387	388	387
query51	6748	6786	6574	6574
query52	113	103	98	98
query53	354	292	290	290
query54	298	239	244	239
query55	101	86	75	75
query56	239	221	224	221
query57	1224	1129	1126	1126
query58	256	218	222	218
query59	2480	2165	2251	2165
query60	269	227	243	227
query61	112	98	100	98
query62	700	474	455	455
query63	312	284	283	283
query64	6124	3493	3237	3237
query65	3060	3005	3008	3005
query66	1518	353	330	330
query67	15260	14810	14833	14810
query68	5171	557	565	557
query69	488	343	335	335
query70	1178	1134	1143	1134
query71	467	282	272	272
query72	6329	2717	2563	2563
query73	718	324	322	322
query74	6795	6457	6253	6253
query75	2931	2310	2296	2296
query76	3316	1064	1205	1064
query77	409	262	268	262
query78	10783	10116	10044	10044
query79	8463	538	537	537
query80	1672	423	418	418
query81	508	245	240	240
query82	684	108	108	108
query83	204	164	160	160
query84	273	90	91	90
query85	1524	288	287	287
query86	449	279	309	279
query87	3722	3550	3479	3479
query88	4317	2264	2281	2264
query89	571	366	362	362
query90	2012	172	180	172
query91	139	109	106	106
query92	62	57	52	52
query93	7202	531	518	518
query94	1184	198	198	198
query95	432	335	327	327
query96	602	271	272	271
query97	2681	2498	2457	2457
query98	242	221	202	202
query99	1339	841	855	841
Total cold run time: 291134 ms
Total hot run time: 180866 ms

@doris-robot
Copy link

doris-robot commented Apr 3, 2024

ClickBench: Total hot run time: 30.57 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit e4e88caa1e4ac1c4d0d8ef3525546c2cc81baca9, data reload: false

query1	0.04	0.03	0.03
query2	0.08	0.04	0.04
query3	0.23	0.04	0.05
query4	1.68	0.07	0.07
query5	0.49	0.48	0.49
query6	1.13	0.65	0.64
query7	0.02	0.01	0.02
query8	0.04	0.04	0.04
query9	0.56	0.52	0.52
query10	0.56	0.58	0.57
query11	0.15	0.11	0.11
query12	0.13	0.11	0.12
query13	0.61	0.60	0.60
query14	0.77	0.79	0.80
query15	0.86	0.85	0.85
query16	0.34	0.36	0.35
query17	0.97	1.01	0.99
query18	0.24	0.25	0.27
query19	1.86	1.72	1.73
query20	0.01	0.01	0.01
query21	15.43	0.78	0.68
query22	3.21	4.79	2.01
query23	17.57	1.22	1.12
query24	1.42	0.32	0.22
query25	0.12	0.09	0.09
query26	0.29	0.15	0.18
query27	0.08	0.08	0.07
query28	13.63	0.97	0.94
query29	12.69	3.50	3.52
query30	0.26	0.07	0.05
query31	2.87	0.38	0.38
query32	3.29	0.48	0.47
query33	2.85	2.85	2.85
query34	15.54	4.33	4.35
query35	4.39	4.37	4.38
query36	0.67	0.48	0.47
query37	0.19	0.17	0.18
query38	0.18	0.16	0.17
query39	0.04	0.04	0.05
query40	0.18	0.14	0.15
query41	0.09	0.05	0.05
query42	0.06	0.05	0.05
query43	0.04	0.04	0.04
Total cold run time: 105.86 s
Total hot run time: 30.57 s

@doris-robot
Copy link

doris-robot commented Apr 3, 2024

Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Load test result on commit e4e88caa1e4ac1c4d0d8ef3525546c2cc81baca9 with default session variables
Stream load json:         19 seconds loaded 2358488459 Bytes, about 118 MB/s
Stream load orc:          59 seconds loaded 1101869774 Bytes, about 17 MB/s
Stream load parquet:      31 seconds loaded 861443392 Bytes, about 26 MB/s
Insert into select:       16.0 seconds inserted 10000000 Rows, about 625K ops/s

liaoxin01
liaoxin01 approved these changes Apr 3, 2024
View reviewed changes
Copy link
Contributor

@liaoxin01 liaoxin01 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 3, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2024

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

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2024

PR approved by anyone and no changes requested.

dataroaring
dataroaring approved these changes Apr 3, 2024
View reviewed changes
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

@liaoxin01 liaoxin01 merged commit c34b52c into apache:master Apr 3, 2024
liaoxin01 pushed a commit to liaoxin01/doris that referenced this pull request Apr 3, 2024
@kaijchen @liaoxin01
[fix](move-memtable) fix initial use count of streams for auto partit…
ee63f4c 
…ion (apache#33165)
liaoxin01 added a commit that referenced this pull request Apr 3, 2024
@liaoxin01 @kaijchen
[fix](move-memtable) fix initial use count of streams for auto partit…
df197c6 
…ion (#33165) (#33236)

Co-authored-by: Kaijie Chen <ckj@apache.org>
yiguolei pushed a commit that referenced this pull request Apr 3, 2024
@Doris-Extras
Revert "[fix](move-memtable) fix initial use count of streams for aut…
0f0567e 
…o partition (#33165) (#33236)"

This reverts commit df197c6.
liaoxin01 pushed a commit to liaoxin01/doris that referenced this pull request Apr 4, 2024
@kaijchen @liaoxin01
[fix](move-memtable) fix initial use count of streams for auto partit…
1f37082 
…ion (apache#33165)
@liaoxin01 liaoxin01 mentioned this pull request Apr 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@liaoxin01 liaoxin01 liaoxin01 approved these changes

@dataroaring dataroaring dataroaring approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/2.1.2-merged reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kaijchen @doris-robot @liaoxin01 @dataroaring