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.

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

[bugfix](asyncwriter) async writer's lock should not include finish or close method #33077

Merged
merged 2 commits into from Apr 1, 2024
Merged

[bugfix](asyncwriter) async writer's lock should not include finish or close method #33077

merged 2 commits into from Apr 1, 2024

Conversation

yiguolei
Copy link
Contributor

@yiguolei yiguolei commented Mar 31, 2024 •
edited

Proposed changes

  1. close or finish method will take a lot of time, and the lock will hold a lot of time. If there is a bug in close or finish method, it will affect pipeline execute thread.
  2. writer's close method will need this lock, so that it will hang when close method is called.

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

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.

@yiguolei
Copy link
Contributor Author

run buildall

1 similar comment
@yiguolei
Copy link
Contributor Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17875	4252	4190	4190
q2	2429	192	190	190
q3	12042	1249	1415	1249
q4	10218	851	989	851
q5	7481	2971	2933	2933
q6	215	134	132	132
q7	1104	626	614	614
q8	9415	2059	2029	2029
q9	6789	6265	6163	6163
q10	8453	3529	3496	3496
q11	421	242	244	242
q12	386	222	212	212
q13	17785	2901	2894	2894
q14	272	246	244	244
q15	542	492	488	488
q16	488	387	380	380
q17	949	920	894	894
q18	7335	6542	6509	6509
q19	1597	1524	1537	1524
q20	609	311	305	305
q21	3471	3097	3092	3092
q22	361	295	311	295
Total cold run time: 110237 ms
Total hot run time: 38926 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4131	4040	4104	4040
q2	332	228	222	222
q3	2964	2954	2981	2954
q4	1917	1876	1833	1833
q5	5217	5197	5196	5196
q6	210	126	123	123
q7	2249	1844	1813	1813
q8	3211	3278	3285	3278
q9	8448	8460	8472	8460
q10	3767	3803	3837	3803
q11	541	446	431	431
q12	708	542	557	542
q13	16765	2874	2893	2874
q14	289	256	262	256
q15	514	494	489	489
q16	469	425	420	420
q17	1726	1676	1692	1676
q18	7701	7258	7194	7194
q19	1637	1654	1635	1635
q20	1940	1735	1728	1728
q21	5085	4770	4764	4764
q22	485	430	427	427
Total cold run time: 70306 ms
Total hot run time: 54158 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.55% (8839/24864)
Line Coverage: 27.29% (72483/265626)
Region Coverage: 26.50% (37519/141586)
Branch Coverage: 23.30% (19125/82076)
Coverage Report: http://coverage.selectdb-in.cc/coverage/68397d6e27d698fceac40a4cf369bc0580d2741f_68397d6e27d698fceac40a4cf369bc0580d2741f/report/index.html

@doris-robot
Copy link

TPC-DS: Total hot run time: 181111 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 68397d6e27d698fceac40a4cf369bc0580d2741f, data reload: false

query1	1217	1111	1126	1111
query2	6489	1736	1754	1736
query3	6662	215	226	215
query4	25169	21511	21625	21511
query5	4232	413	406	406
query6	286	199	191	191
query7	4604	306	308	306
query8	238	178	194	178
query9	8489	2256	2241	2241
query10	551	250	279	250
query11	14953	14662	14492	14492
query12	145	93	99	93
query13	1628	372	392	372
query14	8635	6880	6771	6771
query15	204	176	186	176
query16	7167	281	273	273
query17	998	615	589	589
query18	1917	293	300	293
query19	221	168	170	168
query20	99	97	95	95
query21	198	128	127	127
query22	4920	4811	4778	4778
query23	33624	32681	32492	32492
query24	12555	3152	3120	3120
query25	700	409	408	408
query26	1912	171	165	165
query27	3052	325	342	325
query28	6711	1828	1828	1828
query29	1348	628	611	611
query30	306	156	158	156
query31	1042	738	725	725
query32	99	66	60	60
query33	727	285	277	277
query34	985	497	493	493
query35	840	711	696	696
query36	1019	879	889	879
query37	285	78	77	77
query38	3593	3405	3409	3405
query39	1585	1677	1514	1514
query40	314	144	139	139
query41	51	48	46	46
query42	116	109	103	103
query43	432	392	406	392
query44	1101	720	710	710
query45	280	258	278	258
query46	1074	816	788	788
query47	1915	1794	1796	1794
query48	402	325	321	321
query49	1152	375	373	373
query50	819	394	406	394
query51	6728	6670	6617	6617
query52	108	104	100	100
query53	366	300	292	292
query54	329	240	250	240
query55	103	90	88	88
query56	274	243	230	230
query57	1232	1139	1119	1119
query58	242	237	240	237
query59	2555	2265	2181	2181
query60	271	252	272	252
query61	113	122	118	118
query62	706	447	450	447
query63	318	290	287	287
query64	6522	3195	3154	3154
query65	3130	3029	3026	3026
query66	1475	357	339	339
query67	15386	14859	14928	14859
query68	8552	576	577	576
query69	564	334	336	334
query70	1220	1120	1119	1119
query71	501	284	271	271
query72	6379	2592	2457	2457
query73	808	339	342	339
query74	6687	6325	6389	6325
query75	3454	2317	2292	2292
query76	5182	1132	1206	1132
query77	609	263	254	254
query78	10771	10161	10038	10038
query79	9393	533	540	533
query80	1246	438	424	424
query81	498	237	230	230
query82	371	100	102	100
query83	230	164	163	163
query84	266	90	95	90
query85	1073	297	284	284
query86	378	313	299	299
query87	3789	3535	3490	3490
query88	3702	2318	2309	2309
query89	557	384	378	378
query90	2035	182	182	182
query91	143	112	109	109
query92	61	53	59	53
query93	6617	538	521	521
query94	1373	197	197	197
query95	438	350	334	334
query96	628	274	273	273
query97	2668	2466	2477	2466
query98	233	218	217	217
query99	1234	815	846	815
Total cold run time: 298579 ms
Total hot run time: 181111 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.07	0.04	0.04
query3	0.24	0.04	0.05
query4	1.66	0.06	0.06
query5	0.47	0.48	0.48
query6	1.15	0.66	0.66
query7	0.02	0.01	0.02
query8	0.05	0.04	0.04
query9	0.56	0.52	0.51
query10	0.56	0.56	0.57
query11	0.15	0.11	0.11
query12	0.14	0.12	0.12
query13	0.60	0.60	0.60
query14	0.77	0.79	0.78
query15	0.87	0.84	0.86
query16	0.36	0.36	0.37
query17	0.97	0.97	1.02
query18	0.25	0.26	0.24
query19	1.85	1.72	1.72
query20	0.01	0.01	0.02
query21	15.55	0.77	0.71
query22	3.16	5.00	1.49
query23	17.51	1.18	1.13
query24	1.47	0.20	0.36
query25	0.13	0.10	0.08
query26	0.30	0.17	0.20
query27	0.09	0.10	0.07
query28	13.67	1.00	0.93
query29	12.73	3.29	3.31
query30	0.28	0.08	0.07
query31	2.80	0.39	0.39
query32	3.28	0.47	0.47
query33	2.89	2.86	2.88
query34	15.52	4.33	4.34
query35	4.37	4.36	4.36
query36	0.69	0.48	0.48
query37	0.20	0.19	0.18
query38	0.18	0.18	0.16
query39	0.05	0.04	0.05
query40	0.18	0.16	0.14
query41	0.09	0.06	0.05
query42	0.06	0.05	0.05
query43	0.05	0.04	0.05
Total cold run time: 106.04 s
Total hot run time: 29.87 s

@yiguolei
Copy link
Contributor Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 35.55% (8840/24864)
Line Coverage: 27.28% (72476/265631)
Region Coverage: 26.49% (37507/141593)
Branch Coverage: 23.30% (19124/82080)
Coverage Report: http://coverage.selectdb-in.cc/coverage/0fcc5670be4538abd597454d63b73cc08e648581_0fcc5670be4538abd597454d63b73cc08e648581/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17619	4109	4084	4084
q2	2095	190	180	180
q3	10489	1237	1396	1237
q4	10200	879	1012	879
q5	7463	2966	2913	2913
q6	217	131	133	131
q7	1085	619	598	598
q8	9410	2021	2013	2013
q9	6648	6171	6101	6101
q10	8439	3501	3502	3501
q11	425	258	229	229
q12	379	222	207	207
q13	17771	2884	2891	2884
q14	267	233	246	233
q15	519	474	473	473
q16	498	386	379	379
q17	939	892	862	862
q18	7165	6394	6406	6394
q19	1589	1519	1549	1519
q20	597	312	303	303
q21	3524	3100	3097	3097
q22	356	308	315	308
Total cold run time: 107694 ms
Total hot run time: 38525 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4052	4015	4027	4015
q2	329	219	227	219
q3	2951	2937	2919	2919
q4	1889	1824	1829	1824
q5	5224	5211	5202	5202
q6	210	124	125	124
q7	2235	1812	1803	1803
q8	3199	3284	3252	3252
q9	8442	8421	8403	8403
q10	3742	3866	3976	3866
q11	547	463	464	463
q12	740	589	593	589
q13	15203	3063	3079	3063
q14	316	275	280	275
q15	555	507	506	506
q16	510	446	460	446
q17	1746	1737	1729	1729
q18	8367	7895	7582	7582
q19	1670	1677	1694	1677
q20	1998	1886	1799	1799
q21	5131	4954	4970	4954
q22	484	427	439	427
Total cold run time: 69540 ms
Total hot run time: 55137 ms

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Mar 31, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

TPC-DS: Total hot run time: 181768 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 0fcc5670be4538abd597454d63b73cc08e648581, data reload: false

query1	1240	1120	1118	1118
query2	6279	2090	1928	1928
query3	6923	213	210	210
query4	24902	21506	21518	21506
query5	4164	397	406	397
query6	284	205	185	185
query7	4617	289	306	289
query8	226	185	174	174
query9	8485	2225	2217	2217
query10	451	245	251	245
query11	14840	14503	14401	14401
query12	142	94	94	94
query13	1628	372	384	372
query14	8484	6923	6828	6828
query15	218	173	174	173
query16	6834	281	279	279
query17	976	604	602	602
query18	1872	302	307	302
query19	220	175	168	168
query20	102	95	96	95
query21	202	137	136	136
query22	5174	4978	4857	4857
query23	33280	32287	32621	32287
query24	13007	3229	3203	3203
query25	731	454	434	434
query26	1916	171	177	171
query27	3426	382	380	380
query28	7095	1859	1882	1859
query29	1294	616	647	616
query30	311	163	166	163
query31	1004	759	757	757
query32	105	62	72	62
query33	719	246	267	246
query34	1472	517	512	512
query35	890	756	740	740
query36	1010	853	861	853
query37	176	85	82	82
query38	3684	3628	3581	3581
query39	1664	1597	1631	1597
query40	257	142	152	142
query41	48	46	47	46
query42	117	107	118	107
query43	450	403	396	396
query44	1198	718	712	712
query45	289	282	271	271
query46	1097	828	812	812
query47	1996	1917	1962	1917
query48	393	339	313	313
query49	998	382	398	382
query50	832	415	409	409
query51	6834	6749	6810	6749
query52	113	107	100	100
query53	377	297	299	297
query54	309	244	245	244
query55	88	85	81	81
query56	248	239	246	239
query57	1272	1226	1196	1196
query58	252	238	227	227
query59	2549	2435	2323	2323
query60	257	231	235	231
query61	94	91	90	90
query62	654	461	450	450
query63	307	277	280	277
query64	5792	3058	3515	3058
query65	3016	3008	2999	2999
query66	1228	345	329	329
query67	15458	14830	14794	14794
query68	9301	576	576	576
query69	571	329	323	323
query70	1367	1085	1089	1085
query71	525	278	269	269
query72	6445	2613	2452	2452
query73	1536	326	325	325
query74	6801	6284	6336	6284
query75	3748	2297	2262	2262
query76	5635	1159	1217	1159
query77	572	249	248	248
query78	10868	10109	10081	10081
query79	8920	522	532	522
query80	1537	430	433	430
query81	482	226	221	221
query82	356	105	101	101
query83	212	165	162	162
query84	268	94	87	87
query85	928	302	299	299
query86	364	259	278	259
query87	3635	3490	3491	3490
query88	3578	2308	2303	2303
query89	555	376	366	366
query90	2063	172	172	172
query91	137	107	109	107
query92	62	52	50	50
query93	6480	518	515	515
query94	1079	200	194	194
query95	437	335	321	321
query96	604	270	270	270
query97	2629	2460	2472	2460
query98	252	215	213	213
query99	1326	852	869	852
Total cold run time: 299843 ms
Total hot run time: 181768 ms

