Skip to content

Conversation

@praveenc7
Copy link
Contributor

@praveenc7 praveenc7 commented Oct 31, 2025

Summary

  1. Add support for custom class implementation using reflections to avoid creating a custom implementation of the entire starter class. This follows the convention we generally use for some class in pinot-minion
  2. Removing WorkloadBudgetExceeded Error code since it is no longer used and instead SERVER_RESOURCE_LIMIT_EXCEEDED is generalized to be used instead also WorkloadBudgetExceeded conflicts TOO_MANY_REQUESTS error-code leading to issues on broker testing side
  3. Fetch workload budgets on startup - Brokers and servers now fetch their assigned workload budgets from the controller during initialization, ensuring budgets are available immediately after restart
  4. Broker debug API - Added /debug/queryWorkloadCost endpoints to brokers (matching existing server APIs) for runtime workload inspection
  5. Use IdealState for broker resolution - Controller now uses IdealState instead of ExternalView when propagating workload configs, ensuring offline brokers (e.g., during rolling restart) receive updates
  6. canAdmitQuery should check whether enforcement is enabled—similar to how WorkloadResourceAggregator does—instead of relying on costCollection. This ensures correct behavior in scenarios where cost collection is active but enforcement is intentionally disabled for workload analysis or monitoring purposes.
  7. Foundational metrics to track basic availability for workload

Testing Done

Unit test and Integ test

…on (apache#70)

* Interface

* Interface framework

* Interface framework
* startup budget

* startup

* Integ test

* review comments

* Fix canAdmitQuery

* review comments on debug api
* workload metric

* review comments
@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2025

Codecov Report

❌ Patch coverage is 38.92617% with 182 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.20%. Comparing base (9a5a5cb) to head (8c492c1).
⚠️ Report is 57 commits behind head on master.

Files with missing lines Patch % Lines
...t/spi/accounting/DefaultWorkloadBudgetManager.java 46.53% 45 Missing and 9 partials ⚠️
.../common/utils/config/QueryWorkloadConfigUtils.java 36.98% 35 Missing and 11 partials ⚠️
...apache/pinot/common/utils/WorkloadBudgetUtils.java 0.00% 38 Missing ⚠️
...ot/core/accounting/WorkloadResourceAggregator.java 0.00% 13 Missing ⚠️
...t/spi/accounting/WorkloadBudgetManagerFactory.java 64.51% 6 Missing and 5 partials ⚠️
...che/pinot/core/query/logger/ServerQueryLogger.java 0.00% 5 Missing ⚠️
.../pinot/core/query/scheduler/WorkloadScheduler.java 25.00% 3 Missing ⚠️
...e/pinot/broker/api/resources/PinotBrokerDebug.java 0.00% 2 Missing ⚠️
.../helix/BrokerUserDefinedMessageHandlerFactory.java 0.00% 2 Missing ⚠️
...ache/pinot/server/api/resources/DebugResource.java 0.00% 2 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #17124      +/-   ##
============================================
+ Coverage     63.17%   63.20%   +0.03%     
- Complexity     1428     1429       +1     
============================================
  Files          3104     3107       +3     
  Lines        183488   183594     +106     
  Branches      28127    28141      +14     
============================================
+ Hits         115911   116049     +138     
+ Misses        58611    58561      -50     
- Partials       8966     8984      +18     
Flag Coverage Δ
custom-integration1 100.00% <ø> (?)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 ?
java-11 63.18% <38.92%> (+0.03%) ⬆️
java-21 63.18% <38.92%> (+0.04%) ⬆️
temurin 63.20% <38.92%> (+0.03%) ⬆️
unittests 63.20% <38.92%> (+0.03%) ⬆️
unittests1 56.17% <28.46%> (-0.01%) ⬇️
unittests2 33.62% <16.10%> (+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.

@praveenc7 praveenc7 marked this pull request as ready for review November 1, 2025 22:17
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