Skip to content

[Bug](fix) Fix topn agg limit may get error result in when refresh heap (#47844) #47903

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
Feb 14, 2025
Merged

[Bug](fix) Fix topn agg limit may get error result in when refresh heap (#47844) #47903

dataroaring merged 1 commit into from
Feb 14, 2025

Conversation

@HappenLee
Copy link
Contributor

@HappenLee HappenLee commented Feb 14, 2025

cherry pick #47844

@HappenLee
[Bug](fix) Fix topn agg limit may get error result in when refresh he…
9fdb1d9 
…ap (apache#47844)

1.  heap refresh should emplace before pop
2.  Add UT for the heap
3.  refactor the refresh to AggSharedState
@Thearas
Copy link
Contributor

Thearas commented Feb 14, 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?

@HappenLee
Copy link
Contributor Author

HappenLee commented Feb 14, 2025

run buildall

@doris-robot
Copy link

doris-robot commented Feb 14, 2025

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

------ Round 1 ----------------------------------
q1	17567	7477	7210	7210
q2	2066	177	164	164
q3	10649	1066	1185	1066
q4	10569	765	716	716
q5	7723	2806	2767	2767
q6	236	149	140	140
q7	1000	616	601	601
q8	9360	1949	2032	1949
q9	6541	6402	6331	6331
q10	6992	2257	2282	2257
q11	470	265	259	259
q12	397	212	212	212
q13	17777	2992	3017	2992
q14	234	216	213	213
q15	573	546	519	519
q16	668	595	597	595
q17	977	578	540	540
q18	7238	6666	6724	6666
q19	1406	1036	1120	1036
q20	481	212	198	198
q21	4078	3159	3186	3159
q22	1100	995	965	965
Total cold run time: 108102 ms
Total hot run time: 40555 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7198	7218	7193	7193
q2	328	232	247	232
q3	2863	2916	2882	2882
q4	2083	1781	1822	1781
q5	5694	5682	5749	5682
q6	223	136	143	136
q7	2306	1764	1833	1764
q8	3365	3490	3534	3490
q9	8751	8945	8892	8892
q10	3599	3573	3563	3563
q11	589	499	502	499
q12	791	604	595	595
q13	9081	3135	3144	3135
q14	300	289	267	267
q15	574	527	537	527
q16	690	656	659	656
q17	1860	1622	1640	1622
q18	8319	7820	7724	7724
q19	1649	1644	1622	1622
q20	2107	1897	1860	1860
q21	5596	5278	5309	5278
q22	1161	1051	1033	1033
Total cold run time: 69127 ms
Total hot run time: 60433 ms

@doris-robot
Copy link

doris-robot commented Feb 14, 2025

TPC-DS: Total hot run time: 198570 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 9fdb1d959972d04d36cdddb7df830caaeb48f530, data reload: false

query1	1304	951	936	936
query2	6234	2134	2168	2134
query3	10816	4475	4197	4197
query4	65766	28747	24024	24024
query5	4958	468	454	454
query6	411	179	198	179
query7	5661	311	307	307
query8	307	223	221	221
query9	9128	2710	2694	2694
query10	472	266	253	253
query11	17359	15262	15965	15262
query12	164	99	104	99
query13	1581	441	423	423
query14	10445	7791	7179	7179
query15	214	191	183	183
query16	7135	513	510	510
query17	1076	591	590	590
query18	1877	343	336	336
query19	227	159	165	159
query20	112	108	104	104
query21	213	102	110	102
query22	4597	4725	4404	4404
query23	34518	34163	34017	34017
query24	6147	2863	2866	2863
query25	529	413	405	405
query26	668	166	167	166
query27	1781	354	366	354
query28	3921	2480	2424	2424
query29	719	448	445	445
query30	245	160	168	160
query31	1028	827	828	827
query32	68	57	57	57
query33	453	282	275	275
query34	934	504	517	504
query35	845	768	763	763
query36	1114	951	983	951
query37	124	75	75	75
query38	4158	3974	4008	3974
query39	1535	1462	1522	1462
query40	200	104	100	100
query41	48	47	49	47
query42	113	99	103	99
query43	544	515	517	515
query44	1213	837	841	837
query45	186	172	169	169
query46	1116	735	727	727
query47	2051	1958	1936	1936
query48	464	369	396	369
query49	742	409	391	391
query50	844	434	453	434
query51	7349	7322	6923	6923
query52	105	91	89	89
query53	257	182	180	180
query54	552	459	450	450
query55	78	76	77	76
query56	261	260	248	248
query57	1246	1091	1112	1091
query58	212	214	213	213
query59	3227	2994	3036	2994
query60	279	276	255	255
query61	109	106	110	106
query62	835	715	753	715
query63	214	187	196	187
query64	1390	701	670	670
query65	3258	3200	3198	3198
query66	709	309	306	306
query67	15861	15707	15676	15676
query68	3752	603	577	577
query69	429	264	263	263
query70	1177	1146	1120	1120
query71	363	257	251	251
query72	6254	4178	4273	4178
query73	744	352	354	352
query74	10041	9047	9078	9047
query75	3370	2688	2670	2670
query76	1840	1134	1035	1035
query77	500	270	260	260
query78	10445	9681	9655	9655
query79	2468	594	605	594
query80	1244	422	419	419
query81	538	240	238	238
query82	449	118	114	114
query83	173	147	138	138
query84	285	82	76	76
query85	965	299	291	291
query86	422	303	307	303
query87	4500	4416	4364	4364
query88	4372	2370	2352	2352
query89	418	293	281	281
query90	2038	187	188	187
query91	179	146	153	146
query92	68	47	48	47
query93	2688	554	549	549
query94	897	303	307	303
query95	359	256	256	256
query96	610	284	291	284
query97	3333	3141	3254	3141
query98	208	218	200	200
query99	1680	1436	1406	1406
Total cold run time: 319116 ms
Total hot run time: 198570 ms

@doris-robot
Copy link

doris-robot commented Feb 14, 2025

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

query1	0.03	0.03	0.03
query2	0.06	0.03	0.03
query3	0.23	0.06	0.06
query4	1.62	0.10	0.10
query5	0.53	0.52	0.53
query6	1.13	0.73	0.74
query7	0.02	0.01	0.01
query8	0.05	0.05	0.03
query9	0.56	0.50	0.51
query10	0.54	0.54	0.57
query11	0.15	0.11	0.10
query12	0.14	0.12	0.11
query13	0.62	0.59	0.59
query14	2.85	2.71	2.78
query15	0.92	0.83	0.82
query16	0.39	0.38	0.37
query17	1.06	1.04	1.04
query18	0.23	0.21	0.22
query19	1.93	1.73	2.03
query20	0.01	0.01	0.01
query21	15.35	0.57	0.59
query22	2.48	2.59	1.63
query23	16.89	1.08	0.89
query24	3.11	0.61	0.92
query25	0.26	0.04	0.07
query26	0.44	0.13	0.14
query27	0.05	0.03	0.04
query28	11.03	1.10	1.06
query29	12.58	3.28	3.26
query30	0.24	0.07	0.05
query31	2.87	0.38	0.37
query32	3.24	0.46	0.45
query33	3.02	3.01	3.04
query34	17.23	4.48	4.49
query35	4.54	4.55	4.55
query36	0.67	0.48	0.50
query37	0.10	0.07	0.06
query38	0.05	0.03	0.03
query39	0.04	0.02	0.03
query40	0.15	0.13	0.12
query41	0.09	0.02	0.02
query42	0.04	0.02	0.02
query43	0.04	0.04	0.03
Total cold run time: 107.58 s
Total hot run time: 32.15 s

superdiaodiao
superdiaodiao reviewed Feb 14, 2025
View reviewed changes
be/test/pipeline/operator/agg_shared_state_test.cpp
_test_column = int_type->create_column();
auto* col_data = reinterpret_cast<vectorized::ColumnVector<int>*>(_test_column.get());

// Insert test values: 5, 3, 1, -2, -1, 0
Copy link
Contributor

@superdiaodiao superdiaodiao Feb 14, 2025

Choose a reason for hiding this comment

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

nit:
It seems that the following code inserted the vaule 2 not the value -2?

dataroaring
dataroaring approved these changes Feb 14, 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 95f9b32 into apache:branch-3.0 Feb 14, 2025
23 of 26 checks passed
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

+1 more reviewer

@superdiaodiao superdiaodiao superdiaodiao left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@HappenLee @Thearas @doris-robot @superdiaodiao @dataroaring