Skip to content

Conversation

@Jackie-Jiang
Copy link
Contributor

Summary

This PR fixes the thread accountant setup in LeafOperator to properly handle resource accounting for multi-stage query execution.

Changes

  • Changed from setupWorker() to setupRunner() for thread accountant initialization
  • Added proper cleanup with Tracing.ThreadAccountantOps.clear() in finally blocks
  • Treats LeafOperator threads as SSE runner anchor threads instead of worker threads
  • Ensures consistent thread accounting for both single table and dual table scenarios

Technical Details

The fix addresses thread accounting issues by:

  1. Using setupRunner with queryId and workloadName instead of setupWorker with taskId
  2. Adding proper resource cleanup in finally blocks to prevent resource leaks
  3. Ensuring consistent behavior between single-request and multi-request execution paths

Files Changed

  • pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/LeafOperator.java

Testing

  • Code compiles successfully
  • Existing tests pass
  • Manual testing of multi-stage queries

This change improves resource management and prevents potential memory/thread accounting issues in multi-stage query execution.

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 fixes thread accounting issues in the LeafOperator by changing how thread accountants are initialized and cleaned up for multi-stage query execution. The fix ensures proper resource management by treating LeafOperator threads as SSE runner anchor threads instead of worker threads.

Key changes:

  • Replace setupWorker() calls with setupRunner() for proper thread accountant initialization
  • Add cleanup calls with Tracing.ThreadAccountantOps.clear() in finally blocks
  • Ensure consistent thread accounting behavior for both single and dual table execution scenarios

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Jackie-Jiang Jackie-Jiang force-pushed the fix_leaf_operator_accountant branch from 7a2fd5d to 2001f84 Compare August 21, 2025 00:57
@codecov-commenter
Copy link

codecov-commenter commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 42.85714% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.31%. Comparing base (5799642) to head (2001f84).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...che/pinot/query/runtime/operator/LeafOperator.java 42.85% 16 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #16651      +/-   ##
============================================
+ Coverage     63.29%   63.31%   +0.02%     
  Complexity     1379     1379              
============================================
  Files          3033     3033              
  Lines        176632   176649      +17     
  Branches      27113    27113              
============================================
+ Hits         111796   111843      +47     
+ Misses        56241    56230      -11     
+ Partials       8595     8576      -19     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 63.27% <42.85%> (+0.02%) ⬆️
java-21 63.26% <42.85%> (+<0.01%) ⬆️
temurin 63.31% <42.85%> (+0.02%) ⬆️
unittests 63.31% <42.85%> (+0.02%) ⬆️
unittests1 56.44% <42.85%> (+0.04%) ⬆️
unittests2 33.11% <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.

@Jackie-Jiang Jackie-Jiang merged commit 633c90d into apache:master Aug 22, 2025
32 of 36 checks passed
@Jackie-Jiang Jackie-Jiang deleted the fix_leaf_operator_accountant branch August 22, 2025 18:10
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