You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pinot.apache.org by "yashmayya (via GitHub)" <gi...@apache.org> on 2026/05/15 20:51:59 UTC

[PR] Add broker config to enable streaming group-by by default for a cluster [pinot]

yashmayya opened a new pull request, #18510:
URL: https://github.com/apache/pinot/pull/18510

## 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 #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
- [x] 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.
- [x] `./mvnw spotless:apply checkstyle:check license:check -pl pinot-spi,pinot-broker` clean.
- [x] `./mvnw -pl pinot-broker -Dtest=MultiStageBrokerRequestHandlerTest test` — 4/4 passing.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Add broker config to enable streaming group-by by default for a cluster [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.

xiangfu0 commented on PR #18510:
URL: https://github.com/apache/pinot/pull/18510#issuecomment-4510184314

   Documented the new broker default here: https://github.com/pinot-contrib/pinot-docs/pull/815


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Add broker config to enable streaming group-by by default for a cluster [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.

xiangfu0 commented on PR #18510:
URL: https://github.com/apache/pinot/pull/18510#issuecomment-4510171104

   Docs PR opened: https://github.com/pinot-contrib/pinot-docs/pull/812


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Add broker config to enable streaming group-by by default for a cluster [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.

Jackie-Jiang commented on code in PR #18510:
URL: https://github.com/apache/pinot/pull/18510#discussion_r3277881319


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -507,6 +507,16 @@ public static class Broker {
       public static final String CONFIG_OF_IGNORE_MISSING_SEGMENTS =
           "pinot.broker.query.ignore.missing.segments";
       public static final boolean DEFAULT_IGNORE_MISSING_SEGMENTS = false;
+
+    /**
+     * Default flush threshold for the streaming group-by leaf-stage operator on MSE. When {@code > 0}, the broker
+     * injects this value as the {@link Request.QueryOptionKey#STREAMING_GROUP_BY_FLUSH_THRESHOLD} query option for
+     * MSE queries that do not already specify it, opting the cluster into the streaming group-by behavior by default.
+     * Setting the query option explicitly (including to {@code 0} to disable) always wins over the broker default.
+     */
+    public static final String CONFIG_OF_MSE_STREAMING_GROUP_BY_FLUSH_THRESHOLD =
+        "pinot.broker.mse.streaming.group.by.flush.threshold";
+    public static final int DEFAULT_MSE_STREAMING_GROUP_BY_FLUSH_THRESHOLD = 0;

Review Comment:
   `-1` is usually more clear that it is disabled



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -507,6 +507,16 @@ public static class Broker {
       public static final String CONFIG_OF_IGNORE_MISSING_SEGMENTS =
           "pinot.broker.query.ignore.missing.segments";
       public static final boolean DEFAULT_IGNORE_MISSING_SEGMENTS = false;
+
+    /**

Review Comment:
   (minor) Markdown style is more readable (no need of `@code` or `@link`)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Add broker config to enable streaming group-by by default for a cluster [pinot]

Posted by "yashmayya (via GitHub)" <gi...@apache.org>.

yashmayya merged PR #18510:
URL: https://github.com/apache/pinot/pull/18510


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

Re: [PR] Add broker config to enable streaming group-by by default for a cluster [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.

codecov-commenter commented on PR #18510:
URL: https://github.com/apache/pinot/pull/18510#issuecomment-4464215629

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18510?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 55.79%. Comparing base ([`51bcec3`](https://app.codecov.io/gh/apache/pinot/commit/51bcec3b88fe151362ef79bdea1b26515e6ff3ca?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`adcc9ea`](https://app.codecov.io/gh/apache/pinot/commit/adcc9eaffb146cfba40de91cce2f7641803325bb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 3 commits behind head on master.
   > :exclamation:  There is a different number of reports uploaded between BASE (51bcec3) and HEAD (adcc9ea). Click for more details.
   > 
   > <details><summary>HEAD has 16 uploads less than BASE</summary>
   >
   >| Flag | BASE (51bcec3) | HEAD (adcc9ea) |
   >|------|------|------|
   >|java-21|5|1|
   >|unittests|2|1|
   >|temurin|5|1|
   >|unittests2|1|0|
   >|integration|3|0|
   >|integration1|1|0|
   >|integration2|1|0|
   >|custom-integration1|1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #18510      +/-   ##
   ============================================
   - Coverage     63.66%   55.79%   -7.87%     
   + Complexity     1684      815     -869     
   ============================================
     Files          3266     2555     -711     
     Lines        199903   148378   -51525     
     Branches      31061    23995    -7066     
   ============================================
   - Hits         127272    82793   -44479     
   + Misses        62466    58487    -3979     
   + Partials      10165     7098    -3067     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [java-21](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.79% <ø> (-7.87%)` | :arrow_down: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.79% <ø> (-7.87%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.79% <ø> (-7.87%)` | :arrow_down: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.79% <ø> (+<0.01%)` | :arrow_up: |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18510/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18510?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org