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

   Closing this — benchmarking showed the premise doesn't hold. 
`targetMaxChunkSize` already
   provides everything the docs-per-chunk cap was meant to add, so the new knob 
isn't needed.
   
   ## What the measurements showed
   
   Setup: 1M docs, ZSTD, V6, JDK 25, Apple Silicon, warm buffers. 
`randomAccess` = 2000 uniform
   point lookups.
   
   **1. Chunk *bytes*, not docs, is what drives lookup latency.** Normalizing 
the docs-cap sweep two
   ways, cross-dataset spread at a matched setting is 2.08–5.67x per doc but 
only 1.08–2.54x per
   uncompressed KB. Decompression cost is a function of bytes; docs only proxy 
for bytes via average
   value length, which varies per column.
   
   **2. `targetMaxChunkSize` alone already controls chunk geometry — including 
on skewed columns.**
   Through `SingleValueVarByteRawIndexCreator`, with `targetDocsPerChunk` at 
its default 1000 and no
   enforcement:
   
   | targetMaxChunkSize | LOW_CARD_URL docs/chunk | SKEWED_LOG docs/chunk |
   |---|---|---|
   | 8 KB | 104 | 110 |
   | 16 KB | 210 | 189 |
   | 32 KB | 422 | 293 |
   | 1 MB (default) | 954 | 9345 |
   
   SKEWED_LOG is avg 107 B with 0.1% ~50 KB values (max 50094 B). The byte 
budget tracks fine
   regardless of skew. What's broken is only `targetDocsPerChunk`'s `maxLength 
* N` *derivation*,
   which clamps to `targetMaxChunkSize` and stops tracking N — not byte control 
itself.
   
   **3. The docs cap does not buy a better size/latency frontier.** The two 
frontiers interleave:
   
   | dataset | byte-bounded | doc-bounded | result |
   |---|---|---|---|
   | SKEWED_LOG | 32 KB → 8.79 MB, **20.3 µs** | cap 1000 → 8.73 MB, 38.3 µs | 
byte 1.9x faster, same size |
   | SKEWED_LOG | 16 KB → **9.15 MB, 10.3 µs** | cap 250 → 9.24 MB, 12.1 µs | 
byte dominates |
   | JSON_LOG | 16 KB → **36.12 MB, 14.1 µs** | cap 1000 → 36.28 MB, 101.0 µs | 
byte dominates, 7x |
   | LOW_CARD_URL | 64 KB → 3.27 MB, 17.2 µs | cap 1000 → 3.24 MB, 17.8 µs | 
tie |
   | LOW_CARD_URL | 16 KB → 3.87 MB, 9.7 µs | cap 250 → **3.76 MB, 6.9 µs** | 
docs cap slightly ahead |
   
   **4. On skewed data the docs cap is actively worse.** It forces a chunk that 
swallows a 50 KB
   outlier to still collect N docs, so on-disk chunk sizes vary 8.2x; the byte 
cap flushes early and
   stays uniform:
   
   | dataset | mode | median | p95 | p99 | max | max/median |
   |---|---|---|---|---|---|---|
   | SKEWED_LOG | cap 250 | 1573 B | 4646 | 7434 | 12888 | **8.2x** |
   | SKEWED_LOG | byte 32 KB | 2908 B | 3118 | 3133 | 3158 | **1.1x** |
   | LOW_CARD_URL | cap 250 | 933 B | 950 | 957 | 968 | 1.0x |
   | LOW_CARD_URL | byte 32 KB | 1465 B | 1487 | 1495 | 1508 | 1.0x |
   
   Uniform doc counts are the wrong invariant for latency — uniform bytes is 
what you want, and that's
   what the byte budget already gives.
   
   ## The useful finding, which needs no code
   
   For point-lookup-heavy raw string/bytes columns, set `targetMaxChunkSize` to 
16–32 KB. On the
   skewed column that is **336 µs → 10.3 µs per lookup (33x) for 12% more 
space** (8.15 → 9.15 MB).
   The 1 MB default is reasonable for scans and poor for point lookups. Worth 
documenting as tuning
   guidance rather than adding a knob.
   
   Caveats on the numbers: warm-buffer only (cold-page mmap should favor small 
chunks further, so the
   lookup win is likely understated); ZSTD only for latency; random-access 
error bars are wide at 1 MB
   chunks. Directions and orders of magnitude are stable across reruns.
   
   ## Possible follow-up
   
   The one real defect left is that `targetDocsPerChunk` is named as if it 
controls document count but
   only picks a byte budget from `maxLength`, then silently clamps. Deprecating 
that derivation in
   favor of `targetMaxChunkSize` would be a much smaller and clearer change 
than this PR. Happy to
   open that separately if there's interest.
   
   Benchmark used here (`BenchmarkVarByteV6TargetDocsPerChunk`) can be 
resurrected from this branch if
   useful for the follow-up.
   


-- 
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]

Reply via email to