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

[Fix](Variant) fix potential heap use after free when concurrently flush segments on one tablet #37976

Merged
merged 1 commit into from
Jul 17, 2024
Merged

[Fix](Variant) fix potential heap use after free when concurrently flush segments on one tablet #37976

merged 1 commit into from
Jul 17, 2024

Conversation

eldenmoon
Copy link
Member

@eldenmoon eldenmoon commented Jul 17, 2024 •
edited
Loading

TabletSchema is shallow copied, should do deep copy for _flush_schema

Proposed changes

Issue Number: close #xxx

@eldenmoon
[Fix](Variant) fix potential heap use after free when concurrently fl…
dbb1449 
…ush segments on one tablet

TabletSchema is shallow copied, should do deep copy for _flush_schema
@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.

@eldenmoon eldenmoon changed the title [Fix](Variant) fix potential heap use after free when concurrently fl… [Fix](Variant) fix potential heap use after free when concurrently flush segments on one tablet Jul 17, 2024
@eldenmoon
Copy link
Member 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: 39693 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit dbb14491234d3b21dff97cb52ce15ff1838ffc42, data reload: false

------ Round 1 ----------------------------------
q1	17619	4305	4297	4297
q2	2011	185	182	182
q3	10461	1173	1098	1098
q4	10191	761	809	761
q5	7639	2694	2777	2694
q6	220	134	133	133
q7	945	601	595	595
q8	9227	2026	2058	2026
q9	8890	6521	6523	6521
q10	8884	3766	3790	3766
q11	436	231	233	231
q12	417	226	224	224
q13	17759	2944	2955	2944
q14	282	242	245	242
q15	520	483	488	483
q16	499	376	383	376
q17	943	671	609	609
q18	8075	7458	7459	7458
q19	4687	1412	1354	1354
q20	682	320	328	320
q21	4838	3111	3143	3111
q22	330	268	282	268
Total cold run time: 115555 ms
Total hot run time: 39693 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4393	4235	4204	4204
q2	373	275	260	260
q3	2986	2741	2884	2741
q4	1970	1705	1750	1705
q5	5659	5556	5449	5449
q6	230	130	131	130
q7	2188	1844	1856	1844
q8	3259	3380	3379	3379
q9	8713	8786	8874	8786
q10	4078	3929	3699	3699
q11	579	494	511	494
q12	827	652	609	609
q13	17169	3202	3188	3188
q14	307	289	285	285
q15	527	481	496	481
q16	500	437	435	435
q17	1806	1554	1504	1504
q18	8134	7965	7812	7812
q19	1757	1508	1418	1418
q20	2130	1876	1832	1832
q21	4946	4987	4657	4657
q22	588	504	497	497
Total cold run time: 73119 ms
Total hot run time: 55409 ms

@doris-robot
Copy link

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

query1	891	366	360	360
query2	6434	1902	1740	1740
query3	6637	209	220	209
query4	25684	17461	17442	17442
query5	3660	476	479	476
query6	262	178	161	161
query7	4588	291	287	287
query8	245	194	186	186
query9	8636	2401	2411	2401
query10	433	282	270	270
query11	10904	10011	10082	10011
query12	117	86	79	79
query13	1647	359	365	359
query14	10257	7807	7546	7546
query15	218	167	166	166
query16	7685	317	305	305
query17	1826	558	518	518
query18	1909	278	270	270
query19	213	143	160	143
query20	91	83	81	81
query21	214	125	122	122
query22	4287	4025	4002	4002
query23	34438	33753	33634	33634
query24	10636	2913	2906	2906
query25	578	383	376	376
query26	701	153	149	149
query27	2248	279	275	275
query28	6118	2069	2070	2069
query29	891	641	611	611
query30	261	161	160	160
query31	977	769	777	769
query32	96	54	54	54
query33	651	301	298	298
query34	912	486	498	486
query35	693	581	579	579
query36	1124	983	987	983
query37	142	85	82	82
query38	2968	2916	2815	2815
query39	918	869	821	821
query40	201	114	117	114
query41	45	43	44	43
query42	118	98	100	98
query43	498	467	469	467
query44	1091	761	722	722
query45	193	162	161	161
query46	1182	715	750	715
query47	1857	1757	1771	1757
query48	356	284	296	284
query49	838	402	409	402
query50	770	390	383	383
query51	6960	6804	6791	6791
query52	102	93	89	89
query53	362	283	279	279
query54	860	447	436	436
query55	74	71	73	71
query56	280	261	266	261
query57	1136	1054	1037	1037
query58	242	248	273	248
query59	2784	2580	2548	2548
query60	324	276	273	273
query61	95	125	98	98
query62	770	671	616	616
query63	322	288	286	286
query64	9109	2184	7419	2184
query65	3127	3057	3097	3057
query66	744	318	325	318
query67	15461	14952	14773	14773
query68	4480	541	540	540
query69	487	343	317	317
query70	1210	1076	1078	1076
query71	373	281	272	272
query72	7124	5662	5139	5139
query73	738	325	320	320
query74	6029	5759	5714	5714
query75	3391	2709	2688	2688
query76	2136	928	953	928
query77	417	297	301	297
query78	9882	9046	9030	9030
query79	2706	529	523	523
query80	2348	473	478	473
query81	617	224	217	217
query82	886	138	130	130
query83	287	173	172	172
query84	275	89	87	87
query85	1936	333	306	306
query86	490	306	311	306
query87	3338	3129	3150	3129
query88	4432	2363	2391	2363
query89	491	386	388	386
query90	1798	202	196	196
query91	206	99	100	99
query92	59	50	50	50
query93	4021	518	505	505
query94	1172	212	221	212
query95	407	323	323	323
query96	609	282	269	269
query97	3186	3013	2991	2991
query98	213	203	201	201
query99	1565	1282	1234	1234
Total cold run time: 278400 ms
Total hot run time: 173205 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.03
query2	0.08	0.04	0.03
query3	0.22	0.05	0.05
query4	1.68	0.08	0.09
query5	0.49	0.51	0.48
query6	1.15	0.73	0.73
query7	0.02	0.01	0.01
query8	0.05	0.04	0.05
query9	0.55	0.49	0.50
query10	0.55	0.56	0.55
query11	0.16	0.11	0.11
query12	0.15	0.12	0.12
query13	0.60	0.58	0.58
query14	0.75	0.78	0.78
query15	0.85	0.82	0.80
query16	0.36	0.35	0.36
query17	0.94	0.96	0.96
query18	0.23	0.21	0.22
query19	1.86	1.67	1.74
query20	0.01	0.01	0.01
query21	15.39	0.73	0.65
query22	3.96	6.74	2.20
query23	18.22	1.44	1.34
query24	2.12	0.23	0.23
query25	0.16	0.08	0.08
query26	0.30	0.21	0.21
query27	0.45	0.23	0.23
query28	13.34	1.00	1.00
query29	12.64	3.37	3.36
query30	0.27	0.07	0.06
query31	2.86	0.41	0.39
query32	3.26	0.48	0.47
query33	2.87	2.88	2.90
query34	17.16	4.35	4.31
query35	4.47	4.43	4.42
query36	0.66	0.49	0.47
query37	0.19	0.16	0.16
query38	0.16	0.14	0.14
query39	0.04	0.04	0.04
query40	0.14	0.11	0.13
query41	0.10	0.04	0.05
query42	0.06	0.04	0.05
query43	0.05	0.04	0.04
Total cold run time: 109.61 s
Total hot run time: 30.95 s

airborne12
airborne12 approved these changes Jul 17, 2024
View reviewed changes
Copy link
Member

@airborne12 airborne12 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 Jul 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

xiaokang
xiaokang approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@xiaokang xiaokang left a comment

Choose a reason for hiding this comment

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

LGTM

@eldenmoon eldenmoon merged commit 7675a14 into apache:master Jul 17, 2024
29 of 32 checks passed
eldenmoon added a commit to eldenmoon/incubator-doris that referenced this pull request Jul 18, 2024
@eldenmoon
[Fix](Variant) fix potential heap use after free when concurrently fl…
dd71a3b 
…ush segments on one tablet (apache#37976)

TabletSchema is shallow copied, should do deep copy for _flush_schema
@eldenmoon eldenmoon mentioned this pull request Jul 18, 2024
Merged
eldenmoon added a commit to eldenmoon/incubator-doris that referenced this pull request Jul 18, 2024
@eldenmoon
[Fix](Variant) fix potential heap use after free when concurrently fl…
28b54f4 
…ush segments on one tablet (apache#37976)

TabletSchema is shallow copied, should do deep copy for _flush_schema
@eldenmoon eldenmoon mentioned this pull request Jul 18, 2024
Merged
eldenmoon added a commit that referenced this pull request Jul 18, 2024
@eldenmoon
[Pick](Variant) fix heap use after free and optimize cases (#38039)
a9c27c7 
#37991
#37976
xiaokang pushed a commit that referenced this pull request Jul 18, 2024
@eldenmoon
[Pick](Variant) fix heap use after free and optimize cases #37991 #37976
8c6ff22 
 (#38037)
@yiguolei yiguolei mentioned this pull request Jul 19, 2024
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiaokang xiaokang xiaokang approved these changes

@airborne12 airborne12 airborne12 approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.5-merged dev/3.0.1-merged reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eldenmoon @doris-robot @xiaokang @airborne12 @dataroaring