dataroaring
dataroaring approved these changes Mar 31, 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

@doris-robot
Copy link

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

query1	0.04	0.04	0.04
query2	0.08	0.04	0.04
query3	0.23	0.04	0.05
query4	1.69	0.06	0.06
query5	0.48	0.48	0.51
query6	1.13	0.65	0.66
query7	0.02	0.01	0.01
query8	0.06	0.04	0.04
query9	0.56	0.51	0.51
query10	0.56	0.56	0.55
query11	0.14	0.11	0.11
query12	0.14	0.12	0.12
query13	0.60	0.60	0.60
query14	0.78	0.77	0.82
query15	0.86	0.85	0.84
query16	0.36	0.36	0.36
query17	0.97	0.98	0.99
query18	0.25	0.25	0.25
query19	1.79	1.75	1.72
query20	0.01	0.01	0.01
query21	15.54	0.74	0.71
query22	3.09	5.72	1.82
query23	17.47	1.20	1.13
query24	1.42	0.38	0.22
query25	0.14	0.09	0.09
query26	0.28	0.18	0.19
query27	0.09	0.09	0.09
query28	13.49	0.97	0.97
query29	12.65	3.40	3.47
query30	0.28	0.09	0.08
query31	2.82	0.39	0.39
query32	3.29	0.48	0.47
query33	2.91	2.92	2.87
query34	15.50	4.34	4.35
query35	4.37	4.36	4.34
query36	0.68	0.47	0.48
query37	0.19	0.19	0.16
query38	0.18	0.15	0.17
query39	0.04	0.03	0.04
query40	0.18	0.14	0.16
query41	0.09	0.06	0.05
query42	0.06	0.05	0.06
query43	0.04	0.05	0.04
Total cold run time: 105.55 s
Total hot run time: 30.37 s

@doris-robot
Copy link

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

Load test result on commit 0fcc5670be4538abd597454d63b73cc08e648581 with default session variables
Stream load json:         18 seconds loaded 2358488459 Bytes, about 124 MB/s
Stream load orc:          59 seconds loaded 1101869774 Bytes, about 17 MB/s
Stream load parquet:      32 seconds loaded 861443392 Bytes, about 25 MB/s
Insert into select:       16.4 seconds inserted 10000000 Rows, about 609K ops/s

@yiguolei yiguolei merged commit 607fe88 into apache:master Apr 1, 2024
27 of 30 checks passed
yiguolei added a commit that referenced this pull request Apr 2, 2024
@yiguolei @Doris-Extras
[bugfix](asyncwriter) async writer's lock should not include finish o…
6150c54 
…r close method (#33077)

close or finish method will take a lot of time, and the lock will hold a lot of time. If there is a bug in close or finish method, it will affect pipeline execute thread.
writer's close method will need this lock, so that it will hang when close method is called.
@yiguolei yiguolei mentioned this pull request Apr 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gabriel39 Gabriel39 Gabriel39 approved these changes

@dataroaring dataroaring dataroaring approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@yiguolei @doris-robot @Gabriel39 @dataroaring @Doris-Extras