You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/03/27 05:03:29 UTC
[PR] [Phase 0] Group-by perf roadmap: JMH benchmarks + observability stats [pinot]
xiangfu0 opened a new pull request, #17996:
URL: https://github.com/apache/pinot/pull/17996
## Summary
This PR implements **Phase 0** of the multi-stage group-by performance roadmap. No query semantic changes are made — this is purely additive (benchmarks, stats, tests).
### What's included
**JMH benchmarks** (`pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkMSEGroupByKeyGen.java`)
Covers all key-generator code paths:
| Scenario | Generator used |
|---|---|
| `SINGLE_INT` | `OneIntKeyGroupIdGenerator` (Int2IntOpenHashMap) |
| `SINGLE_STRING` | `OneObjectKeyGroupIdGenerator` (Object2IntOpenHashMap) |
| `TWO_INT_KEYS` | `TwoKeysGroupIdGenerator` (Long2IntOpenHashMap, packed longs) |
| `FOUR_INT_KEYS` | `MultiKeysGroupIdGenerator` (Object2IntOpenHashMap<FixedIntArray>) |
| `SKEWED` | `OneIntKeyGroupIdGenerator` with Zipfian key distribution |
| `SORTED_COLUMN` | `OneIntKeyGroupIdGenerator` simulating sorted-column shortcut |
Two benchmark modes per scenario:
- `keyGenInsert` — build map from scratch (reset each invocation, measures insert throughput)
- `keyGenSteadyState` — warm map lookups only (measures lookup throughput at saturation)
Two result-materialization benchmarks:
- `materializeTopK` — ORDER BY sum DESC LIMIT 100 via min-heap PriorityQueue
- `materializeAll` — plain full-table truncation via hash-map iterator
**Observability stats** (zero overhead when not collected)
Added to `AggregateOperator.StatKey`:
- `ROWS_IN` (LONG) — total input rows processed by this stage
- `GENERATOR_TYPE` (STRING) — which `GroupIdGenerator` backend was selected
**Unit tests** (`pinot-query-runtime`)
- `GroupIdGeneratorTest` (21 tests) — correctness harness covering null handling, limit enforcement, iterator fidelity, and factory routing for all generator variants
- `MultistageGroupByCombineLimitTest` (9 tests) — top-K materialization tests including `testLeafTrimCausesIncorrectGlobalTopK`, a **regression test that documents the known open bug** where leaf-level LIMIT K trimming causes the intermediate stage to return the wrong global top-K
### What's NOT changed
- No query plan changes
- No wire protocol changes
- No behaviour changes for existing queries
- All new flags/stats are off or inert by default
### Running the benchmarks
```bash
# Build perf module
./mvnw install -pl pinot-perf -am -DskipTests
# Run key-gen benchmarks (all scenarios, ~5 min)
java -jar pinot-perf/target/benchmarks.jar BenchmarkMSEGroupByKeyGen \
-f 1 -wi 3 -i 5 -tu us
# Run only single-int scenario
java -jar pinot-perf/target/benchmarks.jar BenchmarkMSEGroupByKeyGen \
-f 1 -wi 3 -i 5 -tu us \
-p scenario=SINGLE_INT
```
### Test commands
```bash
./mvnw -pl pinot-query-runtime -Dtest=GroupIdGeneratorTest test
./mvnw -pl pinot-query-runtime -Dtest=MultistageGroupByCombineLimitTest test
```
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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] [Phase 0] Group-by perf roadmap: JMH benchmarks + observability stats [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17996:
URL: https://github.com/apache/pinot/pull/17996#issuecomment-4140370544
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17996?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `51.87110%` with `463 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.47%. Comparing base ([`428d055`](https://app.codecov.io/gh/apache/pinot/commit/428d0558edf996849c40d4026643555b1aaa1077?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`50dfa60`](https://app.codecov.io/gh/apache/pinot/commit/50dfa60c3cbf3a278aa230a8646cff4240da93ac?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 15 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/17996?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ore/data/manager/ingest/InsertRecoveryManager.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Fingest%2FInsertRecoveryManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvaW5nZXN0L0luc2VydFJlY292ZXJ5TWFuYWdlci5qYXZh) | 0.00% | [113 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...inot/core/query/executor/sql/SqlQueryExecutor.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fquery%2Fexecutor%2Fsql%2FSqlQueryExecutor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9zcWwvU3FsUXVlcnlFeGVjdXRvci5qYXZh) | 0.00% | [97 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...not/core/data/manager/ingest/InsertRowApplier.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Fingest%2FInsertRowApplier.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvaW5nZXN0L0luc2VydFJvd0FwcGxpZXIuamF2YQ==) | 0.00% | [66 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ava/org/apache/pinot/spi/ingest/InsertRequest.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Fingest%2FInsertRequest.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvaW5nZXN0L0luc2VydFJlcXVlc3QuamF2YQ==) | 44.06% | [29 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../pinot/core/data/manager/ingest/LocalShardLog.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Fingest%2FLocalShardLog.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvaW5nZXN0L0xvY2FsU2hhcmRMb2cuamF2YQ==) | 79.82% | [15 Missing and 8 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../pinot/sql/parsers/parser/SqlInsertIntoValues.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsql%2Fparsers%2Fparser%2FSqlInsertIntoValues.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9wYXJzZXIvU3FsSW5zZXJ0SW50b1ZhbHVlcy5qYXZh) | 33.33% | [22 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...apache/pinot/sql/parsers/dml/InsertIntoValues.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsql%2Fparsers%2Fdml%2FInsertIntoValues.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvcGFyc2Vycy9kbWwvSW5zZXJ0SW50b1ZhbHVlcy5qYXZh) | 75.64% | [13 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...t/core/data/manager/ingest/LocalPreparedStore.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Fingest%2FLocalPreparedStore.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvaW5nZXN0L0xvY2FsUHJlcGFyZWRTdG9yZS5qYXZh) | 59.57% | [15 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../data/manager/ingest/RowInsertExecutorFactory.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Fingest%2FRowInsertExecutorFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvaW5nZXN0L1Jvd0luc2VydEV4ZWN1dG9yRmFjdG9yeS5qYXZh) | 0.00% | [18 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ore/data/manager/ingest/LocalShardLogProvider.java](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Fingest%2FLocalShardLogProvider.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvaW5nZXN0L0xvY2FsU2hhcmRMb2dQcm92aWRlci5qYXZh) | 40.90% | [12 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [4 more](https://app.codecov.io/gh/apache/pinot/pull/17996?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
> :exclamation: There is a different number of reports uploaded between BASE (428d055) and HEAD (50dfa60). Click for more details.
>
> <details><summary>HEAD has 28 uploads less than BASE</summary>
>
>| Flag | BASE (428d055) | HEAD (50dfa60) |
>|------|------|------|
>|java-21|4|1|
>|unittests1|2|1|
>|unittests|3|1|
>|temurin|8|1|
>|java-11|4|0|
>|unittests2|1|0|
>|integration|5|0|
>|integration1|2|0|
>|integration2|1|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17996 +/- ##
============================================
- Coverage 63.27% 55.47% -7.80%
+ Complexity 1541 745 -796
============================================
Files 3196 2518 -678
Lines 193889 143991 -49898
Branches 29843 23093 -6750
============================================
- Hits 122674 79879 -42795
+ Misses 61579 57322 -4257
+ Partials 9636 6790 -2846
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17996/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/17996/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/17996/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/17996/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/17996/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/17996/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/17996/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.47% <51.87%> (-7.79%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17996/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.47% <51.87%> (-7.80%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17996/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.47% <51.87%> (-7.80%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17996/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.47% <51.87%> (-0.04%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17996/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/17996?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
Re: [PR] [Phase 0] Group-by perf roadmap: JMH benchmarks + observability stats [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 closed pull request #17996: [Phase 0] Group-by perf roadmap: JMH benchmarks + observability stats
URL: https://github.com/apache/pinot/pull/17996
--
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