Skip to content

[Enhancement] ignore the exec_mem_limit in pipeline execution #34120

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
merged 7 commits into from
Dec 15, 2023
Merged

[Enhancement] ignore the exec_mem_limit in pipeline execution #34120

merged 7 commits into from
Dec 15, 2023

Conversation

murphyatwork
Copy link
Contributor

@murphyatwork murphyatwork commented Nov 1, 2023 •
edited
Loading

Fixes #issue

  • Previously
    • Use query_mem_limit or exec_mem_limit if any is positive
    • Use exec_mem_limit for many jobs, like backup/streamload/routineload
  • Now
    • Only consider query_mem_limit in pipeline engine
    • For query_mem_limit, it means no limit if 0, otherwise it means a hard limitation
  • Behavior Change
    • Default behavior is changed, previous has a exec_mem_limit, now there's no limitation

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.2
    • 3.1
    • 3.0
    • 2.5

@murphyatwork
Copy link
Contributor Author

@Mergifyio rebase

@mergify Mergify
Copy link
Contributor

mergify bot commented Nov 3, 2023

rebase

✅ Branch has been successfully rebased

@murphyatwork murphyatwork force-pushed the murphy_ref_exec_mem_limit branch from 11fddc4 to 1839022 Compare November 3, 2023 08:37
@murphyatwork murphyatwork force-pushed the murphy_ref_exec_mem_limit branch from 1839022 to b16bc0e Compare November 28, 2023 07:04
@github-actions github-actions bot removed the 3.0 label Dec 13, 2023
satanson
satanson previously approved these changes Dec 14, 2023
View reviewed changes
trueeyu
trueeyu previously approved these changes Dec 14, 2023
View reviewed changes
@murphyatwork murphyatwork dismissed stale reviews from trueeyu and satanson via 29d73cf December 14, 2023 08:30
@murphyatwork murphyatwork force-pushed the murphy_ref_exec_mem_limit branch from 29d73cf to 8b54fd7 Compare December 14, 2023 10:32
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@github-actions GitHub Actions
Copy link

[FE Incremental Coverage Report]

✅ pass : 1 / 1 (100.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/qe/SessionVariable.java 1 1 100.00% []

@github-actions GitHub Actions
Copy link

[BE Incremental Coverage Report]

❌ fail : 0 / 5 (00.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 src/exec/pipeline/fragment_executor.cpp 0 5 00.00% [232, 233, 236, 237, 239]

@murphyatwork murphyatwork enabled auto-merge (squash) December 15, 2023 01:58
@murphyatwork murphyatwork merged commit 915d762 into StarRocks:main Dec 15, 2023
@github-actions GitHub Actions
Copy link

@Mergifyio backport branch-3.2

@github-actions github-actions bot removed the 3.2 label Dec 15, 2023
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 15, 2023 •
edited
Loading

backport branch-3.2

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Dec 15, 2023
@murphyatwork @mergify
[Enhancement] ignore the exec_mem_limit in pipeline execution (#34120)
2ba384a 
Signed-off-by: Murphy <[email protected]>
Signed-off-by: Murphy <[email protected]>
(cherry picked from commit 915d762)

# Conflicts:
#	be/src/exec/pipeline/fragment_executor.cpp
#	be/src/exec/pipeline/query_context.cpp
#	be/test/exec/pipeline/query_context_test.cpp
@mergify mergify bot mentioned this pull request Dec 15, 2023
Merged
fzhedu
fzhedu reviewed Dec 15, 2023
View reviewed changes
fe/fe-core/src/main/java/com/starrocks/qe/SessionVariable.java
Comment on lines +100 to +101
* If `query_mem_limit` > 0, use it to limit the memory of a query.
* Otherwise, no limitation
Copy link
Contributor

Choose a reason for hiding this comment

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

if set query_mem_limit = 1, can a query run?

@murphyatwork murphyatwork mentioned this pull request Dec 21, 2023
Merged
23 tasks
@wanpengfei-git wanpengfei-git mentioned this pull request Dec 22, 2023
Closed
18 tasks
murphyatwork added a commit that referenced this pull request Dec 22, 2023
@murphyatwork
[Enhancement] ignore the exec_mem_limit in pipeline execution (#34120)
2a9edb0 
Signed-off-by: Murphy <[email protected]>
Signed-off-by: Murphy <[email protected]>
(cherry picked from commit 915d762)
murphyatwork added a commit that referenced this pull request Dec 22, 2023
@murphyatwork
[Enhancement] ignore the exec_mem_limit in pipeline execution (#34120)
1104aa5 
Signed-off-by: Murphy <[email protected]>
Signed-off-by: Murphy <[email protected]>
(cherry picked from commit 915d762)
wanpengfei-git pushed a commit that referenced this pull request Dec 22, 2023
@mergify @murphyatwork
[Enhancement] ignore the exec_mem_limit in pipeline execution (backport
ed3a147 
#34120) (backport #37517) (#37096)

Co-authored-by: Murphy <[email protected]>
@ZiheLiu ZiheLiu mentioned this pull request Dec 25, 2023
Merged
23 tasks
Zhangg7723 pushed a commit to Zhangg7723/starrocks that referenced this pull request Dec 26, 2023
@murphyatwork @Zhangg7723
[Enhancement] ignore the exec_mem_limit in pipeline execution (StarRo…
b8b0cb8 
…cks#34120)

Signed-off-by: Murphy <[email protected]>
Signed-off-by: Murphy <[email protected]>
Signed-off-by: 张敢 <[email protected]>
stdpain added a commit to stdpain/starrocks-2 that referenced this pull request Mar 18, 2024
@stdpain
[BugFix] Fix streaming aggregate always force streaming in when query…
8b5c099 
… memory limit is 0

in PR StarRocks#34120 we change default query_memory_limit to -1, which will make
streaming aggregate change to limited memory mode.

Signed-off-by: stdpain <[email protected]>
@stdpain stdpain mentioned this pull request Mar 18, 2024
Merged
23 tasks
@mergify mergify bot mentioned this pull request Mar 18, 2024
Merged
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fzhedu fzhedu fzhedu left review comments

@silverbullet233 silverbullet233 silverbullet233 approved these changes

@trueeyu trueeyu trueeyu approved these changes

@satanson satanson satanson left review comments

Assignees

@murphyatwork murphyatwork

Labels
backport_ok behavior_changed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@murphyatwork @satanson @silverbullet233 @fzhedu @trueeyu @wangruin