Skip to content

[Bug](partition) should not do reset for the partition_sorts #48458

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
Merged
dataroaring merged 1 commit into from
Mar 19, 2025
Merged

[Bug](partition) should not do reset for the partition_sorts #48458

dataroaring merged 1 commit into from
Mar 19, 2025

Conversation

@zhangstar333
Copy link
Contributor

@zhangstar333 zhangstar333 commented Feb 28, 2025 •
edited
Loading

What problem does this PR solve?

Problem Summary:
the partition_sorts is unique_ptr，could release after destroy.
and when some extreme case like cancel, if source reset the sorter early, the sink operator will coredump still use it.

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 Feb 28, 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?

@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Feb 28, 2025

run buildall

1 similar comment
@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Feb 28, 2025

run buildall

@zhangstar333 zhangstar333 force-pushed the branch-3.0 branch 3 times, most recently from 4d57ab7 to 96da168 Compare March 4, 2025 07:03
@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Mar 4, 2025

run buildall

1 similar comment
@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Mar 5, 2025

run buildall

@zhangstar333
Copy link
Contributor Author

zhangstar333 commented Mar 17, 2025

run buildall

@zhangstar333 zhangstar333 changed the title [branch-30](join) report error in join node when is not nereids [Bug](partition) should not do reset for the partition_sorts Mar 17, 2025
@doris-robot
Copy link

doris-robot commented Mar 17, 2025

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

------ Round 1 ----------------------------------
q1	17565	7171	7050	7050
q2	2055	170	165	165
q3	10662	1075	1199	1075
q4	11062	734	794	734
q5	7754	2849	2783	2783
q6	215	133	128	128
q7	983	618	621	618
q8	9381	1961	2001	1961
q9	6605	6377	6389	6377
q10	7028	2285	2262	2262
q11	473	271	265	265
q12	402	217	218	217
q13	17813	2996	3015	2996
q14	235	203	206	203
q15	499	457	461	457
q16	671	573	590	573
q17	971	587	614	587
q18	7271	6611	6776	6611
q19	1386	1057	1049	1049
q20	489	212	200	200
q21	4049	3377	3371	3371
q22	1073	1015	988	988
Total cold run time: 108642 ms
Total hot run time: 40670 ms

----- Round 2, with runtime_filter_mode=off -----
q1	6556	6531	6538	6531
q2	336	226	234	226
q3	2884	2840	2939	2840
q4	2043	1856	1859	1856
q5	5697	5738	5718	5718
q6	213	127	126	126
q7	2164	1828	1846	1828
q8	3408	3554	3493	3493
q9	8706	8861	8843	8843
q10	3579	3539	3496	3496
q11	594	493	514	493
q12	793	643	604	604
q13	8910	3243	3198	3198
q14	302	280	279	279
q15	509	475	466	466
q16	690	652	634	634
q17	1836	1591	1607	1591
q18	8331	7699	7709	7699
q19	1660	1509	1557	1509
q20	2136	1849	1855	1849
q21	5673	5351	5299	5299
q22	1148	1053	1034	1034
Total cold run time: 68168 ms
Total hot run time: 59612 ms

@doris-robot
Copy link

doris-robot commented Mar 17, 2025

TPC-DS: Total hot run time: 196408 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 03fd985bceabbe94cde804a989efe1a5002312ca, data reload: false

