[refactor](prepare_txn) move prepare_txn to tablet to reduce duplicated by dataroaring · Pull Request #54124 · apache/doris

dataroaring · 2025-07-30T15:09:36Z

code

It is easy to leak the lock, so we move the code to one place.

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

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 · 2025-07-30T15:09:42Z

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:

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

dataroaring · 2025-07-30T15:09:46Z

run buildall

dataroaring · 2025-07-30T15:24:48Z

run buildall

doris-robot · 2025-07-30T15:53:05Z

TPC-H: Total hot run time: 34244 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 7384e2f0417f1be5ca4d93adf40df838087cb11b, data reload: false
------ Round 1 ----------------------------------
q1	17637	5924	5506	5506
q2	1927	287	195	195
q3	10303	1321	693	693
q4	10231	985	519	519
q5	7905	2365	2299	2299
q6	197	167	135	135
q7	885	738	608	608
q8	9311	1297	1099	1099
q9	6946	5132	5150	5132
q10	6947	2381	1959	1959
q11	474	276	272	272
q12	353	356	230	230
q13	17788	3490	2944	2944
q14	249	233	235	233
q15	527	472	480	472
q16	416	429	362	362
q17	583	846	379	379
q18	7512	7253	7087	7087
q19	2030	988	549	549
q20	565	307	219	219
q21	3385	3066	2323	2323
q22	1074	1072	1029	1029
Total cold run time: 107245 ms
Total hot run time: 34244 ms
----- Round 2, with runtime_filter_mode=off -----
q1	5920	5861	5778	5778
q2	402	312	220	220
q3	2091	2587	2249	2249
q4	1330	1715	1331	1331
q5	4429	4509	4354	4354
q6	217	183	138	138
q7	1958	1910	1953	1910
q8	2516	2454	2547	2454
q9	7491	7308	7225	7225
q10	3212	3331	3050	3050
q11	541	512	552	512
q12	698	737	601	601
q13	3425	3608	3189	3189
q14	337	338	279	279
q15	487	445	450	445
q16	465	509	469	469
q17	1298	1521	1383	1383
q18	7850	7754	7567	7567
q19	8421	854	814	814
q20	1885	1885	1796	1796
q21	12601	4264	4256	4256
q22	1027	1059	995	995
Total cold run time: 68601 ms
Total hot run time: 51015 ms

doris-robot · 2025-07-30T16:04:45Z

TPC-DS: Total hot run time: 163163 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 7384e2f0417f1be5ca4d93adf40df838087cb11b, data reload: false
reason	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 22:43:56	2023-12-26 22:44:01	NULL	utf-8	NULL	NULL ============================================
query1	1017	397	425	397
query2	6505	1718	1650	1650
query3	6744	219	217	217
query4	26788	23535	23399	23399
query5	4412	642	511	511
query6	318	244	215	215
query7	4623	509	293	293
query8	283	227	211	211
query9	8588	3159	3157	3157
query10	480	347	296	296
query11	15483	14977	14846	14846
query12	187	138	126	126
query13	1677	558	417	417
query14	10148	7792	7731	7731
query15	216	210	169	169
query16	7819	662	450	450
query17	1625	790	646	646
query18	2117	447	331	331
query19	292	208	187	187
query20	153	152	135	135
query21	224	126	114	114
query22	4190	4277	4150	4150
query23	34760	34285	34238	34238
query24	7591	2457	2386	2386
query25	567	524	435	435
query26	959	289	162	162
query27	2539	471	364	364
query28	4298	2295	2284	2284
query29	694	608	487	487
query30	299	272	198	198
query31	876	770	704	704
query32	85	85	80	80
query33	549	410	360	360
query34	810	823	519	519
query35	787	812	770	770
query36	1041	1041	932	932
query37	131	131	90	90
query38	4059	4033	3952	3952
query39	1440	1388	1375	1375
query40	234	143	131	131
query41	63	58	54	54
query42	147	121	127	121
query43	516	506	477	477
query44	1383	884	884	884
query45	194	182	186	182
query46	957	1050	675	675
query47	1815	1863	1783	1783
query48	399	469	313	313
query49	689	507	416	416
query50	651	702	418	418
query51	4185	4129	4165	4129
query52	134	128	113	113
query53	258	288	213	213
query54	650	645	543	543
query55	89	89	86	86
query56	357	348	374	348
query57	1233	1217	1184	1184
query58	335	339	330	330
query59	2650	2667	2439	2439
query60	406	399	384	384
query61	126	122	125	122
query62	836	741	654	654
query63	248	210	212	210
query64	3571	1119	815	815
query65	4187	4142	4078	4078
query66	971	464	322	322
query67	query68	18954	697	611	611
query69	979	309	284	284
query70	1409	1152	1119	1119
query71	692	333	336	333
query72	9114	2254	2356	2254
query73	3726	638	360	360
query74	9021	9068	8854	8854
query75	7514	3128	2717	2717
query76	8580	1207	792	792
query77	1162	398	334	334
query78	query79	17053	632	584	584
query80	3069	584	484	484
query81	563	356	238	238
query82	424	148	115	115
query83	450	297	278	278
query84	301	95	83	83
query85	1736	379	330	330
query86	373	345	301	301
query87	4357	4298	4127	4127
query88	5145	2215	2210	2210
query89	567	371	314	314
query90	2638	230	227	227
query91	140	134	108	108
query92	84	74	64	64
query93	6794	968	668	668
query94	1160	399	274	274
query95	410	324	316	316
query96	498	582	283	283
query97	2672	2735	2593	2593
query98	249	227	218	218
query99	1547	1354	1282	1282
Total cold run time: 304039 ms
Total hot run time: 163163 ms

doris-robot · 2025-07-30T16:10:13Z

ClickBench: Total hot run time: 33 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 7384e2f0417f1be5ca4d93adf40df838087cb11b, data reload: false
query1	0.04	0.04	0.03
query2	0.07	0.04	0.04
query3	0.25	0.08	0.07
query4	1.64	0.10	0.11
query5	0.44	0.43	0.42
query6	1.15	0.66	0.68
query7	0.02	0.02	0.02
query8	0.06	0.04	0.04
query9	0.56	0.48	0.47
query10	0.53	0.52	0.52
query11	0.16	0.10	0.10
query12	0.15	0.11	0.11
query13	0.64	0.64	0.63
query14	0.90	1.05	1.17
query15	0.91	0.88	0.89
query16	0.38	0.40	0.38
query17	1.01	1.04	1.07
query18	0.22	0.21	0.21
query19	2.02	1.91	1.76
query20	0.01	0.01	0.02
query21	15.39	0.86	0.55
query22	0.76	1.04	0.63
query23	15.13	1.08	0.62
query24	6.53	1.67	0.69
query25	0.55	0.22	0.23
query26	0.66	0.15	0.13
query27	0.05	0.05	0.05
query28	9.57	0.83	0.43
query29	12.58	3.80	3.38
query30	3.03	2.97	2.92
query31	2.87	0.55	0.39
query32	3.23	0.57	0.50
query33	3.06	3.27	3.15
query34	16.04	5.34	4.96
query35	4.86	5.03	4.99
query36	0.69	0.51	0.50
query37	0.09	0.08	0.08
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.14	0.13
query41	0.08	0.02	0.02
query42	0.03	0.02	0.03
query43	0.04	0.04	0.03
Total cold run time: 106.66 s
Total hot run time: 33 s

hello-stephen · 2025-07-30T17:39:33Z

BE UT Coverage Report

Increment line coverage 72.22% (13/18) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	57.78% (16074/27817)
Line Coverage	46.52% (144575/310788)
Region Coverage	35.76% (108775/304164)
Branch Coverage	38.41% (48019/125021)

hello-stephen · 2025-07-30T18:23:05Z

BE Regression && UT Coverage Report

Increment line coverage 68.42% (13/19) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	81.15% (22160/27306)
Line Coverage	73.85% (229228/310413)
Region Coverage	61.64% (192244/311881)
Branch Coverage	65.41% (82671/126393)

github-actions · 2025-07-31T00:59:59Z

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

github-actions · 2025-07-31T01:00:02Z

PR approved by anyone and no changes requested.

code It is easy to leak the lock, so we move the code to one place.

dataroaring · 2025-07-31T01:38:25Z

run buildall

doris-robot · 2025-07-31T02:11:52Z

TPC-H: Total hot run time: 33428 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 104f96848688d53e991bccafc489b375fbb0d80a, data reload: false
------ Round 1 ----------------------------------
q1	17716	5647	5401	5401
q2	1946	299	197	197
q3	10271	1293	690	690
q4	10221	946	509	509
q5	7491	2360	2194	2194
q6	186	166	131	131
q7	886	755	604	604
q8	9283	1198	974	974
q9	6741	5106	5241	5106
q10	6896	2361	1962	1962
q11	453	284	266	266
q12	359	358	216	216
q13	17776	3416	2958	2958
q14	249	249	217	217
q15	518	464	448	448
q16	419	439	362	362
q17	551	816	346	346
q18	7569	7028	6857	6857
q19	1218	957	517	517
q20	309	312	204	204
q21	3376	2488	2274	2274
q22	1061	1083	995	995
Total cold run time: 105495 ms
Total hot run time: 33428 ms
----- Round 2, with runtime_filter_mode=off -----
q1	5794	5746	5689	5689
q2	223	302	214	214
q3	2028	2579	2156	2156
q4	1274	1717	1317	1317
q5	4103	4290	4442	4290
q6	215	174	134	134
q7	2014	1967	1797	1797
q8	2521	2544	2376	2376
q9	7258	7401	7404	7401
q10	3058	3404	2920	2920
q11	573	517	475	475
q12	675	775	657	657
q13	3315	3694	3389	3389
q14	308	301	310	301
q15	498	472	445	445
q16	484	486	448	448
q17	1178	1545	1376	1376
q18	8598	7933	7566	7566
q19	1752	865	763	763
q20	1977	2037	1799	1799
q21	8674	4262	4205	4205
q22	1056	1087	975	975
Total cold run time: 57576 ms
Total hot run time: 50693 ms

doris-robot · 2025-07-31T02:23:28Z

TPC-DS: Total hot run time: 163246 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 104f96848688d53e991bccafc489b375fbb0d80a, data reload: false
reason	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 22:43:56	2023-12-26 22:44:01	NULL	utf-8	NULL	NULL ============================================
query1	1008	387	397	387
query2	6536	1841	1669	1669
query3	6738	224	216	216
query4	27961	23777	23492	23492
query5	4391	653	527	527
query6	315	233	228	228
query7	4648	514	288	288
query8	270	230	225	225
query9	9044	3180	3191	3180
query10	461	317	295	295
query11	15913	15050	14768	14768
query12	183	137	134	134
query13	1652	544	398	398
query14	10306	7634	7641	7634
query15	262	192	171	171
query16	7769	646	439	439
query17	1612	766	636	636
query18	2038	460	332	332
query19	293	214	199	199
query20	153	142	139	139
query21	219	124	107	107
query22	3922	3950	3909	3909
query23	34391	34302	34161	34161
query24	7703	2390	2399	2390
query25	551	527	452	452
query26	738	290	167	167
query27	2547	498	345	345
query28	2997	2269	2284	2269
query29	708	625	521	521
query30	291	236	197	197
query31	896	827	782	782
query32	88	78	78	78
query33	540	427	372	372
query34	794	837	510	510
query35	787	824	746	746
query36	1001	1039	951	951
query37	133	105	103	103
query38	4126	4054	3980	3980
query39	1490	1409	1368	1368
query40	237	144	134	134
query41	63	60	54	54
query42	141	124	126	124
query43	484	504	459	459
query44	1388	873	851	851
query45	199	183	181	181
query46	952	1054	667	667
query47	1789	1853	1769	1769
query48	396	427	324	324
query49	675	503	413	413
query50	653	691	425	425
query51	4125	4245	4130	4130
query52	129	125	117	117
query53	248	293	211	211
query54	658	624	571	571
query55	89	87	84	84
query56	352	358	343	343
query57	1198	1229	1144	1144
query58	335	325	325	325
query59	2586	2660	2497	2497
query60	391	399	399	399
query61	127	125	126	125
query62	807	729	651	651
query63	248	207	210	207
query64	2839	1138	843	843
query65	4244	4091	4104	4091
query66	1215	385	340	340
query67	query68	16995	910	1013	910
query69	1456	280	286	280
query70	1306	1096	1132	1096
query71	702	311	328	311
query72	9183	2279	2154	2154
query73	3490	682	349	349
query74	8988	8897	8861	8861
query75	7679	3191	2684	2684
query76	8632	1203	766	766
query77	1157	418	360	360
query78	query79	17176	626	580	580
query80	2276	591	485	485
query81	567	267	239	239
query82	728	152	120	120
query83	407	286	276	276
query84	304	99	89	89
query85	1740	381	337	337
query86	371	327	311	311
query87	4332	4235	4333	4235
query88	5704	2187	2163	2163
query89	544	358	309	309
query90	2629	230	235	230
query91	150	138	111	111
query92	96	73	69	69
query93	6821	958	661	661
query94	1069	407	277	277
query95	399	336	319	319
query96	520	583	279	279
query97	2708	2736	2666	2666
query98	240	232	223	223
query99	1453	1376	1252	1252
Total cold run time: 301865 ms
Total hot run time: 163246 ms

doris-robot · 2025-07-31T02:28:57Z

ClickBench: Total hot run time: 33.74 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 104f96848688d53e991bccafc489b375fbb0d80a, data reload: false
query1	0.04	0.04	0.03
query2	0.08	0.05	0.04
query3	0.24	0.08	0.07
query4	1.62	0.11	0.10
query5	0.44	0.42	0.43
query6	1.19	0.66	0.67
query7	0.02	0.02	0.02
query8	0.05	0.04	0.04
query9	0.56	0.46	0.47
query10	0.53	0.53	0.52
query11	0.15	0.09	0.10
query12	0.14	0.12	0.11
query13	0.64	0.65	0.64
query14	0.92	1.08	1.09
query15	0.98	0.90	0.88
query16	0.41	0.39	0.39
query17	1.06	1.05	1.08
query18	0.21	0.21	0.20
query19	1.96	1.91	1.88
query20	0.01	0.02	0.01
query21	15.37	0.84	0.55
query22	0.78	1.28	0.67
query23	14.80	1.13	0.61
query24	6.19	1.54	1.26
query25	0.56	0.21	0.16
query26	0.58	0.14	0.14
query27	0.07	0.05	0.06
query28	10.06	0.85	0.46
query29	12.70	3.82	3.36
query30	3.01	3.03	3.01
query31	2.83	0.55	0.40
query32	3.25	0.56	0.49
query33	3.01	3.23	3.16
query34	15.83	5.31	4.94
query35	4.90	4.92	4.96
query36	0.69	0.54	0.50
query37	0.10	0.07	0.07
query38	0.05	0.06	0.04
query39	0.04	0.03	0.03
query40	0.17	0.14	0.14
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.03	0.03
Total cold run time: 106.4 s
Total hot run time: 33.74 s

doris-robot · 2025-07-31T03:16:18Z

BE UT Coverage Report

Increment line coverage 72.22% (13/18) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	57.78% (16074/27817)
Line Coverage	46.52% (144589/310788)
Region Coverage	35.76% (108776/304164)
Branch Coverage	38.42% (48028/125021)

hello-stephen · 2025-07-31T04:02:12Z

BE Regression && UT Coverage Report

Increment line coverage 68.42% (13/19) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	81.25% (22187/27306)
Line Coverage	73.95% (229562/310413)
Region Coverage	61.64% (192252/311881)
Branch Coverage	65.47% (82750/126393)

hello-stephen · 2025-07-31T08:58:40Z

BE Regression && UT Coverage Report

Increment line coverage 68.42% (13/19) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	81.25% (22187/27306)
Line Coverage	73.95% (229562/310413)
Region Coverage	61.64% (192252/311881)
Branch Coverage	65.47% (82750/126393)

liaoxin01

LGTM

Cause: PR #54124 moved the migration and push locks from RowsetBuilder into Tablet::prepare_txn. During that refactor, the call to TxnManager::prepare_txn changed from the const Tablet& overload to the raw tablet_id/tablet_uid overload. This silently bypassed the TABLET_SHUTDOWN guard introduced by PR #42296. Impact: A load thread holding an old Tablet instance could resume after migration and register the stale tablet UID in the transaction map. Publish would then operate on the reloaded Tablet with a new UID and leave the transaction in an E-909 state. Fix: Call the const Tablet& overload with *this while the migration and push locks are held. This preserves the lock encapsulation from PR #54124 and restores the shutdown-tablet protection from PR #42296. Test: Add a deterministic unit test that marks a Tablet as TABLET_SHUTDOWN, verifies Tablet::prepare_txn fails, and verifies no stale tablet entry is registered. TxnManagerTest.* passes 16/16.

Cause: PR apache#54124 moved the migration and push locks from RowsetBuilder into Tablet::prepare_txn. During that refactor, the call to TxnManager::prepare_txn changed from the const Tablet& overload to the raw tablet_id/tablet_uid overload. This silently bypassed the TABLET_SHUTDOWN guard introduced by PR apache#42296. Impact: A load thread holding an old Tablet instance could resume after migration and register the stale tablet UID in the transaction map. Publish would then operate on the reloaded Tablet with a new UID and leave the transaction in an E-909 state. Fix: Call the const Tablet& overload with *this while the migration and push locks are held. This preserves the lock encapsulation from PR apache#54124 and restores the shutdown-tablet protection from PR apache#42296. Test: Add a deterministic unit test that marks a Tablet as TABLET_SHUTDOWN, verifies Tablet::prepare_txn fails, and verifies no stale tablet entry is registered. TxnManagerTest.* passes 16/16.

Cause: PR #54124 moved the migration and push locks from RowsetBuilder into Tablet::prepare_txn. During that refactor, the call to TxnManager::prepare_txn changed from the const Tablet& overload to the raw tablet_id/tablet_uid overload. This silently bypassed the TABLET_SHUTDOWN guard introduced by PR #42296. Impact: A load thread holding an old Tablet instance could resume after migration and register the stale tablet UID in the transaction map. Publish would then operate on the reloaded Tablet with a new UID and leave the transaction in an E-909 state. Fix: Call the const Tablet& overload with *this while the migration and push locks are held. This preserves the lock encapsulation from PR #54124 and restores the shutdown-tablet protection from PR #42296. Test: Add a deterministic unit test that marks a Tablet as TABLET_SHUTDOWN, verifies Tablet::prepare_txn fails, and verifies no stale tablet entry is registered. TxnManagerTest.* passes 16/16.

dataroaring force-pushed the refactor_txn branch 2 times, most recently from b9ad4c1 to 7384e2fCompare July 30, 2025 15:21

w41ter approved these changes Jul 31, 2025
View reviewed changes

github-actionsBot added the approved Indicates a PR has been approved by one committer. label Jul 31, 2025

github-actionsBot added the reviewed label Jul 31, 2025

[refactor](prepare_txn) move prepare_txn to tablet to reduce duplicated
104f968
code It is easy to leak the lock, so we move the code to one place.

dataroaring force-pushed the refactor_txn branch from 7384e2f to 104f968Compare July 31, 2025 01:38

liaoxin01 approved these changes Aug 1, 2025
View reviewed changes

dataroaring merged commit 83765d9 into apache:masterAug 1, 2025
25 of 26 checks passed

deardeng mentioned this pull request Aug 4, 2026
[fix](storage) Reject prepare txn on shutdown tablet #66448
Merged
16 tasks

Uh oh!

Conversation

dataroaring commented Jul 30, 2025

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

hello-stephen commented Jul 30, 2025

Uh oh!

dataroaring commented Jul 30, 2025

Uh oh!

dataroaring commented Jul 30, 2025

Uh oh!

doris-robot commented Jul 30, 2025

Uh oh!

doris-robot commented Jul 30, 2025

Uh oh!

doris-robot commented Jul 30, 2025

Uh oh!

hello-stephen commented Jul 30, 2025

BE UT Coverage Report

Uh oh!

hello-stephen commented Jul 30, 2025

BE Regression && UT Coverage Report

Uh oh!

github-actionsBot commented Jul 31, 2025

Uh oh!

github-actionsBot commented Jul 31, 2025

Uh oh!

dataroaring commented Jul 31, 2025

Uh oh!

doris-robot commented Jul 31, 2025

Uh oh!

doris-robot commented Jul 31, 2025

Uh oh!

doris-robot commented Jul 31, 2025

Uh oh!

doris-robot commented Jul 31, 2025

BE UT Coverage Report

Uh oh!

hello-stephen commented Jul 31, 2025

BE Regression && UT Coverage Report

Uh oh!

hello-stephen commented Jul 31, 2025

BE Regression && UT Coverage Report

Uh oh!

liaoxin01 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants