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](merge-on-write) Add defensive check before partial update #44687

Merged
merged 1 commit into from
Dec 6, 2024
Merged

[Fix](merge-on-write) Add defensive check before partial update #44687

merged 1 commit into from
Dec 6, 2024

Conversation

bobhan1
Copy link
Contributor

@bobhan1 bobhan1 commented Nov 27, 2024 •
edited by zhannngchen
Loading

What problem does this PR solve?

as stated in #44692, Some user can create Duplicate keys table with enable_unique_key_merge_on_write=true successfully in version 1.2. After they upgrade to higher version, the incorrect table property still remains. Some logic in BE treat such table as an unique key mow table, makes a wrong query plan and BE may do partial update on duplicate table, which causes BE core.
This PR add checks before doing partial update and return error when some preconditions are not met to prevent coredump.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

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

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@bobhan1 bobhan1 force-pushed the check-table-when-do-partial-update branch from b0bba4f to 1951dfc Compare November 27, 2024 11:55
@bobhan1
Copy link
Contributor Author

bobhan1 commented Nov 27, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.35% (9977/26018)
Line Coverage: 29.44% (83508/283692)
Region Coverage: 28.59% (42999/150396)
Branch Coverage: 25.19% (21836/86690)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1951dfc960f252e8bfac0d852e44044cc5dbfd4a_1951dfc960f252e8bfac0d852e44044cc5dbfd4a/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17581	7377	7268	7268
q2	2042	183	162	162
q3	10878	1054	1139	1054
q4	10537	739	703	703
q5	7630	2749	2732	2732
q6	242	150	150	150
q7	972	651	619	619
q8	9239	1864	1897	1864
q9	6579	6333	6334	6333
q10	7015	2313	2322	2313
q11	483	260	261	260
q12	420	222	218	218
q13	17774	3031	3036	3031
q14	243	209	207	207
q15	578	530	508	508
q16	667	572	578	572
q17	979	605	578	578
q18	7261	6702	6616	6616
q19	1358	999	987	987
q20	488	183	189	183
q21	4006	3230	3082	3082
q22	380	319	328	319
Total cold run time: 107352 ms
Total hot run time: 39759 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7254	7219	7182	7182
q2	324	232	228	228
q3	2897	2790	2933	2790
q4	2120	1826	1850	1826
q5	5667	5672	5597	5597
q6	226	147	147	147
q7	2293	1801	1833	1801
q8	3381	3511	3483	3483
q9	8759	8900	8810	8810
q10	3626	3537	3558	3537
q11	593	511	510	510
q12	803	624	625	624
q13	12594	3212	3213	3212
q14	302	266	268	266
q15	569	507	524	507
q16	685	644	629	629
q17	1862	1631	1650	1631
q18	8366	7818	7573	7573
q19	1721	1583	1549	1549
q20	2121	1852	1872	1852
q21	5585	5513	5320	5320
q22	652	573	578	573
Total cold run time: 72400 ms
Total hot run time: 59647 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 197876 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 1951dfc960f252e8bfac0d852e44044cc5dbfd4a, data reload: false

query1	1234	957	982	957
query2	6250	2206	2098	2098
query3	10891	3842	4016	3842
query4	67829	32072	23883	23883
query5	4945	484	479	479
query6	427	187	182	182
query7	5571	297	304	297
query8	327	229	255	229
query9	8679	2754	2723	2723
query10	449	269	248	248
query11	17302	15496	15815	15496
query12	156	101	114	101
query13	1544	403	413	403
query14	10512	7289	7358	7289
query15	218	192	182	182
query16	7460	504	443	443
query17	1057	577	575	575
query18	2024	328	299	299
query19	204	158	150	150
query20	125	134	110	110
query21	204	103	100	100
query22	4946	4516	4599	4516
query23	35130	34629	34624	34624
query24	5540	2497	2481	2481
query25	510	395	382	382
query26	660	166	153	153
query27	2028	281	298	281
query28	4205	2476	2497	2476
query29	704	410	428	410
query30	219	147	151	147
query31	1016	830	842	830
query32	66	53	57	53
query33	443	285	280	280
query34	936	532	527	527
query35	903	768	739	739
query36	1099	965	956	956
query37	129	78	77	77
query38	4475	4491	4442	4442
query39	1505	1448	1459	1448
query40	202	110	100	100
query41	47	42	44	42
query42	116	104	101	101
query43	554	513	511	511
query44	1187	853	830	830
query45	187	166	167	166
query46	1156	721	707	707
query47	2054	1968	1947	1947
query48	413	339	323	323
query49	718	387	406	387
query50	829	400	399	399
query51	7361	7162	7084	7084
query52	97	86	88	86
query53	254	184	186	184
query54	510	395	420	395
query55	79	76	81	76
query56	268	246	259	246
query57	1328	1213	1155	1155
query58	222	241	233	233
query59	3196	3045	3025	3025
query60	276	251	268	251
query61	133	128	129	128
query62	782	666	690	666
query63	226	197	189	189
query64	1511	670	647	647
query65	3286	3251	3205	3205
query66	619	298	295	295
query67	16224	15891	15726	15726
query68	4117	551	562	551
query69	419	253	254	253
query70	1187	1121	1143	1121
query71	333	258	250	250
query72	6483	3915	3967	3915
query73	774	372	365	365
query74	10114	9053	9028	9028
query75	3407	2651	2759	2651
query76	1900	1061	1108	1061
query77	478	266	291	266
query78	10565	9339	9363	9339
query79	2178	607	593	593
query80	1441	429	436	429
query81	519	233	234	233
query82	1314	121	129	121
query83	262	153	153	153
query84	279	71	72	71
query85	1056	298	304	298
query86	422	286	292	286
query87	4693	4513	4622	4513
query88	3594	2244	2214	2214
query89	428	299	290	290
query90	1856	183	199	183
query91	139	104	104	104
query92	64	50	49	49
query93	2861	532	538	532
query94	768	297	295	295
query95	351	246	247	246
query96	629	272	283	272
query97	2872	2698	2676	2676
query98	221	195	195	195
query99	1883	1311	1336	1311
Total cold run time: 323364 ms
Total hot run time: 197876 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.03
query2	0.07	0.03	0.03
query3	0.23	0.08	0.08
query4	1.61	0.10	0.11
query5	0.44	0.41	0.42
query6	1.15	0.66	0.65
query7	0.02	0.02	0.02
query8	0.04	0.03	0.04
query9	0.55	0.52	0.50
query10	0.55	0.55	0.55
query11	0.13	0.10	0.10
query12	0.14	0.12	0.12
query13	0.62	0.61	0.61
query14	2.73	2.73	2.69
query15	0.88	0.85	0.82
query16	0.37	0.40	0.36
query17	1.01	1.00	1.02
query18	0.24	0.22	0.21
query19	1.98	1.82	1.87
query20	0.02	0.01	0.01
query21	15.36	0.58	0.59
query22	2.22	2.24	1.73
query23	17.05	0.97	0.86
query24	2.96	1.91	0.46
query25	0.15	0.16	0.04
query26	0.61	0.14	0.14
query27	0.05	0.04	0.04
query28	10.62	1.10	1.07
query29	12.54	3.19	3.24
query30	0.25	0.06	0.06
query31	2.87	0.38	0.38
query32	3.26	0.46	0.47
query33	2.99	3.04	3.05
query34	16.82	4.44	4.44
query35	4.48	4.45	4.55
query36	0.68	0.49	0.49
query37	0.09	0.07	0.05
query38	0.04	0.04	0.04
query39	0.04	0.03	0.02
query40	0.16	0.13	0.13
query41	0.07	0.02	0.02
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 106.2 s
Total hot run time: 31.88 s

@bobhan1
Copy link
Contributor Author

bobhan1 commented Nov 28, 2024

run cloud_p0

@bobhan1 bobhan1 changed the title [Fix](merge-on-write) Add defensive check when do partial update [Fix](merge-on-write) Add defensive check before partial update Nov 28, 2024
zhannngchen
zhannngchen previously approved these changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@zhannngchen zhannngchen 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 Nov 28, 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.

@bobhan1 bobhan1 force-pushed the check-table-when-do-partial-update branch from 1951dfc to 8f3fbe9 Compare November 29, 2024 12:41
@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 29, 2024
@bobhan1
Copy link
Contributor Author

bobhan1 commented Nov 29, 2024

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17582	7560	7341	7341
q2	2045	187	181	181
q3	10679	1136	1253	1136
q4	10545	762	783	762
q5	7617	2731	2767	2731
q6	240	148	147	147
q7	996	643	617	617
q8	9271	1896	1944	1896
q9	6745	6570	6524	6524
q10	6982	2304	2320	2304
q11	451	262	270	262
q12	437	244	219	219
q13	17776	3035	3037	3035
q14	244	204	208	204
q15	565	531	546	531
q16	649	561	592	561
q17	995	571	570	570
q18	7415	6810	6738	6738
q19	1338	1049	991	991
q20	478	172	182	172
q21	4027	3172	3285	3172
q22	381	324	320	320
Total cold run time: 107458 ms
Total hot run time: 40414 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7286	7257	7235	7235
q2	324	223	222	222
q3	2918	2943	2958	2943
q4	2128	1861	1879	1861
q5	5647	5740	5699	5699
q6	231	137	136	136
q7	2293	1833	1876	1833
q8	3386	3561	3594	3561
q9	8934	9091	9052	9052
q10	3611	3564	3572	3564
q11	595	522	519	519
q12	808	607	619	607
q13	12096	3265	3233	3233
q14	310	291	286	286
q15	584	532	519	519
q16	679	642	654	642
q17	1881	1641	1612	1612
q18	8377	7786	7566	7566
q19	1778	1641	1677	1641
q20	2143	1857	1886	1857
q21	5621	5496	5561	5496
q22	641	586	565	565
Total cold run time: 72271 ms
Total hot run time: 60649 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.35% (9978/26016)
Line Coverage: 29.43% (83596/284052)
Region Coverage: 28.54% (42994/150653)
Branch Coverage: 25.16% (21844/86808)
Coverage Report: http://coverage.selectdb-in.cc/coverage/8f3fbe9c501bbe57e292bbfc5f241faa9db6f40b_8f3fbe9c501bbe57e292bbfc5f241faa9db6f40b/report/index.html

@doris-robot
Copy link

TPC-DS: Total hot run time: 197196 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 8f3fbe9c501bbe57e292bbfc5f241faa9db6f40b, data reload: false

query1	1239	942	947	942
query2	6239	2114	2013	2013
query3	10973	4482	4373	4373
query4	66836	28797	23549	23549
query5	4931	467	469	467
query6	414	188	190	188
query7	5477	305	290	290
query8	321	246	230	230
query9	8658	2760	2737	2737
query10	456	248	251	248
query11	17336	15217	15973	15217
query12	159	108	103	103
query13	1447	446	450	446
query14	10388	7286	7311	7286
query15	217	188	198	188
query16	7098	479	453	453
query17	1139	597	591	591
query18	1809	301	294	294
query19	208	155	157	155
query20	120	116	116	116
query21	211	106	103	103
query22	4738	4576	4488	4488
query23	35056	34222	34805	34222
query24	5446	2502	2546	2502
query25	498	376	390	376
query26	691	153	159	153
query27	1975	282	279	279
query28	4387	2522	2497	2497
query29	672	415	413	413
query30	205	152	150	150
query31	997	828	848	828
query32	68	57	58	57
query33	414	308	295	295
query34	922	542	497	497
query35	882	757	777	757
query36	1101	960	977	960
query37	124	80	81	80
query38	4505	4540	4365	4365
query39	1519	1484	1464	1464
query40	208	99	101	99
query41	47	42	42	42
query42	108	95	99	95
query43	534	501	493	493
query44	1195	833	812	812
query45	187	171	174	171
query46	1164	717	724	717
query47	2009	1932	1927	1927
query48	427	325	343	325
query49	729	402	385	385
query50	840	385	390	385
query51	7467	7193	7027	7027
query52	96	89	89	89
query53	250	181	180	180
query54	508	399	392	392
query55	75	76	73	73
query56	239	231	224	224
query57	1254	1113	1113	1113
query58	255	206	215	206
query59	3259	2947	3059	2947
query60	265	246	248	246
query61	107	104	103	103
query62	864	647	658	647
query63	213	176	182	176
query64	1345	705	627	627
query65	3272	3259	3200	3200
query66	692	296	299	296
query67	15871	15543	15609	15543
query68	4228	554	537	537
query69	417	245	260	245
query70	1202	1148	1140	1140
query71	328	248	245	245
query72	6449	4172	4122	4122
query73	754	372	365	365
query74	10250	9271	9172	9172
query75	3342	2635	2625	2625
query76	2064	1076	1104	1076
query77	502	266	353	266
query78	10583	9463	9412	9412
query79	2573	593	605	593
query80	1386	426	433	426
query81	514	232	232	232
query82	1271	120	113	113
query83	243	138	142	138
query84	286	86	77	77
query85	1010	374	294	294
query86	446	309	279	279
query87	4797	4619	4734	4619
query88	3950	2214	2184	2184
query89	423	305	290	290
query90	1997	188	188	188
query91	143	101	102	101
query92	73	53	49	49
query93	3007	541	533	533
query94	911	289	288	288
query95	346	252	244	244
query96	634	286	283	283
query97	2854	2664	2641	2641
query98	213	198	197	197
query99	1623	1310	1328	1310
Total cold run time: 322175 ms
Total hot run time: 197196 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.07	0.03	0.03
query3	0.24	0.07	0.07
query4	1.62	0.10	0.11
query5	0.42	0.40	0.44
query6	1.15	0.66	0.65
query7	0.02	0.02	0.01
query8	0.05	0.04	0.03
query9	0.60	0.51	0.51
query10	0.55	0.56	0.57
query11	0.14	0.10	0.10
query12	0.13	0.12	0.12
query13	0.61	0.60	0.61
query14	2.70	2.71	2.68
query15	0.92	0.82	0.82
query16	0.40	0.38	0.39
query17	1.09	1.04	1.04
query18	0.22	0.20	0.21
query19	1.98	1.90	2.08
query20	0.02	0.01	0.01
query21	15.37	0.59	0.56
query22	2.27	1.89	2.14
query23	16.98	0.95	0.80
query24	3.05	1.80	0.50
query25	0.18	0.07	0.05
query26	0.56	0.14	0.15
query27	0.04	0.06	0.04
query28	10.68	1.09	1.08
query29	12.55	3.29	3.25
query30	0.25	0.07	0.06
query31	2.84	0.40	0.37
query32	3.27	0.48	0.48
query33	3.02	3.05	3.07
query34	16.92	4.44	4.45
query35	4.50	4.58	4.43
query36	0.66	0.48	0.49
query37	0.09	0.06	0.06
query38	0.04	0.04	0.04
query39	0.04	0.03	0.02
query40	0.16	0.13	0.12
query41	0.08	0.02	0.03
query42	0.04	0.03	0.02
query43	0.04	0.03	0.03
Total cold run time: 106.59 s
Total hot run time: 32.17 s

@zhannngchen
Copy link
Contributor

run buildall

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 3, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 3, 2024

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

@bobhan1
Copy link
Contributor Author

bobhan1 commented Dec 3, 2024

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.51% (10009/25990)
Line Coverage: 29.50% (83806/284097)
Region Coverage: 28.63% (43113/150595)
Branch Coverage: 25.23% (21912/86834)
Coverage Report: http://coverage.selectdb-in.cc/coverage/8f3fbe9c501bbe57e292bbfc5f241faa9db6f40b_8f3fbe9c501bbe57e292bbfc5f241faa9db6f40b/report/index.html

dataroaring
dataroaring approved these changes Dec 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

@zhannngchen zhannngchen merged commit 08d1e76 into apache:master Dec 6, 2024
23 of 26 checks passed
This was referenced Dec 6, 2024
Merged
Merged
yiguolei pushed a commit that referenced this pull request Dec 6, 2024
@bobhan1
branch-2.1: [Fix](merge-on-write) Add defensive check before partial …
ebb21ef 
…update #44687 (#45086)

pick #44687
dataroaring pushed a commit that referenced this pull request Dec 6, 2024
@bobhan1
branch-3.0: [Fix](merge-on-write) Add defensive check before partial …
f4b9840 
…update #44687 (#45082)

pick #44687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dataroaring dataroaring dataroaring approved these changes

@zhannngchen zhannngchen zhannngchen approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@bobhan1 @doris-robot @zhannngchen @dataroaring @yiguolei