Skip to content

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

Merged
HappenLee merged 1 commit into
apache:masterfrom
HappenLee:old
Feb 13, 2025
Merged

[Bug](fix) Fix topn agg limit may get error result in when refresh heap#47844
HappenLee merged 1 commit into
apache:masterfrom
HappenLee:old

Conversation

@HappenLee

@HappenLeeHappenLee commented Feb 12, 2025

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

  1. heap refresh should emplace before pop
  2. Add UT for the heap
  3. refactor the refresh to AggSharedState

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

Thearas commented Feb 12, 2025

Copy link
Copy Markdown
Contributor

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
HappenLee marked this pull request as ready for review February 12, 2025 14:11
@HappenLee

HappenLee commented Feb 12, 2025

Copy link
Copy Markdown
ContributorAuthor

run buildall

@doris-robot

doris-robot commented Feb 12, 2025

Copy link
Copy Markdown
TPC-H: Total hot run time: 31733 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 3b67c79622a6d40eb5c1b53f73e6edd92153bce5, data reload: false
------ Round 1 ----------------------------------
q1	17595	5195	5130	5130
q2	2057	321	169	169
q3	10388	1255	771	771
q4	10206	1025	553	553
q5	7512	2400	2371	2371
q6	192	168	137	137
q7	903	756	607	607
q8	9293	1312	1173	1173
q9	4889	4601	4682	4601
q10	6816	2328	1893	1893
q11	493	283	250	250
q12	348	358	221	221
q13	17764	3678	3144	3144
q14	235	221	212	212
q15	509	463	468	463
q16	621	624	597	597
q17	585	868	350	350
q18	6632	6379	6102	6102
q19	1441	948	555	555
q20	322	334	186	186
q21	2823	2189	1941	1941
q22	369	340	307	307
Total cold run time: 101993 ms
Total hot run time: 31733 ms
----- Round 2, with runtime_filter_mode=off -----
q1	5139	5157	5154	5154
q2	238	319	226	226
q3	2151	2705	2326	2326
q4	1413	1828	1401	1401
q5	4214	4144	4164	4144
q6	208	166	123	123
q7	1851	1826	1658	1658
q8	2631	2622	2572	2572
q9	7215	7159	7100	7100
q10	3015	3242	2775	2775
q11	583	522	493	493
q12	688	782	624	624
q13	3579	3956	3265	3265
q14	282	292	274	274
q15	502	450	468	450
q16	638	694	651	651
q17	1153	1562	1391	1391
q18	7567	7428	7328	7328
q19	821	902	1069	902
q20	1982	2038	1886	1886
q21	5553	4997	4820	4820
q22	615	613	541	541
Total cold run time: 52038 ms
Total hot run time: 50104 ms

@doris-robot

doris-robot commented Feb 12, 2025

Copy link
Copy Markdown
TPC-DS: Total hot run time: 190468 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 3b67c79622a6d40eb5c1b53f73e6edd92153bce5, data reload: false
query1	1301	944	932	932
query2	6293	1839	1843	1839
query3	11055	4640	4388	4388
query4	54542	25624	23360	23360
query5	5234	547	470	470
query6	356	196	195	195
query7	4950	525	294	294
query8	311	239	235	235
query9	5925	2543	2546	2543
query10	423	321	242	242
query11	15239	15077	14921	14921
query12	162	102	103	102
query13	1069	516	398	398
query14	10540	6556	6594	6556
query15	204	212	177	177
query16	6988	688	477	477
query17	1064	723	580	580
query18	1521	438	333	333
query19	203	189	169	169
query20	131	122	125	122
query21	206	128	114	114
query22	4542	4505	4391	4391
query23	34195	33434	33494	33434
query24	5812	2431	2413	2413
query25	504	525	393	393
query26	711	284	151	151
query27	1871	504	338	338
query28	2780	2459	2395	2395
query29	567	558	461	461
query30	210	193	157	157
query31	907	839	804	804
query32	81	62	58	58
query33	455	357	291	291
query34	781	879	503	503
query35	801	849	764	764
query36	981	993	923	923
query37	125	114	73	73
query38	4216	4471	4298	4298
query39	1502	1442	1416	1416
query40	200	117	102	102
query41	51	48	48	48
query42	122	109	105	105
query43	490	515	487	487
query44	1332	822	808	808
query45	186	173	177	173
query46	907	1106	660	660
query47	1889	1887	1821	1821
query48	384	415	339	339
query49	731	505	436	436
query50	789	779	423	423
query51	4244	4320	4271	4271
query52	118	114	94	94
query53	236	262	186	186
query54	474	484	416	416
query55	83	78	80	78
query56	271	278	254	254
query57	1182	1229	1143	1143
query58	248	241	237	237
query59	2672	2803	2728	2728
query60	279	276	268	268
query61	116	115	118	115
query62	755	771	691	691
query63	242	194	189	189
query64	1740	1047	660	660
query65	3245	3151	3104	3104
query66	730	446	294	294
query67	16001	15601	15554	15554
query68	5442	782	499	499
query69	526	301	261	261
query70	1260	1131	1044	1044
query71	429	306	250	250
query72	6323	3809	3813	3809
query73	1285	781	345	345
query74	8914	9152	8830	8830
query75	3200	3133	2686	2686
query76	3848	1223	763	763
query77	535	397	277	277
query78	10045	10381	9290	9290
query79	1836	838	582	582
query80	671	561	440	440
query81	495	279	244	244
query82	278	133	100	100
query83	173	174	155	155
query84	283	93	75	75
query85	731	371	364	364
query86	337	302	278	278
query87	4395	4576	4346	4346
query88	2830	2185	2161	2161
query89	394	322	283	283
query90	1671	189	190	189
query91	133	139	115	115
query92	66	59	58	58
query93	2032	1051	634	634
query94	627	434	303	303
query95	343	274	254	254
query96	479	596	266	266
query97	2816	2888	2721	2721
query98	224	203	201	201
query99	1307	1414	1251	1251
Total cold run time: 292403 ms
Total hot run time: 190468 ms

@doris-robot

doris-robot commented Feb 12, 2025

Copy link
Copy Markdown
ClickBench: Total hot run time: 30.66 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 3b67c79622a6d40eb5c1b53f73e6edd92153bce5, data reload: false
query1	0.04	0.03	0.03
query2	0.09	0.03	0.03
query3	0.24	0.06	0.07
query4	1.61	0.10	0.10
query5	0.42	0.43	0.40
query6	1.17	0.67	0.66
query7	0.02	0.02	0.02
query8	0.04	0.04	0.04
query9	0.60	0.54	0.52
query10	0.58	0.58	0.57
query11	0.15	0.10	0.11
query12	0.14	0.12	0.12
query13	0.61	0.59	0.61
query14	2.81	2.68	2.70
query15	0.92	0.85	0.85
query16	0.39	0.37	0.39
query17	1.07	1.01	1.04
query18	0.21	0.21	0.20
query19	1.94	1.83	2.01
query20	0.01	0.01	0.01
query21	15.65	0.90	0.55
query22	0.91	1.14	0.65
query23	15.12	1.38	0.64
query24	7.35	1.01	0.72
query25	0.45	0.18	0.18
query26	0.75	0.17	0.14
query27	0.05	0.05	0.05
query28	9.25	0.88	0.43
query29	12.55	4.07	3.34
query30	0.25	0.09	0.06
query31	2.83	0.59	0.39
query32	3.22	0.55	0.47
query33	3.00	2.99	3.10
query34	15.71	5.16	4.53
query35	4.57	4.53	4.53
query36	0.66	0.49	0.48
query37	0.10	0.06	0.06
query38	0.05	0.05	0.04
query39	0.03	0.03	0.03
query40	0.16	0.12	0.12
query41	0.08	0.02	0.02
query42	0.03	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 105.86 s
Total hot run time: 30.66 s

@doris-robot

doris-robot commented Feb 12, 2025

Copy link
Copy Markdown

TeamCity be ut coverage result:
Function Coverage: 42.98% (11310/26312)
Line Coverage: 32.92% (94980/288558)
Region Coverage: 32.07% (48686/151829)
Branch Coverage: 27.92% (24556/87946)
Coverage Report: http://coverage.selectdb-in.cc/coverage/3b67c79622a6d40eb5c1b53f73e6edd92153bce5_3b67c79622a6d40eb5c1b53f73e6edd92153bce5/report/index.html

@github-actions

github-actionsBot commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

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

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Feb 13, 2025
@github-actions

github-actionsBot commented Feb 13, 2025

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@HappenLee
HappenLee merged commit 4d42e33 into apache:masterFeb 13, 2025
HappenLee added a commit to HappenLee/incubator-doris that referenced this pull request Feb 14, 2025
@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
@HappenLeeHappenLee mentioned this pull request Feb 14, 2025
Merged
@HappenLeeHappenLee added the usercase Important user case type label label Feb 14, 2025
dataroaring pushed a commit that referenced this pull request Feb 14, 2025
@HappenLee
[Bug](fix) Fix topn agg limit may get error result in when refresh he…
95f9b32
…ap (#47844) (#47903)
cherry pick #47844
@gavinchougavinchou mentioned this pull request Feb 18, 2025
Closed
lzyy2024 pushed a commit to lzyy2024/doris that referenced this pull request Feb 21, 2025
@HappenLee@lzyy2024
[Bug](fix) Fix topn agg limit may get error result in when refresh he…
04d8fed
…ap (apache#47844)
1. heap refresh should emplace before pop
2. Add UT for the heap
3. refactor the refresh to AggSharedState
koarz pushed a commit to koarz/doris that referenced this pull request Jun 4, 2025
@HappenLeekoarz
[Bug](fix) Fix topn agg limit may get error result in when refresh he…
cb489f5
…ap (apache#47844)
1. heap refresh should emplace before pop
2. Add UT for the heap
3. refactor the refresh to AggSharedState
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BiteTheDDDDtBiteTheDDDDtBiteTheDDDDt approved these changes
@zhangstar333zhangstar333zhangstar333 approved these changes

Assignees

No one assigned

Labels

approvedIndicates a PR has been approved by one committer.dev/3.0.4-mergedp0_wreviewedusercaseImportant user case type label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@HappenLee@Thearas@doris-robot@BiteTheDDDDt@zhangstar333@dataroaring