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](delete) Incorrect status handling in CloudDeleteTask::execute() #39428

Merged
merged 1 commit into from
Aug 15, 2024
Merged

[fix](delete) Incorrect status handling in CloudDeleteTask::execute() #39428

merged 1 commit into from
Aug 15, 2024

Conversation

gavinchou
Copy link
Contributor

Previous impl. always returns OK to user when commit_rowset for the delete stmt. It may lead to missing delete predicate.

@gavinchou
[fix](delete) Incorrect status handling in CloudDeleteTask::execute()
6a3db77 
Previous impl. always returns OK to user when commit_rowset for the delete stmt.
It may lead to missing delete predicate.
@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.

@gavinchou
Copy link
Contributor 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: 38551 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 6a3db77fd1f23163ff390df37d66c2a6bea630a6, data reload: false

------ Round 1 ----------------------------------
q1	17899	4417	4317	4317
q2	2055	240	213	213
q3	10391	1239	1116	1116
q4	10171	842	773	773
q5	8526	2861	2791	2791
q6	263	156	158	156
q7	1017	645	652	645
q8	9398	2078	2077	2077
q9	7260	6556	6617	6556
q10	7087	2208	2271	2208
q11	497	275	276	275
q12	434	267	265	265
q13	18759	3009	2983	2983
q14	297	269	259	259
q15	565	535	543	535
q16	546	426	410	410
q17	1000	710	678	678
q18	7517	6987	6955	6955
q19	6094	1094	1023	1023
q20	705	360	373	360
q21	3914	2910	2949	2910
q22	1165	1081	1046	1046
Total cold run time: 115560 ms
Total hot run time: 38551 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4573	4322	4350	4322
q2	416	312	297	297
q3	2883	2641	2635	2635
q4	1970	1684	1671	1671
q5	5390	5465	5374	5374
q6	237	150	147	147
q7	2072	1713	1748	1713
q8	3228	3349	3315	3315
q9	8434	8353	8410	8353
q10	3422	3188	3192	3188
q11	618	523	536	523
q12	822	635	635	635
q13	17755	2996	3013	2996
q14	326	306	287	287
q15	556	508	517	508
q16	501	442	442	442
q17	1811	1480	1484	1480
q18	7902	7737	7437	7437
q19	5706	1522	1702	1522
q20	2105	1829	1798	1798
q21	12614	5112	5151	5112
q22	1146	1077	1029	1029
Total cold run time: 84487 ms
Total hot run time: 54784 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 191050 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 6a3db77fd1f23163ff390df37d66c2a6bea630a6, data reload: false

query1	990	408	398	398
query2	6745	2010	1948	1948
query3	6668	235	245	235
query4	34363	23771	23267	23267
query5	4359	634	651	634
query6	300	187	197	187
query7	4613	324	330	324
query8	467	431	425	425
query9	8753	2547	2537	2537
query10	501	344	363	344
query11	17498	15073	15164	15073
query12	172	126	121	121
query13	1684	412	404	404
query14	10107	7238	7237	7237
query15	275	190	190	190
query16	8069	521	512	512
query17	1594	607	581	581
query18	2156	337	330	330
query19	357	167	165	165
query20	145	131	134	131
query21	258	141	140	140
query22	4587	4133	4040	4040
query23	34035	33334	33315	33315
query24	9658	2987	2900	2900
query25	613	429	427	427
query26	723	184	177	177
query27	2001	305	304	304
query28	5932	2143	2137	2137
query29	793	446	450	446
query30	337	178	183	178
query31	1052	840	785	785
query32	114	75	78	75
query33	695	347	350	347
query34	889	524	494	494
query35	879	772	771	771
query36	1110	994	961	961
query37	157	102	101	101
query38	3952	3823	3856	3823
query39	1522	1483	1454	1454
query40	235	151	153	151
query41	137	135	136	135
query42	138	117	118	117
query43	532	511	495	495
query44	1141	779	774	774
query45	223	191	194	191
query46	1129	776	800	776
query47	1915	1831	1834	1831
query48	418	349	349	349
query49	1224	601	591	591
query50	861	462	466	462
query51	6920	6920	6843	6843
query52	121	107	108	107
query53	305	229	227	227
query54	810	512	496	496
query55	92	90	90	90
query56	337	321	316	316
query57	1227	1131	1136	1131
query58	311	296	323	296
query59	3005	2882	2801	2801
query60	352	329	328	328
query61	147	145	145	145
query62	841	692	707	692
query63	263	231	235	231
query64	4859	2362	1834	1834
query65	3257	3181	3218	3181
query66	1157	684	703	684
query67	15864	15131	14913	14913
query68	6227	603	596	596
query69	796	432	360	360
query70	1250	1176	1195	1176
query71	521	327	322	322
query72	7557	2355	2077	2077
query73	873	361	361	361
query74	9470	8742	8931	8742
query75	4401	2761	2775	2761
query76	4339	994	1005	994
query77	900	496	435	435
query78	9646	9782	9101	9101
query79	9818	569	565	565
query80	1138	602	613	602
query81	621	255	258	255
query82	672	156	156	156
query83	386	214	209	209
query84	285	97	98	97
query85	976	347	349	347
query86	401	310	325	310
query87	4393	4196	4292	4196
query88	4155	2489	2481	2481
query89	540	333	322	322
query90	2056	237	235	235
query91	154	159	131	131
query92	85	73	75	73
query93	6463	549	561	549
query94	1042	327	333	327
query95	390	295	292	292
query96	626	285	286	285
query97	3219	3073	3104	3073
query98	252	238	224	224
query99	1611	1291	1322	1291
Total cold run time: 312207 ms
Total hot run time: 191050 ms

@doris-robot
Copy link

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

query1	0.05	0.04	0.04
query2	0.08	0.04	0.05
query3	0.23	0.05	0.05
query4	1.67	0.08	0.08
query5	0.51	0.49	0.47
query6	1.12	0.73	0.72
query7	0.04	0.02	0.02
query8	0.06	0.05	0.04
query9	0.55	0.50	0.50
query10	0.56	0.56	0.54
query11	0.16	0.13	0.12
query12	0.16	0.13	0.13
query13	0.61	0.61	0.60
query14	0.78	0.78	0.78
query15	0.87	0.82	0.82
query16	0.38	0.36	0.37
query17	1.01	1.04	0.98
query18	0.22	0.22	0.22
query19	1.84	1.81	1.68
query20	0.01	0.01	0.02
query21	15.41	0.84	0.66
query22	4.31	6.55	2.63
query23	18.24	1.40	1.25
query24	2.15	0.22	0.22
query25	0.16	0.09	0.09
query26	0.31	0.21	0.21
query27	0.46	0.23	0.22
query28	13.30	1.04	1.02
query29	12.64	3.43	3.40
query30	0.37	0.18	0.18
query31	2.80	0.40	0.40
query32	3.23	0.50	0.49
query33	2.97	3.01	2.97
query34	17.21	4.42	4.41
query35	4.50	4.47	4.45
query36	0.67	0.47	0.50
query37	0.20	0.18	0.17
query38	0.18	0.17	0.17
query39	0.07	0.06	0.06
query40	0.18	0.15	0.15
query41	0.11	0.07	0.06
query42	0.08	0.07	0.07
query43	0.07	0.06	0.06
Total cold run time: 110.53 s
Total hot run time: 32.01 s

dataroaring
dataroaring approved these changes Aug 15, 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

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Aug 15, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

w41ter
w41ter approved these changes Aug 15, 2024
View reviewed changes
cloud/src/meta-service/meta_service_helper.h
if constexpr (category == ErrCategory::READ) {
return MetaServiceCode::KV_TXN_GET_ERR;
} else if constexpr (category == ErrCategory::CREATE) {
return MetaServiceCode::KV_TXN_CREATE_ERR;
} else {
return MetaServiceCode::KV_TXN_COMMIT_ERR;
}
[[fallthrough]];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[[fallthrough]];

@gavinchou gavinchou merged commit b1eb70d into apache:master Aug 15, 2024
27 of 31 checks passed
dataroaring pushed a commit that referenced this pull request Aug 17, 2024
@gavinchou @dataroaring
[fix](delete) Incorrect status handling in CloudDeleteTask::execute() (…
bbf9772 
…#39428)

Previous impl. always returns OK to user when commit_rowset for the
delete stmt. It may lead to missing delete predicate.
@gavinchou gavinchou mentioned this pull request Oct 13, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@w41ter w41ter w41ter approved these changes

@dataroaring dataroaring dataroaring approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@gavinchou @doris-robot @w41ter @dataroaring