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.

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

[fix](commit) Fix does not skip commit if txn state is committed or visible #39786

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

mymeiyi
Copy link
Contributor

@mymeiyi mymeiyi commented Aug 22, 2024

introduced in #32980
if a txn is committed twice, the check txn state is committed or visible and skip commit logic does not work

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

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Aug 22, 2024

run buildall

Copy link
Collaborator

@yujun777 yujun777 left a comment

Choose a reason for hiding this comment

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

void   XXXTransaction() {
        readlock()
         txn = getTransaction()
         checkTransactionStateBeforeXXX() 
        readunlock()

         do something no need lock()

         writeLock()
         checkTransactionStateBeforeXXX() 
          ...
         writeUnlock()
}

maybe in write lock scope, check transaction status again is better ?

here XXXTransaction XXX should include PreCommit/Commit/Abort

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17650	4745	4420	4420
q2	2369	178	181	178
q3	10482	1171	1117	1117
q4	10140	713	757	713
q5	7712	2888	2854	2854
q6	219	137	141	137
q7	985	620	596	596
q8	9331	2109	2082	2082
q9	7081	6561	6557	6557
q10	7002	2140	2302	2140
q11	465	246	257	246
q12	396	229	224	224
q13	17891	3071	3049	3049
q14	282	257	238	238
q15	525	495	490	490
q16	498	405	403	403
q17	1005	740	745	740
q18	7472	6902	6857	6857
q19	1399	1064	1148	1064
q20	676	330	331	330
q21	3997	3063	2929	2929
q22	1129	1018	1025	1018
Total cold run time: 108706 ms
Total hot run time: 38382 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4499	4406	4313	4313
q2	389	275	266	266
q3	2934	2673	2675	2673
q4	2032	1692	1629	1629
q5	5402	5470	5430	5430
q6	217	132	133	132
q7	2156	1775	1777	1775
q8	3292	3366	3344	3344
q9	8538	8530	8468	8468
q10	3467	3211	3155	3155
q11	595	513	510	510
q12	790	594	585	585
q13	14541	3107	3064	3064
q14	308	267	278	267
q15	527	468	482	468
q16	478	438	453	438
q17	1831	1507	1485	1485
q18	7949	7699	7591	7591
q19	1688	1562	1494	1494
q20	2065	1773	1805	1773
q21	5573	5224	5239	5224
q22	1122	1008	1041	1008
Total cold run time: 70393 ms
Total hot run time: 55092 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 186931 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 15c3d955c6258e431809c7caf8c973879c908f0c, data reload: false

query1	896	378	359	359
query2	6444	1905	1891	1891
query3	6650	207	214	207
query4	28959	23188	23118	23118
query5	4133	513	475	475
query6	271	167	173	167
query7	4586	297	294	294
query8	253	199	212	199
query9	8510	2446	2438	2438
query10	435	280	272	272
query11	15677	14965	14983	14965
query12	153	100	98	98
query13	1638	372	393	372
query14	10123	7063	7024	7024
query15	280	173	172	172
query16	7881	483	475	475
query17	1605	583	569	569
query18	1867	308	305	305
query19	226	184	145	145
query20	120	110	109	109
query21	210	104	101	101
query22	4271	4199	4236	4199
query23	34244	33739	33388	33388
query24	11190	2887	2899	2887
query25	624	383	393	383
query26	1155	166	161	161
query27	2341	285	281	281
query28	7069	2035	2009	2009
query29	789	431	397	397
query30	311	160	149	149
query31	999	799	769	769
query32	96	58	59	58
query33	781	286	279	279
query34	1007	502	502	502
query35	865	743	721	721
query36	1098	954	954	954
query37	156	87	83	83
query38	4067	3899	3937	3899
query39	1442	1368	1370	1368
query40	200	120	118	118
query41	47	46	45	45
query42	110	98	99	98
query43	486	468	468	468
query44	1267	744	774	744
query45	195	166	168	166
query46	1106	782	742	742
query47	1839	1769	1771	1769
query48	380	288	308	288
query49	1083	424	431	424
query50	810	424	440	424
query51	7061	6932	6943	6932
query52	99	88	86	86
query53	254	184	184	184
query54	984	472	465	465
query55	81	77	79	77
query56	287	262	265	262
query57	1157	1039	1035	1035
query58	242	219	240	219
query59	3127	2625	2734	2625
query60	300	276	282	276
query61	101	98	102	98
query62	822	650	670	650
query63	231	190	190	190
query64	5456	2401	1844	1844
query65	3276	3188	3170	3170
query66	1330	327	337	327
query67	15594	15286	15076	15076
query68	3560	592	579	579
query69	395	294	294	294
query70	1164	1068	1120	1068
query71	345	285	280	280
query72	6348	2309	2085	2085
query73	769	322	331	322
query74	9200	8768	8870	8768
query75	3384	2680	2709	2680
query76	2186	1083	971	971
query77	487	374	315	315
query78	9605	9022	9071	9022
query79	1022	553	547	547
query80	718	518	529	518
query81	489	232	232	232
query82	232	143	142	142
query83	177	150	155	150
query84	226	86	79	79
query85	900	292	285	285
query86	308	300	284	284
query87	4490	4346	4366	4346
query88	2973	2381	2366	2366
query89	392	301	290	290
query90	1957	205	204	204
query91	127	106	104	104
query92	65	55	55	55
query93	1028	550	544	544
query94	924	288	292	288
query95	363	274	267	267
query96	589	283	274	274
query97	3214	3065	3080	3065
query98	215	221	202	202
query99	1500	1281	1261	1261
Total cold run time: 280699 ms
Total hot run time: 186931 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.05
query2	0.09	0.04	0.03
query3	0.23	0.06	0.06
query4	1.65	0.11	0.10
query5	0.50	0.49	0.50
query6	1.14	0.73	0.72
query7	0.02	0.02	0.02
query8	0.06	0.04	0.04
query9	0.55	0.48	0.48
query10	0.54	0.54	0.53
query11	0.16	0.12	0.12
query12	0.15	0.12	0.13
query13	0.62	0.58	0.59
query14	0.76	0.80	0.78
query15	0.89	0.82	0.82
query16	0.37	0.37	0.38
query17	0.98	1.02	0.98
query18	0.22	0.21	0.20
query19	1.83	1.76	1.74
query20	0.02	0.01	0.01
query21	15.40	0.67	0.66
query22	3.98	7.89	1.85
query23	18.28	1.44	1.29
query24	2.14	0.23	0.22
query25	0.15	0.09	0.08
query26	0.27	0.17	0.18
query27	0.08	0.08	0.07
query28	13.17	1.01	1.00
query29	12.62	3.37	3.38
query30	0.26	0.05	0.06
query31	2.89	0.41	0.39
query32	3.24	0.47	0.48
query33	2.99	2.97	3.02
query34	17.16	4.38	4.39
query35	4.42	4.47	4.43
query36	0.66	0.47	0.47
query37	0.18	0.16	0.15
query38	0.16	0.15	0.15
query39	0.05	0.04	0.04
query40	0.15	0.13	0.13
query41	0.09	0.05	0.05
query42	0.05	0.05	0.05
query43	0.05	0.04	0.04
Total cold run time: 109.26 s
Total hot run time: 30.71 s

@mymeiyi mymeiyi changed the title [fix](commit) Fix dose not skip commit if txn state is committed or visible [fix](commit) Fix does not skip commit if txn state is committed or visible Aug 22, 2024
@mymeiyi
Copy link
Contributor Author

mymeiyi commented Aug 23, 2024

run fe_ut

@mymeiyi
Copy link
Contributor Author

mymeiyi commented Aug 23, 2024

run feut

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

Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 23, 2024
@liaoxin01
Copy link
Contributor

Please add a description of what scenario triggers the issue.

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.

There should be a lock hold when check and commit. Otherwise, two parallel thread may check ok and commit together. It seems that lock on trasaction state is enough.

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Aug 24, 2024
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

Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Aug 24, 2024
Copy link
Contributor

@liaoxin01 liaoxin01 left a comment

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 33d4d93 into apache:master Aug 27, 2024
35 of 40 checks passed
dataroaring pushed a commit that referenced this pull request Aug 27, 2024
…isible (#39786)

introduced in #32980
if a txn is committed twice, the check txn state is committed or visible
and skip commit logic does not work
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. dev/3.0.2-merged p0_b
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants