Skip to content

Add config to set consumption rate limit at partition level#18904

Merged
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:partition_consumption_rate_limit
Jul 2, 2026
Merged

Add config to set consumption rate limit at partition level#18904
Jackie-Jiang merged 1 commit into
apache:masterfrom
Jackie-Jiang:partition_consumption_rate_limit

Conversation

@Jackie-Jiang

Copy link
Copy Markdown
Contributor

Summary

Add a new stream config partition.consumption.rate.limit to set the realtime consumption rate limit directly at partition level. Unlike the existing topic.consumption.rate.limit (which is divided by the topic's partition count), the partition level limit doesn't need to be reconfigured when the partition count changes.

  • RealtimeConsumptionRateManager.createRateLimiter checks the partition level limit first and uses it directly, skipping the partition count fetch (and its cache) entirely. When both are specified, partition level limit takes precedence. Topic level behavior is unchanged.
  • Both rate limit getters in StreamConfig now return primitive double (non-positive means not throttled) instead of Optional<Double>, consistent with other configs. CONSUMPTION_RATE_LIMIT_NOT_SPECIFIED is made public.
  • Cleanup: removed dead _groupId / getGroupId() from StreamConfig and the unused GROUP_ID (hlc.group.id, HLC legacy) and PARTITION_MSG_OFFSET_FACTORY_CLASS keys from StreamConfigProperties; reordered equals / hashCode / toString to match field declaration order and fixed two malformed toString labels (missing =, _offSet typo).

Note: getTopicConsumptionRateLimit() signature change and the removed dead members are binary-incompatible for third-party plugins compiled against pinot-spi. There are no callers in the Pinot codebase.

@Jackie-Jiang Jackie-Jiang added configuration Config changes (addition/deletion/change in behavior) ingestion Related to data ingestion pipeline cleanup Code cleanup or removal of dead code feature New functionality real-time Related to realtime table ingestion and serving labels Jul 1, 2026
@Jackie-Jiang Jackie-Jiang requested a review from Copilot July 1, 2026 22:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new realtime stream config (partition.consumption.rate.limit) to support setting consumption throttling directly at the partition level (so it doesn’t need recalculation when topic partition counts change), and updates the rate limiter creation path to prefer this partition-level limit over the existing topic-level limit.

Changes:

  • Add partition.consumption.rate.limit stream config key and plumb it through StreamConfig + RealtimeConsumptionRateManager.
  • Change StreamConfig consumption rate limit getters from Optional<Double> to primitive double (non-positive = disabled), and remove some legacy/dead stream config members.
  • Update RealtimeConsumptionRateManagerTest to cover the new precedence behavior.

Reviewed changes

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

File Description
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamConfigProperties.java Adds the new partition-level rate limit key; removes legacy constants.
pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamConfig.java Stores/returns partition-level + topic-level rate limits and updates public API surface (getter signature changes).
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeConsumptionRateManager.java Prefers partition-level limit and skips partition-count cache when present.
pinot-core/src/test/java/org/apache/pinot/core/data/manager/realtime/RealtimeConsumptionRateManagerTest.java Updates tests for the new rate limit precedence behavior.

@codecov-commenter

codecov-commenter commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.80%. Comparing base (91be61f) to head (dbe9437).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...java/org/apache/pinot/spi/stream/StreamConfig.java 87.50% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master   #18904   +/-   ##
=========================================
  Coverage     64.80%   64.80%           
  Complexity     1347     1347           
=========================================
  Files          3393     3393           
  Lines        211666   211676   +10     
  Branches      33305    33312    +7     
=========================================
+ Hits         137164   137182   +18     
+ Misses        63431    63419   -12     
- Partials      11071    11075    +4     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 64.80% <90.62%> (+<0.01%) ⬆️
temurin 64.80% <90.62%> (+<0.01%) ⬆️
unittests 64.80% <90.62%> (+<0.01%) ⬆️
unittests1 56.99% <90.62%> (+<0.01%) ⬆️
unittests2 37.16% <18.75%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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 7ee1b42 into apache:master Jul 2, 2026
10 of 11 checks passed
@Jackie-Jiang Jackie-Jiang deleted the partition_consumption_rate_limit branch July 2, 2026 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Code cleanup or removal of dead code configuration Config changes (addition/deletion/change in behavior) feature New functionality ingestion Related to data ingestion pipeline real-time Related to realtime table ingestion and serving

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants