Skip to content

Use alarming sleep time for CRITICAL and PANIC level#17788

Open
Jackie-Jiang wants to merge 1 commit intoapache:masterfrom
Jackie-Jiang:fix_oom_sleep_time
Open

Use alarming sleep time for CRITICAL and PANIC level#17788
Jackie-Jiang wants to merge 1 commit intoapache:masterfrom
Jackie-Jiang:fix_oom_sleep_time

Conversation

@Jackie-Jiang
Copy link
Contributor

Check more frequently for CRITICAL and PANIC level

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the query accounting trigger evaluation so that when heap usage reaches CRITICAL or PANIC levels, the system also switches to the more frequent alarming sleep time, enabling faster follow-up checks during severe memory pressure.

Changes:

  • Apply alarmingSleepTime when heap usage exceeds the alarming threshold, including when it is also CRITICAL/PANIC.
  • Restructure trigger evaluation logic in both the global (QueryResourceAggregator) and per-query (PerQueryCPUMemAccountantFactory) accounting paths.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
pinot-core/src/main/java/org/apache/pinot/core/accounting/QueryResourceAggregator.java Restructures heap trigger evaluation so CRITICAL/PANIC also use alarming sleep time.
pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java Mirrors the same trigger/sleep-time restructuring for the per-query accountant.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

@Jackie-Jiang Jackie-Jiang force-pushed the fix_oom_sleep_time branch 2 times, most recently from e544d82 to 38acca5 Compare March 1, 2026 03:10
@Jackie-Jiang Jackie-Jiang requested a review from Copilot March 1, 2026 03:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

@codecov-commenter
Copy link

codecov-commenter commented Mar 1, 2026

Codecov Report

❌ Patch coverage is 0% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.22%. Comparing base (337faad) to head (e32c9e2).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...re/accounting/PerQueryCPUMemAccountantFactory.java 0.00% 10 Missing ⚠️
...pinot/core/accounting/QueryResourceAggregator.java 0.00% 10 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #17788      +/-   ##
============================================
- Coverage     63.23%   63.22%   -0.01%     
  Complexity     1454     1454              
============================================
  Files          3183     3183              
  Lines        191430   191512      +82     
  Branches      29274    29291      +17     
============================================
+ Hits         121047   121083      +36     
- Misses        60943    60964      +21     
- Partials       9440     9465      +25     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.17% <0.00%> (-0.03%) ⬇️
java-21 63.19% <0.00%> (-0.01%) ⬇️
temurin 63.22% <0.00%> (-0.01%) ⬇️
unittests 63.22% <0.00%> (-0.01%) ⬇️
unittests1 55.59% <0.00%> (-0.03%) ⬇️
unittests2 34.12% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants