arunkumarucet commented on PR #19056: URL: https://github.com/apache/pinot/pull/19056#issuecomment-5139891204
Fair point on the asymptotics — I benchmarked it rather than argue. Two updates pushed since: 1. `866e12e` removed the sorting of scan-side hash sets entirely (drain the sorted values into the hash set with plain O(n) inserts). 2. Benchmarking that version showed a different regression (draining a large accumulated sorted set into one small hash set was ~2x baseline because value-ordered inserts scatter across the table), so `9981221` makes the merge **size-aware**: the hash side is sorted only when it is at least 4x smaller than the accumulated sorted values (only the small side is ever sorted, e.g. one small scan segment among many no-scan segments); otherwise the sorted values drain into the hash set at plain insert cost and the accumulator stays a hash set from then on. A pure scan query never touches the new code path at all. **Merge benchmark** (the combine phase in isolation, 10 segments with the real per-segment cardinalities from the dataset below, ~1.59M values / 1.55M distinct, every iteration asserts the exact count; JDK 25, M-series): | scenario (shuffled block order) | baseline: all hash sets | patched: `union()` | |---|---|---| | 0 scan + 10 no-scan (pure no-scan) | 26–31ms | **4.9–5.7ms** | | 1 scan + 9 no-scan | 26–31ms | 8.3–9.1ms | | 5 scan + 5 no-scan | 26–31ms | 27–31ms | | 9 scan + 1 no-scan | 26–31ms | 27–32ms | | 10 scan (pure scan) | 26–31ms | 27–31ms | | 5/5, hash blocks arrive first | — | 30–33ms | | 5/5, sorted blocks arrive first | — | 30–31ms | No mix ratio or arrival order regresses against baseline; the win concentrates where the optimization applies. **End-to-end functional check** on a live single-server cluster built from this branch (ClickBench `hits`, 10M rows / 10 segments, `UserID` LONG with 1,530,334 distinct). The mixed case uses a `UserID >= T` predicate with T chosen between per-segment minimums so exactly 5 segments are match-all (no-scan) and 5 scan — confirmed by `numEntriesScannedInFilter = 5,000,000`. Expected values computed independently from the parquet files with pyarrow: | query | result | expected | warm latency | |---|---|---|---| | `DISTINCTCOUNT(UserID)` unfiltered (pure no-scan) | 1,530,334 | 1,530,334 | 20ms | | `DISTINCTCOUNT(UserID) WHERE UserID >= T` (5 no-scan + 5 scan) | 1,530,325 | 1,530,325 | 48ms | | `DISTINCTCOUNT(UserID) WHERE UserID >= 0` (scan-dominated) | 1,473,024 | 1,473,024 | 43ms | | `COUNT(DISTINCT UserID)` mixed | 1,530,325 | 1,530,325 | 48ms | | `DISTINCTCOUNTSMARTHLL(UserID, 'threshold=3000000;dictThreshold=3000000')` mixed, under threshold | 1,530,325 | 1,530,325 (exact) | 50ms | | `DISTINCTSUM(RegionID)` / group-by `DISTINCTCOUNT` | unchanged | — | unchanged | Also re-ran the randomized differential fuzz (random segment counts/sizes/overlaps, random scan/no-scan type per block, random merge order, verified against a hash-set ground truth): 160/160 trials pass. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