query1	1286	931	953	931
query2	6210	2027	1992	1992
query3	10927	4215	4206	4206
query4	65581	28908	23104	23104
query5	4954	471	462	462
query6	415	184	187	184
query7	5588	325	324	324
query8	305	227	226	226
query9	9099	2617	2609	2609
query10	466	267	279	267
query11	17334	15334	15605	15334
query12	161	104	102	102
query13	1545	458	463	458
query14	10416	6837	7254	6837
query15	201	185	191	185
query16	7084	490	456	456
query17	1055	562	590	562
query18	1889	327	340	327
query19	211	178	183	178
query20	126	120	113	113
query21	212	108	110	108
query22	4677	4480	4380	4380
query23	34039	33914	34460	33914
query24	6271	2952	2971	2952
query25	536	414	427	414
query26	673	170	170	170
query27	1955	354	359	354
query28	4365	2486	2455	2455
query29	718	506	459	459
query30	253	166	161	161
query31	990	831	811	811
query32	69	61	58	58
query33	415	301	311	301
query34	917	526	524	524
query35	851	772	747	747
query36	1090	988	964	964
query37	114	71	70	70
query38	4160	4004	3971	3971
query39	1495	1493	1496	1493
query40	204	100	99	99
query41	47	48	47	47
query42	119	100	101	100
query43	544	507	490	490
query44	1203	835	829	829
query45	186	164	169	164
query46	1156	749	730	730
query47	2030	1945	1932	1932
query48	487	398	384	384
query49	731	406	387	387
query50	859	437	429	429
query51	7336	7381	7106	7106
query52	104	87	89	87
query53	260	181	191	181
query54	572	458	465	458
query55	77	79	76	76
query56	257	235	246	235
query57	1214	1135	1127	1127
query58	211	204	207	204
query59	3156	3045	2840	2840
query60	278	258	262	258
query61	108	108	108	108
query62	757	654	672	654
query63	227	193	193	193
query64	1363	660	673	660
query65	3232	3176	3222	3176
query66	700	297	304	297
query67	16037	15741	15568	15568
query68	3997	579	573	573
query69	434	265	259	259
query70	1093	1146	1109	1109
query71	360	258	268	258
query72	6350	4022	4138	4022
query73	754	359	346	346
query74	10338	8993	8992	8992
query75	3354	2624	2694	2624
query76	2198	1036	1014	1014
query77	531	285	273	273
query78	10791	9598	10017	9598
query79	1104	605	597	597
query80	935	441	428	428
query81	522	238	238	238
query82	185	89	86	86
query83	161	150	143	143
query84	282	81	79	79
query85	957	299	286	286
query86	400	305	300	300
query87	4459	4219	4499	4219
query88	4078	2407	2352	2352
query89	412	286	300	286
query90	1984	186	187	186
query91	180	148	147	147
query92	66	48	50	48
query93	1092	547	582	547
query94	858	295	265	265
query95	365	264	255	255
query96	605	288	285	285
query97	3295	3174	3135	3135
query98	218	207	200	200
query99	1574	1276	1283	1276
Total cold run time: 316040 ms
Total hot run time: 196408 ms

@doris-robot
Copy link

doris-robot commented Mar 17, 2025

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

query1	0.04	0.03	0.03
query2	0.07	0.04	0.02
query3	0.23	0.06	0.07
query4	1.62	0.10	0.10
query5	0.52	0.49	0.52
query6	1.13	0.73	0.73
query7	0.02	0.02	0.02
query8	0.04	0.03	0.03
query9	0.56	0.50	0.48
query10	0.55	0.55	0.55
query11	0.15	0.11	0.12
query12	0.14	0.11	0.12
query13	0.61	0.58	0.60
query14	2.87	2.75	2.74
query15	0.89	0.83	0.83
query16	0.38	0.38	0.38
query17	1.05	1.05	1.07
query18	0.26	0.23	0.22
query19	1.95	1.87	2.09
query20	0.01	0.02	0.01
query21	15.37	0.61	0.61
query22	2.44	1.88	1.69
query23	17.05	0.89	0.94
query24	3.12	1.54	1.32
query25	0.25	0.11	0.05
query26	0.58	0.14	0.13
query27	0.05	0.04	0.04
query28	9.71	0.54	0.45
query29	12.60	3.30	3.27
query30	0.24	0.06	0.06
query31	2.86	0.39	0.40
query32	3.23	0.46	0.45
query33	2.94	3.00	2.98
query34	16.97	4.47	4.51
query35	4.57	4.52	4.52
query36	0.69	0.49	0.50
query37	0.08	0.06	0.05
query38	0.05	0.04	0.03
query39	0.03	0.02	0.03
query40	0.16	0.12	0.12
query41	0.07	0.02	0.02
query42	0.04	0.02	0.02
query43	0.03	0.02	0.02
Total cold run time: 106.22 s
Total hot run time: 32.5 s

@doris-robot
Copy link

doris-robot commented Mar 17, 2025

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 38.87% (10166/26154)
Line Coverage 30.29% (86643/286087)
Region Coverage 29.35% (44535/151737)
Branch Coverage 25.87% (22647/87548)

dataroaring
dataroaring approved these changes Mar 19, 2025
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

@dataroaring dataroaring merged commit f528713 into apache:branch-3.0 Mar 19, 2025
23 of 25 checks passed
csun5285 pushed a commit to csun5285/doris that referenced this pull request Mar 28, 2025
@zhangstar333
[Bug](partition) should not do reset for the partition_sorts (apache#…
4b25bca 
…48458) (apache#3822)

cherry-pick from master
[apache#48458](https://github.com/apache/doris/pull/48458/files)
@gavinchou gavinchou mentioned this pull request Apr 23, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dataroaring dataroaring dataroaring approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zhangstar333 @Thearas @doris-robot @dataroaring