Skip to content

[Fix](partial update) abort partial update on shadow index's tablet when the including columns miss key columns on new schema - #46347

Merged
dataroaring merged 6 commits into
apache:masterfrom
bobhan1:bh-add-key-sc-partial-update-dup-key
Jan 7, 2025
Merged

[Fix](partial update) abort partial update on shadow index's tablet when the including columns miss key columns on new schema#46347
dataroaring merged 6 commits into
apache:masterfrom
bobhan1:bh-add-key-sc-partial-update-dup-key

Conversation

@bobhan1

@bobhan1bobhan1 commented Jan 3, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Problem Summary:

During a schema change which changes key columns, partial update on shadow index's tablet may cause duplicate key problem. So we abort the partial update in this situation.

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

@hello-stephen

hello-stephen commented Jan 3, 2025

Copy link
Copy Markdown
Contributor

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
bobhan1force-pushed the bh-add-key-sc-partial-update-dup-key branch from cc8956b to 1a307efCompareJanuary 3, 2025 03:59
@bobhan1

bobhan1 commented Jan 3, 2025

Copy link
Copy Markdown
ContributorAuthor

run buildall

@doris-robot

doris-robot commented Jan 3, 2025

Copy link
Copy Markdown
TPC-H: Total hot run time: 32994 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 1a307ef27f3965acea78bd739f0d68d78af82e7c, data reload: false
------ Round 1 ----------------------------------
q1	17598	6166	6079	6079
q2	2044	305	173	173
q3	10408	1303	742	742
q4	10236	885	453	453
q5	7654	2229	2054	2054
q6	207	177	146	146
q7	915	746	604	604
q8	9223	1411	1220	1220
q9	5324	4941	4987	4941
q10	6754	2330	1874	1874
q11	475	284	272	272
q12	345	380	232	232
q13	17763	3783	3062	3062
q14	231	230	212	212
q15	562	492	490	490
q16	619	618	584	584
q17	590	865	338	338
q18	7168	6498	6426	6426
q19	1882	968	554	554
q20	314	322	184	184
q21	3067	2258	2036	2036
q22	353	330	318	318
Total cold run time: 103732 ms
Total hot run time: 32994 ms
----- Round 2, with runtime_filter_mode=off -----
q1	6273	6235	6248	6235
q2	240	326	226	226
q3	2266	2620	2341	2341
q4	1412	1855	1356	1356
q5	4375	4784	4927	4784
q6	182	179	145	145
q7	2144	1943	1765	1765
q8	2611	2816	2718	2718
q9	7320	7259	7205	7205
q10	3078	3345	2847	2847
q11	586	506	488	488
q12	642	731	614	614
q13	3538	3918	3265	3265
q14	299	312	293	293
q15	559	502	498	498
q16	663	676	641	641
q17	1226	1757	1248	1248
q18	7821	7490	7216	7216
q19	807	1144	1033	1033
q20	1893	1978	1820	1820
q21	5489	5004	4839	4839
q22	622	618	551	551
Total cold run time: 54046 ms
Total hot run time: 52128 ms

@doris-robot

doris-robot commented Jan 3, 2025

Copy link
Copy Markdown
TPC-DS: Total hot run time: 190908 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 1a307ef27f3965acea78bd739f0d68d78af82e7c, data reload: false
query1	977	391	363	363
query2	6529	2390	2336	2336
query3	6716	215	211	211
query4	33851	23461	23461	23461
query5	4300	635	469	469
query6	291	202	211	202
query7	4623	502	308	308
query8	305	255	223	223
query9	9572	2652	2620	2620
query10	471	306	246	246
query11	18129	15660	15256	15256
query12	161	103	105	103
query13	1653	502	402	402
query14	12113	7647	8218	7647
query15	246	187	186	186
query16	8309	581	437	437
query17	1601	735	553	553
query18	1431	381	304	304
query19	214	172	151	151
query20	133	109	139	109
query21	211	118	109	109
query22	4484	4538	4225	4225
query23	34549	33367	33364	33364
query24	6162	2209	2289	2209
query25	498	442	371	371
query26	1181	269	150	150
query27	2028	467	330	330
query28	5294	2410	2424	2410
query29	701	523	421	421
query30	234	177	150	150
query31	1001	916	804	804
query32	96	60	58	58
query33	512	367	322	322
query34	762	834	506	506
query35	810	812	731	731
query36	1020	1010	950	950
query37	121	107	76	76
query38	4079	4195	4158	4158
query39	1504	1453	1413	1413
query40	206	120	108	108
query41	49	50	47	47
query42	122	106	113	106
query43	524	529	498	498
query44	1283	797	803	797
query45	183	168	170	168
query46	877	1040	649	649
query47	1924	1960	1856	1856
query48	386	422	337	337
query49	819	466	375	375
query50	611	648	404	404
query51	7214	7201	7085	7085
query52	95	97	89	89
query53	225	247	188	188
query54	470	479	412	412
query55	79	82	79	79
query56	258	267	244	244
query57	1235	1213	1139	1139
query58	230	231	227	227
query59	3063	3227	3115	3115
query60	272	262	249	249
query61	112	104	109	104
query62	914	818	752	752
query63	222	196	194	194
query64	4164	983	633	633
query65	3269	3167	3225	3167
query66	987	418	307	307
query67	16159	15797	15725	15725
query68	8996	707	514	514
query69	462	288	253	253
query70	1256	1158	1111	1111
query71	445	278	266	266
query72	6228	3805	3881	3805
query73	667	752	366	366
query74	10223	9040	8800	8800
query75	4326	3170	2654	2654
query76	4459	1199	768	768
query77	822	380	282	282
query78	10073	10113	9603	9603
query79	3059	789	615	615
query80	664	511	416	416
query81	471	278	227	227
query82	606	152	125	125
query83	204	168	143	143
query84	282	90	75	75
query85	796	425	300	300
query86	339	323	301	301
query87	4417	4469	4297	4297
query88	4170	2163	2139	2139
query89	410	335	306	306
query90	1885	190	193	190
query91	134	143	104	104
query92	65	54	49	49
query93	1010	834	538	538
query94	659	404	287	287
query95	338	334	250	250
query96	485	602	279	279
query97	2919	2983	2827	2827
query98	234	199	198	198
query99	1681	1543	1422	1422
Total cold run time: 295609 ms
Total hot run time: 190908 ms

@doris-robot

doris-robot commented Jan 3, 2025

Copy link
Copy Markdown
ClickBench: Total hot run time: 31.34 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 1a307ef27f3965acea78bd739f0d68d78af82e7c, data reload: false
query1	0.03	0.03	0.03
query2	0.06	0.03	0.04
query3	0.24	0.07	0.07
query4	1.61	0.11	0.10
query5	0.40	0.41	0.41
query6	1.15	0.65	0.65
query7	0.02	0.02	0.02
query8	0.04	0.04	0.03
query9	0.58	0.52	0.52
query10	0.56	0.57	0.57
query11	0.15	0.10	0.10
query12	0.14	0.11	0.11
query13	0.61	0.62	0.60
query14	2.82	2.74	2.73
query15	0.91	0.82	0.82
query16	0.37	0.38	0.39
query17	0.97	1.00	1.00
query18	0.23	0.21	0.21
query19	1.84	1.95	1.77
query20	0.01	0.01	0.02
query21	15.37	0.93	0.60
query22	0.77	0.89	0.75
query23	15.08	1.49	0.51
query24	3.35	0.87	1.64
query25	0.23	0.18	0.15
query26	0.23	0.15	0.14
query27	0.06	0.04	0.05
query28	13.72	1.53	1.04
query29	12.85	3.99	3.30
query30	0.25	0.09	0.06
query31	2.82	0.63	0.38
query32	3.23	0.54	0.46
query33	3.03	3.15	3.07
query34	16.70	5.07	4.54
query35	4.51	4.47	4.50
query36	0.62	0.49	0.51
query37	0.09	0.06	0.06
query38	0.04	0.03	0.03
query39	0.03	0.02	0.03
query40	0.17	0.14	0.13
query41	0.08	0.02	0.02
query42	0.04	0.03	0.02
query43	0.04	0.04	0.03
Total cold run time: 106.05 s
Total hot run time: 31.34 s

@doris-robot

doris-robot commented Jan 3, 2025

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 38.89% (10129/26043)
Line Coverage: 29.91% (85670/286428)
Region Coverage: 29.03% (43740/150691)
Branch Coverage: 25.56% (22321/87320)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1a307ef27f3965acea78bd739f0d68d78af82e7c_1a307ef27f3965acea78bd739f0d68d78af82e7c/report/index.html

zhannngchen
zhannngchen reviewed Jan 3, 2025
View reviewed changes
Comment threadbe/src/olap/partial_update_info.cpp
"txn_id={}. Missing key column {}.",
tablet_id, txn_id, key_col.name());
LOG_WARNING(msg);
return Status::Aborted<false>(msg);

@zhannngchenzhannngchenJan 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is INVALID_SCHEMA error better?

zhannngchen
zhannngchen approved these changes Jan 3, 2025
View reviewed changes

@zhannngchenzhannngchen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Jan 3, 2025
@github-actions

github-actionsBot commented Jan 3, 2025

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actionsBot commented Jan 3, 2025

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

dataroaring
dataroaring approved these changes Jan 7, 2025
View reviewed changes

@dataroaringdataroaring left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dataroaring
dataroaring merged commit b088cf5 into apache:masterJan 7, 2025
bobhan1 added a commit to bobhan1/doris that referenced this pull request Jan 8, 2025
@bobhan1
[Fix](partial update) abort partial update on shadow index's tablet w…
0a54978
…hen the including columns miss key columns on new schema (apache#46347)
During a schema change which changes key columns, partial update on
shadow index's tablet may cause duplicate key problem. So we abort the
partial update in this situation.
@bobhan1bobhan1 mentioned this pull request Jan 8, 2025
Merged
bobhan1 added a commit to bobhan1/doris that referenced this pull request Jan 8, 2025
@bobhan1
[Fix](partial update) abort partial update on shadow index's tablet w…
21d8925
…hen the including columns miss key columns on new schema (apache#46347)
During a schema change which changes key columns, partial update on
shadow index's tablet may cause duplicate key problem. So we abort the
partial update in this situation.
@bobhan1bobhan1 mentioned this pull request Jan 8, 2025
Closed
dataroaring pushed a commit that referenced this pull request Jan 8, 2025
@bobhan1
branch-3.0: [Fix](partial update) abort partial update on shadow inde…
d0568b8
…x's tablet when the including columns miss key columns on new schema #46347 (#46587)
pick #46347
@gavinchougavinchou mentioned this pull request Feb 18, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhannngchenzhannngchenzhannngchen approved these changes
@dataroaringdataroaringdataroaring approved these changes

Assignees

No one assigned

Labels

approvedIndicates a PR has been approved by one committer.dev/3.0.4-mergedkind/behavior-changedp0_wreviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bobhan1@hello-stephen@doris-robot@zhannngchen@dataroaring