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](Compaction) atomic should not be implicitly converted to int64_t #49427

Merged
merged 3 commits into from
Mar 25, 2025

Conversation

Yukang-Lian
Copy link
Collaborator

@Yukang-Lian Yukang-Lian commented Mar 24, 2025

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

atomic<int64_t> cannot be implicitly converted to int64_t, which may lead to data errors in high concurrency situations.

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

@Thearas
Copy link
Contributor

Thearas commented Mar 24, 2025

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?

@Yukang-Lian
Copy link
Collaborator Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	26110	5155	5346	5155
q2	2063	290	163	163
q3	10423	1215	688	688
q4	10232	994	525	525
q5	7502	2421	2343	2343
q6	185	163	131	131
q7	934	739	615	615
q8	9316	1299	1079	1079
q9	6782	5054	5114	5054
q10	6861	2322	1883	1883
q11	474	278	245	245
q12	355	356	214	214
q13	17773	3688	3056	3056
q14	238	237	206	206
q15	530	489	484	484
q16	644	636	599	599
q17	576	842	356	356
q18	7564	7373	7079	7079
q19	1469	991	562	562
q20	309	341	193	193
q21	4110	2722	2449	2449
q22	1087	1030	1003	1003
Total cold run time: 115537 ms
Total hot run time: 34082 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5589	5124	5115	5115
q2	247	330	227	227
q3	2184	2677	2341	2341
q4	1482	1870	1572	1572
q5	4523	4513	4446	4446
q6	221	167	126	126
q7	2004	1892	1767	1767
q8	2584	2551	2536	2536
q9	7342	7031	7262	7031
q10	3034	3208	2784	2784
q11	590	508	503	503
q12	683	795	630	630
q13	3549	3908	3375	3375
q14	286	297	289	289
q15	523	483	464	464
q16	651	694	672	672
q17	1151	1616	1389	1389
q18	7817	7523	7598	7523
q19	855	806	884	806
q20	1913	2007	1847	1847
q21	5430	5189	5048	5048
q22	1126	1084	1050	1050
Total cold run time: 53784 ms
Total hot run time: 51541 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 193942 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 7f81c6992427ae3eff345671336846ecc7e60b16, data reload: false

query1	1422	1061	1067	1061
query2	6195	1974	1907	1907
query3	11146	4640	4646	4640
query4	26031	23305	23267	23267
query5	3856	658	479	479
query6	319	204	207	204
query7	3988	481	271	271
query8	307	249	236	236
query9	8527	2578	2578	2578
query10	483	312	250	250
query11	16087	15117	14867	14867
query12	160	107	104	104
query13	1584	528	384	384
query14	9657	6168	6378	6168
query15	199	185	167	167
query16	7610	616	456	456
query17	1228	755	559	559
query18	2028	397	332	332
query19	211	186	186	186
query20	125	132	133	132
query21	208	119	112	112
query22	4548	4726	4379	4379
query23	34749	33656	33691	33656
query24	8418	2409	2442	2409
query25	527	466	404	404
query26	1270	272	142	142
query27	2868	512	333	333
query28	4638	2449	2410	2410
query29	696	566	437	437
query30	279	227	200	200
query31	918	887	810	810
query32	75	70	59	59
query33	533	365	371	365
query34	803	886	502	502
query35	828	835	774	774
query36	946	1028	916	916
query37	119	99	79	79
query38	4229	4139	4266	4139
query39	1539	1462	1434	1434
query40	212	116	106	106
query41	52	52	52	52
query42	134	107	110	107
query43	506	517	499	499
query44	1345	817	853	817
query45	194	183	169	169
query46	849	1020	643	643
query47	1906	1886	1815	1815
query48	396	431	324	324
query49	749	508	430	430
query50	701	751	420	420
query51	4337	4364	4271	4271
query52	110	105	94	94
query53	225	267	184	184
query54	517	520	435	435
query55	85	84	84	84
query56	290	294	253	253
query57	1192	1212	1122	1122
query58	270	248	253	248
query59	2859	2906	2842	2842
query60	303	287	294	287
query61	151	146	147	146
query62	776	769	668	668
query63	225	190	193	190
query64	4351	1178	679	679
query65	4536	4463	4486	4463
query66	1089	401	300	300
query67	16043	15847	15467	15467
query68	9013	872	505	505
query69	466	293	260	260
query70	1173	1126	1083	1083
query71	482	288	269	269
query72	5735	5096	5218	5096
query73	731	635	344	344
query74	9103	8905	8971	8905
query75	4066	3246	2717	2717
query76	3760	1195	740	740
query77	787	363	283	283
query78	10117	10598	9343	9343
query79	2268	808	560	560
query80	603	516	441	441
query81	487	267	212	212
query82	452	125	100	100
query83	206	167	160	160
query84	286	104	75	75
query85	766	346	299	299
query86	349	313	289	289
query87	4475	4574	4293	4293
query88	3364	2204	2198	2198
query89	398	313	289	289
query90	1963	204	206	204
query91	140	144	107	107
query92	70	61	60	60
query93	1862	1072	576	576
query94	671	413	266	266
query95	351	270	256	256
query96	493	557	287	287
query97	3321	3454	3331	3331
query98	217	200	201	200
query99	1468	1404	1271	1271
Total cold run time: 283400 ms
Total hot run time: 193942 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.12	0.11	0.11
query3	0.25	0.18	0.19
query4	1.59	0.17	0.19
query5	0.60	0.58	0.59
query6	1.18	0.72	0.71
query7	0.02	0.02	0.01
query8	0.04	0.04	0.03
query9	0.57	0.54	0.52
query10	0.57	0.58	0.58
query11	0.15	0.11	0.11
query12	0.14	0.12	0.11
query13	0.62	0.60	0.62
query14	2.70	2.70	2.82
query15	0.94	0.87	0.86
query16	0.38	0.40	0.38
query17	1.04	1.02	1.04
query18	0.21	0.20	0.20
query19	1.89	1.97	1.78
query20	0.01	0.01	0.01
query21	15.38	0.91	0.54
query22	0.77	1.18	0.77
query23	14.79	1.32	0.63
query24	7.40	1.83	0.32
query25	0.30	0.18	0.07
query26	0.53	0.17	0.14
query27	0.05	0.05	0.05
query28	9.62	0.92	0.45
query29	12.57	4.11	3.37
query30	0.26	0.09	0.07
query31	2.82	0.64	0.39
query32	3.22	0.55	0.48
query33	3.05	3.12	3.06
query34	15.77	5.16	4.48
query35	4.59	4.56	4.56
query36	0.68	0.50	0.49
query37	0.08	0.07	0.06
query38	0.06	0.04	0.04
query39	0.03	0.02	0.02
query40	0.17	0.13	0.12
query41	0.08	0.03	0.02
query42	0.04	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 105.36 s
Total hot run time: 30.87 s

@hello-stephen
Copy link
Contributor

BE UT Coverage Report

Increment line coverage 14.29% (1/7) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 50.22% (13439/26760)
Line Coverage 39.67% (116425/293481)
Region Coverage 38.38% (59163/154162)
Branch Coverage 33.51% (29886/89182)

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 bot added the approved Indicates a PR has been approved by one committer. label Mar 25, 2025
Copy link
Contributor

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

Copy link
Contributor

PR approved by anyone and no changes requested.

@liaoxin01
Copy link
Contributor

Please add a description for this PR.

@dataroaring dataroaring merged commit f70aee9 into apache:master Mar 25, 2025
27 of 30 checks passed
github-actions bot pushed a commit that referenced this pull request Mar 25, 2025
#49427)

atomic<int64_t> cannot be implicitly converted to int64_t, which may
lead to data errors in high concurrency situations.
github-actions bot pushed a commit that referenced this pull request Mar 25, 2025
#49427)

atomic<int64_t> cannot be implicitly converted to int64_t, which may
lead to data errors in high concurrency situations.
dataroaring pushed a commit that referenced this pull request Mar 25, 2025
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/2.1.x dev/3.0.5-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants