You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "davecromberge (via GitHub)" <gi...@apache.org> on 2026/09/03 14:22:00 UTC
[PR] Do not build a discarded result when a sketch accumulator reaches its threshold [pinot]
davecromberge opened a new pull request, #19449:
URL: https://github.com/apache/pinot/pull/19449
## What
When an accumulator reaches its threshold, `applyInternal` calls `getResult()` to push the pending
batch into the union, and throws the returned sketch away:
```java
if (_accumulator.size() >= _threshold) {
getResult();
_accumulator.clear();
}
```
The union is the part that's needed. Building a sketch out of it is not.
This splits the union out of `getResult()` into a new `flush()`, and calls that instead. For Theta
the discarded result was five passes over the union gadget's hash table plus a compact sketch of up
to `nominalEntries` hashes, on every batch; for Tuple it also copied every summary.
Merging 400 stored sketches at `accumulatorThreshold=10`:
| | before | after |
| --- | --- | --- |
| Theta, lgK=14 | 16.01 ms / 17.92 MB | **5.20 ms / 2.91 MB** |
| Tuple, lgK=12 | 9.05 ms / 9.97 MB | **4.26 ms / 2.58 MB** |
| CPC, lgK=10 | 0.39 ms / 3.20 MB | 0.43 ms / 3.14 MB |
CPC is unchanged — `CpcUnion.getResult()` is cheap next to Theta's. It's included for consistency
and for the fix below.
Profiling a production cluster (16 servers, 80 minutes of live traffic) put
`ThetaUnionImpl.getResult` at 10.3% of all server CPU, and 21% in a Theta-heavy window. Almost all
of it was discarded.
## It also fixes data loss at accumulatorThreshold=1
The threshold branch called `getResult()`, which returns the single accumulated sketch *without*
unioning it, and `applyInternal` then cleared the accumulator — so the first sketch of every group
was silently dropped, and a later `getResult()` hit `IndexOutOfBoundsException` on the empty list.
`flush()` always unions, and the single-sketch shortcut now checks the sketch is still pending.
Seven sketches of 1000 distinct values each, at threshold 1, before this change:
```
ThetaSketchAccumulatorTest expected [6960.14] but found [5914.93]
CpcSketchAccumulatorTest expected [7000.78] but found [6003.72]
TupleIntSketchAccumulatorTest expected [6960.14] but found [5914.93]
```
## Tests
No existing test crossed a threshold — `testThresholdBehavior` sets it to 3 and applies 2 sketches
— so the flush path was uncovered, which is how both problems survived. Each accumulator test now
merges seven sketches at thresholds one through four and compares against a direct union of the
same sketches. The Tuple one also compares summary sums, since summaries are summed and a sketch
merged twice doubles a summary while leaving the estimate correct.
## Note for reviewers
`flush()` is `protected abstract`, so any accumulator outside this repo would need to implement it.
--
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] Query Performance: Optimise sketch accumulator for threshold condition [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19449:
URL: https://github.com/apache/pinot/pull/19449#issuecomment-5527846221
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19449?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 `54.83871%` with `14 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.78%. Comparing base ([`d08ac91`](https://app.codecov.io/gh/apache/pinot/commit/d08ac91365fd2cec3db595bdb636878c833a9d5b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f34e5e2`](https://app.codecov.io/gh/apache/pinot/commit/f34e5e264b974a74b55eeda6281502d227a62c83?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/19449?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...gment/local/customobject/CpcSketchAccumulator.java](https://app.codecov.io/gh/apache/pinot/pull/19449?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fcustomobject%2FCpcSketchAccumulator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvQ3BjU2tldGNoQWNjdW11bGF0b3IuamF2YQ==) | 0.00% | [9 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ent/local/customobject/ThetaSketchAccumulator.java](https://app.codecov.io/gh/apache/pinot/pull/19449?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fcustomobject%2FThetaSketchAccumulator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvVGhldGFTa2V0Y2hBY2N1bXVsYXRvci5qYXZh) | 80.00% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19449?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../local/customobject/TupleIntSketchAccumulator.java](https://app.codecov.io/gh/apache/pinot/pull/19449?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fcustomobject%2FTupleIntSketchAccumulator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvVHVwbGVJbnRTa2V0Y2hBY2N1bXVsYXRvci5qYXZh) | 80.00% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19449?src=pr&el=tree&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 (d08ac91) and HEAD (f34e5e2). Click for more details.
>
> <details><summary>HEAD has 24 uploads less than BASE</summary>
>
>| Flag | BASE (d08ac91) | HEAD (f34e5e2) |
>|------|------|------|
>|unittests2|1|0|
>|unittests|2|1|
>|java-25|6|1|
>|temurin|6|1|
>|lane-a|2|0|
>|integration|4|0|
>|integration1|2|0|
>|lane-b|2|0|
>|integration2|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19449 +/- ##
============================================
- Coverage 67.67% 57.78% -9.89%
+ Complexity 1430 1 -1429
============================================
Files 3488 2685 -803
Lines 224395 164192 -60203
Branches 35422 26660 -8762
============================================
- Hits 151867 94884 -56983
- Misses 60488 61276 +788
+ Partials 12040 8032 -4008
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19449/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19449/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/19449/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/19449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.78% <54.83%> (-9.89%)` | :arrow_down: |
| [lane-a](https://app.codecov.io/gh/apache/pinot/pull/19449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [lane-b](https://app.codecov.io/gh/apache/pinot/pull/19449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.78% <54.83%> (-9.89%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.78% <54.83%> (-9.89%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19449/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.78% <54.83%> (+<0.01%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19449/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 Harness](https://app.codecov.io/gh/apache/pinot/pull/19449?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] Query Performance: Optimise sketch accumulator for threshold condition [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #19449:
URL: https://github.com/apache/pinot/pull/19449
--
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