Skip to content

Add broker config to enable streaming group-by by default for a cluster#18510

Open
yashmayya wants to merge 1 commit into
apache:masterfrom
yashmayya:streaming-group-by-broker-default
Open

Add broker config to enable streaming group-by by default for a cluster#18510
yashmayya wants to merge 1 commit into
apache:masterfrom
yashmayya:streaming-group-by-broker-default

Conversation

@yashmayya
Copy link
Copy Markdown
Contributor

Summary

Follow-up to #18035. Adds a broker-level config to opt a cluster into the MSE streaming group-by leaf-stage operator by default, while leaving per-query control to operators and users.

  • New broker config pinot.broker.mse.streaming.group.by.flush.threshold (default 0, disabled).
  • When set to a positive value, the broker injects it as the streamingGroupByFlushThreshold query option for MSE queries that don't already specify it.
  • Per-query SET streamingGroupByFlushThreshold = N always wins via putIfAbsent, including SET streamingGroupByFlushThreshold = 0 to disable streaming group-by for a specific query when the cluster default is non-zero.
  • Injection happens once after compilation but before the EXPLAIN/execute branch, so EXPLAIN PLAN FOR … reflects the same options the executed query would see.

Rollout / backwards compatibility

  • Default is 0 → no behavior change for existing deployments. Existing query option still works exactly as before.
  • Recommended starting value mirrors the PR Implement streaming group by leaf stage operator for multi-stage engine #18035 guidance (a few thousand groups); operators should tune based on observed leaf-stage cardinality.
  • Mixed-broker rollouts are safe: brokers without the config simply don't inject the option, and servers handle the option the same way either way.
  • The config takes effect on broker restart (resolved once in the constructor, like the other MSE broker defaults).

Test plan

  • New unit tests in MultiStageBrokerRequestHandlerTest covering: injection when option absent, per-query SET=0 / SET=N overriding the broker default, no injection when config unset.
  • ./mvnw spotless:apply checkstyle:check license:check -pl pinot-spi,pinot-broker clean.
  • ./mvnw -pl pinot-broker -Dtest=MultiStageBrokerRequestHandlerTest test — 4/4 passing.

@yashmayya yashmayya added configuration Config changes (addition/deletion/change in behavior) multi-stage Related to the multi-stage query engine labels May 15, 2026
@yashmayya yashmayya requested a review from Jackie-Jiang May 15, 2026 21:08
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 15, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 63.69%. Comparing base (51bcec3) to head (adcc9ea).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
...requesthandler/MultiStageBrokerRequestHandler.java 85.71% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18510      +/-   ##
============================================
+ Coverage     63.66%   63.69%   +0.02%     
  Complexity     1684     1684              
============================================
  Files          3266     3266              
  Lines        199903   199934      +31     
  Branches      31061    31069       +8     
============================================
+ Hits         127272   127349      +77     
+ Misses        62466    62423      -43     
+ Partials      10165    10162       -3     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 63.69% <85.71%> (+0.02%) ⬆️
temurin 63.69% <85.71%> (+0.02%) ⬆️
unittests 63.69% <85.71%> (+0.02%) ⬆️
unittests1 55.79% <ø> (+<0.01%) ⬆️
unittests2 34.97% <85.71%> (+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

configuration Config changes (addition/deletion/change in behavior) multi-stage Related to the multi-stage query engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants