Skip to content

[fix](hash join) fix stack overflow caused by evaluate case expr on huge build block #28823

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
Closed
jacktengg wants to merge 2 commits into from
Closed

[fix](hash join) fix stack overflow caused by evaluate case expr on huge build block #28823

jacktengg wants to merge 2 commits into from

Conversation

@jacktengg
Copy link
Contributor

@jacktengg jacktengg commented Dec 21, 2023 •
edited
Loading

Proposed changes

Issue Number: close #xxx

Hash join operator will merge build blocks to a big block with a lot rows, and using variable length array in expr execution may cause stack overflow.

Also add -Werror=vla compiler option to report compile error by default and

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wvla"
#pragma clang diagnostic pop

to ignore VLA usages where it's sure not to cause stack overflow.

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...

@jacktengg
Copy link
Contributor Author

jacktengg commented Dec 21, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2023

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

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2023

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

@jacktengg
Copy link
Contributor Author

jacktengg commented Dec 21, 2023

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2023

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

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Dec 21, 2023

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

@doris-robot
Copy link

doris-robot commented Dec 21, 2023

TeamCity be ut coverage result:
Function Coverage: 36.48% (8539/23405)
Line Coverage: 28.61% (69564/243116)
Region Coverage: 27.62% (35935/130111)
Branch Coverage: 24.35% (18361/75398)
Coverage Report: http://coverage.selectdb-in.cc/coverage/6e0f8b06fa207dafe857b0b004cb0d24ae3fa23a_6e0f8b06fa207dafe857b0b004cb0d24ae3fa23a/report/index.html

@doris-robot
Copy link

doris-robot commented Dec 21, 2023

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Tpch sf100 test result on commit 6e0f8b06fa207dafe857b0b004cb0d24ae3fa23a, data reload: false

run tpch-sf100 query with default conf and session variables
q1	4673	4400	4431	4400
q2	369	147	157	147
q3	1446	1270	1226	1226
q4	1120	978	918	918
q5	3147	3173	3177	3173
q6	243	126	127	126
q7	1005	480	490	480
q8	2308	2227	2180	2180
q9	6680	6673	6661	6661
q10	3213	3249	3261	3249
q11	307	185	185	185
q12	349	218	210	210
q13	4572	3842	3817	3817
q14	237	209	221	209
q15	555	522	519	519
q16	451	382	391	382
q17	1013	557	503	503
q18	7260	6982	7040	6982
q19	1531	1271	1459	1271
q20	527	306	312	306
q21	3034	2682	2668	2668
q22	353	287	287	287
Total cold run time: 44393 ms
Total hot run time: 39899 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	4370	4347	4347	4347
q2	266	162	174	162
q3	3557	3545	3550	3545
q4	2386	2389	2382	2382
q5	5748	5736	5728	5728
q6	243	120	122	120
q7	2380	1861	1868	1861
q8	3523	3524	3538	3524
q9	9058	9050	8984	8984
q10	3906	4007	3988	3988
q11	487	386	369	369
q12	764	592	598	592
q13	4301	3573	3591	3573
q14	284	262	259	259
q15	565	512	517	512
q16	505	502	472	472
q17	1879	1855	1840	1840
q18	8766	8381	8463	8381
q19	1749	1736	1754	1736
q20	2253	1932	1935	1932
q21	6551	6199	6192	6192
q22	513	435	428	428
Total cold run time: 64054 ms
Total hot run time: 60927 ms

@doris-robot
Copy link

doris-robot commented Dec 21, 2023

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 44.62 seconds
stream load tsv: 582 seconds loaded 74807831229 Bytes, about 122 MB/s
stream load json: 19 seconds loaded 2358488459 Bytes, about 118 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 33 seconds loaded 861443392 Bytes, about 24 MB/s
insert into select: 28.2 seconds inserted 10000000 Rows, about 354K ops/s
storage size: 17184023723 Bytes

This was referenced Dec 21, 2023
Closed
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Jun 19, 2024

We're closing this PR because it hasn't been updated in a while.
This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and feel free a maintainer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Jun 19, 2024
@github-actions github-actions bot closed this Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jacktengg @doris-robot