Skip to content

Conversation

@J-HowHuang
Copy link
Collaborator

Description

Currently Pinot server uses the default Helix thread pool for state transition messages (appears as HelixTaskExecutor-message_handle_thread_<tid>). The main concern of this is that

  1. Every transition regardless of the states uses the same pool. New consuming segments (OFFLINE->ONLINE) could be blocked by operations on other tables (e.g. reset, rebalance) because they flood the message queue.
  2. It's not possible to manage the threads within the scope of Pinot. For example, change max threads.

Changes

  • Add a new interface StateTransitionThreadPoolManager in charge of constructing the thread pools for different state transition messages
  • Override getExecutorService methods in SegmentOnlineOfflineStateModelFactory to return the thread pools created by StateTransitionThreadPoolManager
  • By default the StateTransitionThreadPoolManager is null, which returns null in all the getExecutorService method calls. This would be no difference than using the default shared thread pool in Helix. The implementation of StateTransitionThreadPoolManager needs to be provided and set during pinot server start.

@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2026

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.30%. Comparing base (668bf02) to head (773e089).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...r/helix/SegmentOnlineOfflineStateModelFactory.java 0.00% 11 Missing ⚠️
.../pinot/server/starter/helix/BaseServerStarter.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #17453      +/-   ##
============================================
+ Coverage     63.28%   63.30%   +0.01%     
  Complexity     1474     1474              
============================================
  Files          3161     3161              
  Lines        188588   188600      +12     
  Branches      28857    28858       +1     
============================================
+ Hits         119345   119387      +42     
+ Misses        59992    59956      -36     
- Partials       9251     9257       +6     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (?)
java-11 63.25% <0.00%> (+<0.01%) ⬆️
java-21 63.26% <0.00%> (+7.68%) ⬆️
temurin 63.30% <0.00%> (+0.01%) ⬆️
unittests 63.29% <0.00%> (+0.01%) ⬆️
unittests1 55.61% <ø> (+<0.01%) ⬆️
unittests2 34.03% <0.00%> (+0.02%) ⬆️

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants