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

[fix](multi table) do not use strlen to calculate the length of msg (#40367) #40500

Merged
merged 1 commit into from
Sep 7, 2024

Conversation

sollhui
Copy link
Contributor

@sollhui sollhui commented Sep 7, 2024

pick #40367

Meet code dump when using single stream multi table load:

SUMMARY: AddressSanitizer: heap-buffer-overflow /root/doris/be/src/io/fs/multi_table_pipe.cpp:99:22 in doris::io::MultiTablePipe::dispatch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, char const*, unsigned long, doris::Status (doris::io::KafkaConsumerPipe::*)(char const*, unsigned long))
  1. It is hard to guaranteed that msg is a C-style string ending in '\0' character. If not, it may cause the core dump to access memory out of bounds.
  2. It is not need to calculate the length of msg twice.

Therefore, deleting the logic that using strlen to calculate the length of msg.

…pache#40367)

Meet code dump when using single stream multi table load:
```
SUMMARY: AddressSanitizer: heap-buffer-overflow /root/doris/be/src/io/fs/multi_table_pipe.cpp:99:22 in doris::io::MultiTablePipe::dispatch(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>> const&, char const*, unsigned long, doris::Status (doris::io::KafkaConsumerPipe::*)(char const*, unsigned long))
```

1. It is hard to guaranteed that msg is a C-style string ending in '\0'
character. If not, it may cause the core dump to access memory out of
bounds.
2. It is not need to calculate the length of msg twice.

Therefore, deleting the logic that using strlen to calculate the length
of msg.
@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.

@sollhui
Copy link
Contributor Author

sollhui commented Sep 7, 2024

run buildall

Copy link
Contributor

github-actions bot commented Sep 7, 2024

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17762	4522	4525	4522
q2	2090	154	147	147
q3	10438	1912	1880	1880
q4	10289	1244	1303	1244
q5	8495	3934	3896	3896
q6	230	122	123	122
q7	2022	1614	1581	1581
q8	9281	2704	2689	2689
q9	9933	9838	9783	9783
q10	8636	3485	3546	3485
q11	424	236	250	236
q12	471	290	301	290
q13	18321	3988	4003	3988
q14	348	331	320	320
q15	507	450	449	449
q16	543	450	466	450
q17	1120	960	929	929
q18	7183	6925	6816	6816
q19	1659	1493	1479	1479
q20	519	303	277	277
q21	4406	4157	4055	4055
q22	480	390	392	390
Total cold run time: 115157 ms
Total hot run time: 49028 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4519	4513	4497	4497
q2	320	227	221	221
q3	4124	4138	4139	4138
q4	2745	2736	2746	2736
q5	7084	7091	7098	7091
q6	236	119	119	119
q7	3169	2776	2805	2776
q8	4362	4434	4460	4434
q9	13721	13571	13564	13564
q10	4241	4217	4228	4217
q11	754	670	696	670
q12	998	861	837	837
q13	6776	3749	3714	3714
q14	461	415	422	415
q15	512	462	460	460
q16	639	609	598	598
q17	3777	3821	3775	3775
q18	8812	8848	8725	8725
q19	1707	1622	1648	1622
q20	2432	2119	2097	2097
q21	8391	8464	8451	8451
q22	1049	979	891	891
Total cold run time: 80829 ms
Total hot run time: 76048 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.84% (8135/21498)
Line Coverage: 29.59% (66961/226322)
Region Coverage: 29.07% (34550/118831)
Branch Coverage: 24.99% (17806/71266)
Coverage Report: http://coverage.selectdb-in.cc/coverage/d75bdffb0c10e87534f387fb8821078b44513fbf_d75bdffb0c10e87534f387fb8821078b44513fbf/report/index.html

@doris-robot
Copy link

TPC-DS: Total hot run time: 213226 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 d75bdffb0c10e87534f387fb8821078b44513fbf, data reload: false

query1	923	384	426	384
query2	6552	2258	2266	2258
query3	6923	207	206	206
query4	22916	21738	21810	21738
query5	20932	6557	6523	6523
query6	283	219	224	219
query7	4327	297	303	297
query8	259	307	247	247
query9	3099	2649	2577	2577
query10	467	302	301	301
query11	16252	15337	15603	15337
query12	133	77	77	77
query13	1078	465	461	461
query14	18470	13484	13407	13407
query15	371	221	231	221
query16	6866	283	262	262
query17	1927	927	892	892
query18	905	309	313	309
query19	221	146	147	146
query20	80	78	75	75
query21	187	99	91	91
query22	5204	5034	4972	4972
query23	34223	33489	33512	33489
query24	6737	6302	6294	6294
query25	527	428	420	420
query26	791	158	161	158
query27	2240	302	292	292
query28	6082	2312	2235	2235
query29	2860	2689	2763	2689
query30	242	174	168	168
query31	980	770	761	761
query32	70	63	58	58
query33	438	259	256	256
query34	859	469	468	468
query35	1125	898	904	898
query36	1251	1266	1142	1142
query37	92	57	57	57
query38	3056	2922	2913	2913
query39	1371	1316	1325	1316
query40	207	89	91	89
query41	40	37	36	36
query42	84	88	81	81
query43	611	629	562	562
query44	1161	724	716	716
query45	245	228	229	228
query46	1236	949	934	934
query47	1934	1632	1771	1632
query48	509	413	406	406
query49	612	375	371	371
query50	851	629	607	607
query51	4726	4706	4650	4650
query52	85	84	81	81
query53	225	182	180	180
query54	2649	2459	2473	2459
query55	94	92	80	80
query56	229	205	200	200
query57	1244	1128	1066	1066
query58	220	190	216	190
query59	3612	3337	3486	3337
query60	217	203	204	203
query61	95	92	92	92
query62	862	448	533	448
query63	196	179	176	176
query64	2884	1536	1417	1417
query65	3605	3615	3560	3560
query66	734	386	416	386
query67	15718	15802	15936	15802
query68	9561	654	645	645
query69	486	249	272	249
query70	1802	1379	1444	1379
query71	423	310	310	310
query72	6774	4817	4683	4683
query73	777	307	318	307
query74	6328	5951	5848	5848
query75	5079	3822	3768	3768
query76	5120	1177	1060	1060
query77	874	253	258	253
query78	12416	12159	11534	11534
query79	12482	660	631	631
query80	888	373	379	373
query81	491	237	231	231
query82	1253	95	97	95
query83	164	130	130	130
query84	263	70	68	68
query85	969	320	310	310
query86	311	316	299	299
query87	3288	3013	3008	3008
query88	5352	2282	2285	2282
query89	459	301	294	294
query90	1933	212	212	212
query91	168	124	131	124
query92	56	48	52	48
query93	6060	566	555	555
query94	656	207	200	200
query95	1961	2017	1968	1968
query96	654	327	322	322
query97	6462	6433	6375	6375
query98	227	209	209	209
query99	3073	887	853	853
Total cold run time: 323014 ms
Total hot run time: 213226 ms

@doris-robot
Copy link

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

query1	0.02	0.02	0.02
query2	0.07	0.02	0.02
query3	0.25	0.04	0.04
query4	1.81	0.07	0.06
query5	0.54	0.53	0.52
query6	1.24	0.61	0.62
query7	0.01	0.01	0.01
query8	0.03	0.02	0.02
query9	0.53	0.47	0.48
query10	0.54	0.53	0.54
query11	0.13	0.08	0.08
query12	0.12	0.09	0.09
query13	0.62	0.62	0.60
query14	0.78	0.79	0.79
query15	0.78	0.75	0.76
query16	0.39	0.37	0.36
query17	1.00	0.99	1.02
query18	0.21	0.25	0.26
query19	1.91	1.82	1.84
query20	0.01	0.01	0.01
query21	15.55	0.53	0.54
query22	2.38	1.96	1.96
query23	17.20	1.02	1.20
query24	6.10	1.06	1.14
query25	0.33	0.07	0.07
query26	0.69	0.17	0.16
query27	0.04	0.04	0.04
query28	6.87	0.79	0.69
query29	12.62	2.38	2.36
query30	0.61	0.48	0.52
query31	2.81	0.40	0.38
query32	3.36	0.49	0.49
query33	3.03	3.08	3.09
query34	15.26	4.78	4.82
query35	4.88	4.85	4.82
query36	1.04	1.02	1.01
query37	0.06	0.04	0.04
query38	0.04	0.02	0.02
query39	0.02	0.02	0.01
query40	0.16	0.14	0.15
query41	0.07	0.01	0.02
query42	0.02	0.01	0.02
query43	0.03	0.02	0.01
Total cold run time: 104.16 s
Total hot run time: 31.21 s

@doris-robot
Copy link

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

Load test result on commit d75bdffb0c10e87534f387fb8821078b44513fbf with default session variables
Stream load json:         19 seconds loaded 2358488459 Bytes, about 118 MB/s
Stream load orc:          58 seconds loaded 1101869774 Bytes, about 18 MB/s
Stream load parquet:      32 seconds loaded 861443392 Bytes, about 25 MB/s
Insert into select:       22.5 seconds inserted 10000000 Rows, about 444K ops/s

@dataroaring dataroaring merged commit ea7e236 into apache:branch-2.0 Sep 7, 2024
20 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants