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](load) skip cancel already cancelled channels #27111

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

kaijchen
Copy link
Contributor

Proposed changes

Fix NPE when cancelling already cancelled channels.

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@kaijchen
Copy link
Contributor Author

run buildall

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.7 seconds
stream load tsv: 568 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 18 seconds loaded 2358488459 Bytes, about 124 MB/s
stream load orc: 65 seconds loaded 1101869774 Bytes, about 16 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 28.6 seconds inserted 10000000 Rows, about 349K ops/s
storage size: 17097712296 Bytes

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
Tpch sf100 test result on commit 1beff08df704c2962a19da78090bdb32285da417, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4926	4662	4685	4662
q2	369	159	158	158
q3	2060	1944	1891	1891
q4	1383	1232	1252	1232
q5	3917	3952	4049	3952
q6	269	132	129	129
q7	1386	865	869	865
q8	2784	2783	2773	2773
q9	9890	9560	9589	9560
q10	3465	3510	3544	3510
q11	364	256	247	247
q12	439	295	292	292
q13	4587	3810	3826	3810
q14	318	291	297	291
q15	583	537	514	514
q16	661	582	576	576
q17	1139	968	937	937
q18	7853	7317	7409	7317
q19	1679	1659	1701	1659
q20	546	301	316	301
q21	4386	3965	3997	3965
q22	481	377	372	372
Total cold run time: 53485 ms
Total hot run time: 49013 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4575	4559	4559	4559
q2	337	224	252	224
q3	4022	4007	4017	4007
q4	2716	2705	2692	2692
q5	9631	9565	9630	9565
q6	244	126	123	123
q7	2637	2250	2260	2250
q8	4404	4405	4400	4400
q9	13214	13172	13043	13043
q10	4095	4176	4154	4154
q11	782	680	709	680
q12	981	826	813	813
q13	4283	3591	3587	3587
q14	387	349	353	349
q15	570	522	521	521
q16	730	660	693	660
q17	3972	3857	3859	3857
q18	9523	9115	9111	9111
q19	1806	1778	1775	1775
q20	2387	2054	2033	2033
q21	8806	8545	8786	8545
q22	883	827	842	827
Total cold run time: 80985 ms
Total hot run time: 77775 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.75% (8409/22883)
Line Coverage: 29.27% (68394/233644)
Region Coverage: 27.87% (35353/126871)
Branch Coverage: 24.62% (18061/73358)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1beff08df704c2962a19da78090bdb32285da417_1beff08df704c2962a19da78090bdb32285da417/report/index.html

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 Nov 16, 2023
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.

Copy link
Contributor

@sollhui sollhui 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 afffcfd into apache:master Nov 16, 2023
27 of 29 checks passed
@kaijchen kaijchen deleted the skip-cancelled branch November 17, 2023 12:36
@xiaokang xiaokang added the p0_b label Nov 22, 2023
seawinde pushed a commit to seawinde/doris that referenced this pull request Nov 28, 2023
@xiaokang xiaokang mentioned this pull request Dec 4, 2023
XuJianxu pushed a commit to XuJianxu/doris that referenced this pull request Dec 14, 2023
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.0.3-merged p0_b reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants