Skip to content

Conversation

@kaijchen
Copy link
Member

@kaijchen kaijchen commented Aug 8, 2024

Proposed changes

Previously the bvar g_load_stream_file_writer_cnt is not accurate when error happens.
Some FileWriters are not supposed to be closed manually.

All FileWriter in LoadStreamWriter is managed by std::unique_ptr.
A FileWriter should be able to close itself and cleanup storage state on its deconstructor.

This PR changes the bvar g_load_stream_file_writer_cnt to track deconstruction of FileWriter,
instead of tracking FileWriter close.

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

@kaijchen
Copy link
Member Author

kaijchen commented Aug 8, 2024

run buildall

@github-actions github-actions bot added the doing label Aug 8, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17603	4366	4310	4310
q2	2020	181	190	181
q3	10468	1194	1130	1130
q4	10142	793	779	779
q5	7506	2572	2552	2552
q6	220	137	136	136
q7	962	587	593	587
q8	9221	1918	1900	1900
q9	8580	6588	6544	6544
q10	7061	2253	2130	2130
q11	459	235	249	235
q12	476	216	215	215
q13	18357	3000	2970	2970
q14	280	229	246	229
q15	521	486	490	486
q16	503	383	384	383
q17	966	727	739	727
q18	8108	7424	7522	7424
q19	4300	1026	1059	1026
q20	666	316	326	316
q21	5393	4588	4608	4588
q22	1123	1002	985	985
Total cold run time: 114935 ms
Total hot run time: 39833 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4399	4280	4268	4268
q2	382	264	269	264
q3	2860	2608	2710	2608
q4	2003	1752	1760	1752
q5	5559	5524	5447	5447
q6	236	131	127	127
q7	2134	1741	1810	1741
q8	3266	3417	3470	3417
q9	8732	8722	8898	8722
q10	3506	3292	3259	3259
q11	571	491	497	491
q12	826	644	647	644
q13	16540	3174	3159	3159
q14	327	278	303	278
q15	528	495	482	482
q16	492	450	436	436
q17	1849	1498	1511	1498
q18	8126	7952	7871	7871
q19	2569	1525	1530	1525
q20	2129	1898	1870	1870
q21	5520	5343	5157	5157
q22	1125	1064	1037	1037
Total cold run time: 73679 ms
Total hot run time: 56053 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 Aug 8, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024

PR approved by anyone and no changes requested.

@kaijchen
Copy link
Member Author

kaijchen commented Aug 8, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2024

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17771	5464	4433	4433
q2	2013	191	195	191
q3	10525	1215	1229	1215
q4	10175	780	715	715
q5	7548	2635	2610	2610
q6	245	144	134	134
q7	1032	599	592	592
q8	9223	2028	2024	2024
q9	8834	6827	6798	6798
q10	7125	2278	2246	2246
q11	457	241	235	235
q12	410	215	213	213
q13	17768	2982	3005	2982
q14	288	239	228	228
q15	530	482	482	482
q16	526	389	385	385
q17	1037	786	732	732
q18	8312	7670	7489	7489
q19	4525	1176	1180	1176
q20	708	331	320	320
q21	5631	4723	4620	4620
q22	1130	1037	992	992
Total cold run time: 115813 ms
Total hot run time: 40812 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4602	4432	4442	4432
q2	399	283	270	270
q3	3021	2811	2889	2811
q4	2056	1791	1724	1724
q5	5620	5623	5481	5481
q6	246	137	133	133
q7	2250	1778	1732	1732
q8	3480	3714	3706	3706
q9	8882	8954	8843	8843
q10	3659	3262	3325	3262
q11	625	503	521	503
q12	847	633	642	633
q13	17249	3164	3172	3164
q14	321	277	305	277
q15	544	496	491	491
q16	503	448	439	439
q17	1983	1596	1606	1596
q18	8218	8115	7894	7894
q19	3058	1737	1927	1737
q20	2141	1892	1861	1861
q21	5733	5333	5493	5333
q22	1117	1023	1073	1023
Total cold run time: 76554 ms
Total hot run time: 57345 ms

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17622	4395	4290	4290
q2	2019	172	172	172
q3	10504	1190	1106	1106
q4	10146	736	752	736
q5	7504	2560	2484	2484
q6	223	136	136	136
q7	956	584	583	583
q8	9215	1888	1909	1888
q9	8626	6558	6511	6511
q10	7053	2209	2191	2191
q11	452	239	236	236
q12	436	224	216	216
q13	17763	3012	3032	3012
q14	273	238	232	232
q15	529	491	476	476
q16	514	393	391	391
q17	952	699	719	699
q18	8023	7465	7318	7318
q19	4346	959	904	904
q20	702	323	337	323
q21	5308	4517	4667	4517
q22	1135	1020	1028	1020
Total cold run time: 114301 ms
Total hot run time: 39441 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4506	4258	4227	4227
q2	374	285	268	268
q3	2854	2673	2674	2673
q4	2028	1752	1722	1722
q5	5560	5621	5477	5477
q6	227	131	135	131
q7	2197	1773	1753	1753
q8	3310	3462	3419	3419
q9	8809	8758	8920	8758
q10	3531	3325	3342	3325
q11	596	495	502	495
q12	799	619	578	578
q13	16867	3233	3205	3205
q14	316	294	290	290
q15	539	481	500	481
q16	490	442	433	433
q17	1851	1519	1513	1513
q18	7986	7832	7937	7832
q19	1714	1586	1549	1549
q20	2158	1933	1881	1881
q21	10373	5406	5293	5293
q22	1125	1090	1044	1044
Total cold run time: 78210 ms
Total hot run time: 56347 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 204314 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 4a25bd61e7dddbbca259856c9b9791bc99b7dc47, data reload: false

query1	959	407	391	391
query2	6442	1833	1850	1833
query3	6659	205	224	205
query4	34568	23194	23019	23019
query5	3615	494	506	494
query6	275	172	167	167
query7	4568	292	303	292
query8	250	198	214	198
query9	8499	2386	2357	2357
query10	962	920	906	906
query11	15803	15047	14980	14980
query12	141	99	103	99
query13	1634	390	373	373
query14	10732	8258	8197	8197
query15	416	335	324	324
query16	7679	464	494	464
query17	1717	610	594	594
query18	2046	435	422	422
query19	280	227	228	227
query20	131	114	112	112
query21	206	112	109	109
query22	4509	4429	4492	4429
query23	34463	33937	34060	33937
query24	11269	2917	2936	2917
query25	590	392	388	388
query26	841	163	159	159
query27	2174	286	285	285
query28	6656	2016	1986	1986
query29	791	405	402	402
query30	259	155	144	144
query31	1015	736	717	717
query32	93	52	57	52
query33	724	281	279	279
query34	902	482	484	482
query35	923	898	851	851
query36	1095	922	898	898
query37	143	84	79	79
query38	4344	4186	4163	4163
query39	1410	1394	1388	1388
query40	222	118	115	115
query41	46	43	44	43
query42	119	96	98	96
query43	507	441	462	441
query44	1284	734	740	734
query45	422	394	366	366
query46	1118	782	787	782
query47	1814	1794	1764	1764
query48	379	297	304	297
query49	847	428	439	428
query50	826	410	419	410
query51	6814	6644	6648	6644
query52	99	97	87	87
query53	263	178	179	178
query54	886	470	472	470
query55	74	77	76	76
query56	281	247	246	246
query57	1143	1047	1062	1047
query58	229	234	230	230
query59	2912	2768	2596	2596
query60	298	269	265	265
query61	97	104	96	96
query62	802	646	654	646
query63	214	178	179	178
query64	9315	2443	1935	1935
query65	3207	3163	3149	3149
query66	743	346	323	323
query67	15282	14750	14679	14679
query68	4562	559	556	556
query69	416	382	424	382
query70	1190	1150	1161	1150
query71	461	284	280	280
query72	19880	16827	17015	16827
query73	771	332	326	326
query74	9154	8829	8772	8772
query75	4269	2647	2689	2647
query76	3257	1022	947	947
query77	673	306	322	306
query78	9791	8956	8850	8850
query79	4244	538	523	523
query80	1115	508	502	502
query81	591	229	233	229
query82	943	139	141	139
query83	294	154	149	149
query84	277	81	78	78
query85	1482	305	325	305
query86	446	310	286	286
query87	4625	4493	4452	4452
query88	4817	2495	2480	2480
query89	434	301	288	288
query90	2046	199	198	198
query91	148	118	116	116
query92	66	51	50	50
query93	5298	533	536	533
query94	996	296	294	294
query95	358	264	263	263
query96	617	284	283	283
query97	3178	3083	3025	3025
query98	228	206	197	197
query99	1632	1241	1267	1241
Total cold run time: 318285 ms
Total hot run time: 204314 ms

@doris-robot
Copy link

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 4a25bd61e7dddbbca259856c9b9791bc99b7dc47, data reload: false

query1	0.05	0.04	0.04
query2	0.08	0.05	0.05
query3	0.22	0.05	0.06
query4	1.66	0.08	0.09
query5	0.49	0.48	0.47
query6	1.13	0.73	0.73
query7	0.02	0.01	0.01
query8	0.05	0.04	0.04
query9	0.55	0.48	0.48
query10	0.54	0.55	0.54
query11	0.16	0.12	0.12
query12	0.16	0.12	0.13
query13	0.59	0.60	0.59
query14	0.78	0.78	0.79
query15	0.86	0.82	0.82
query16	0.37	0.38	0.37
query17	0.98	1.00	0.96
query18	0.23	0.23	0.22
query19	1.86	1.67	1.81
query20	0.01	0.01	0.02
query21	15.39	0.77	0.67
query22	4.25	7.51	1.66
query23	18.27	1.39	1.33
query24	2.07	0.23	0.21
query25	0.14	0.08	0.08
query26	0.31	0.22	0.21
query27	0.45	0.23	0.22
query28	13.25	1.02	0.98
query29	12.62	3.36	3.32
query30	0.23	0.06	0.05
query31	2.90	0.39	0.39
query32	3.28	0.47	0.46
query33	2.93	2.97	3.05
query34	16.87	4.35	4.39
query35	4.43	4.45	4.49
query36	0.64	0.48	0.49
query37	0.19	0.16	0.16
query38	0.15	0.15	0.15
query39	0.05	0.03	0.04
query40	0.16	0.12	0.12
query41	0.08	0.05	0.05
query42	0.06	0.04	0.04
query43	0.05	0.04	0.04
Total cold run time: 109.56 s
Total hot run time: 30.57 s

@liaoxin01 liaoxin01 merged commit cd0a8ec into apache:master Aug 9, 2024
dataroaring pushed a commit that referenced this pull request Aug 11, 2024
Previously the bvar `g_load_stream_file_writer_cnt` is not accurate when
error happens.
Some `FileWriters` are not supposed to be closed manually.

All `FileWriter` in `LoadStreamWriter` is managed by `std::unique_ptr`.
A `FileWriter` should be able to close itself and cleanup storage state
on its deconstructor.

This PR changes the bvar `g_load_stream_file_writer_cnt` to track
deconstruction of `FileWriter`,
instead of tracking `FileWriter` close.
wyxxxcat pushed a commit to wyxxxcat/doris that referenced this pull request Aug 14, 2024
…9075)

Previously the bvar `g_load_stream_file_writer_cnt` is not accurate when
error happens.
Some `FileWriters` are not supposed to be closed manually.

All `FileWriter` in `LoadStreamWriter` is managed by `std::unique_ptr`.
A `FileWriter` should be able to close itself and cleanup storage state
on its deconstructor.

This PR changes the bvar `g_load_stream_file_writer_cnt` to track
deconstruction of `FileWriter`,
instead of tracking `FileWriter` close.
dataroaring pushed a commit that referenced this pull request Aug 16, 2024
Previously the bvar `g_load_stream_file_writer_cnt` is not accurate when
error happens.
Some `FileWriters` are not supposed to be closed manually.

All `FileWriter` in `LoadStreamWriter` is managed by `std::unique_ptr`.
A `FileWriter` should be able to close itself and cleanup storage state
on its deconstructor.

This PR changes the bvar `g_load_stream_file_writer_cnt` to track
deconstruction of `FileWriter`,
instead of tracking `FileWriter` close.
kaijchen added a commit to kaijchen/doris that referenced this pull request Sep 19, 2024
kaijchen added a commit to kaijchen/doris that referenced this pull request Sep 19, 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.7-merged dev/3.0.2-merged doing reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants