Jackie-Jiang opened a new pull request, #19431: URL: https://github.com/apache/pinot/pull/19431
## Summary Follow-up to #19399, which enforces the fixed-byte multi-value forward index's per-entry value cap at consume time. - **Drop the `IllegalArgumentException` rethrow in `addNewRow`.** It fired inside the per-index write loop, after the dictionary and earlier indexes had already been updated for the docId but before `_numDocsIndexed` advanced, which is exactly the partially indexed row from #16316 that the validation exists to prevent. Index write failures are fail-soft again. The up-front check, renamed from `validateLengthOfMVColumns()` to `validateNumMultiValues()`, remains the guard and now throws `IllegalStateException`. - **Single source for the default cap.** `ForwardIndexType.MAX_MULTI_VALUES_PER_ROW` moves into the SPI as `ForwardIndexConfig.DEFAULT_MAX_NUM_MULTI_VALUES`. `MutableIndexContext.maxNumMultiValues` defaults to it, replacing the `...PerRowOverride` field and its `0` sentinel. `ForwardIndexType` reads the context value directly, and `MutableSegmentImpl` captures the per-column cap from the context instead of an `instanceof` on the concrete writer. Vector columns still set it to the vector dimension. - **Fix `avgNumMultiValues` buffer sizing.** Since #10687 the mutable index context was fed `getEstimatedAvgColSize()` (a string-size statistic, 32 by default) as the average multi-value count, while the configured `avgMultiValueCount` (default 2) carried by `RealtimeSegmentConfig` was never read. Every fixed-byte MV forward index buffer was therefore sized roughly 16x larger than intended. The context now uses the configured value. - **Naming.** `FixedByteMVMutableForwardIndex` parameters and fields are now `maxNumMultiValues` / `avgNumMultiValues` / `numRowsPerChunk` / `valueSizeInBytes`, since the cap applies per multi-value entry rather than per row. The unused `getMaxChunkCapacity()` and the writer-side cap getter are removed, and `MultiValueRowLimit` becomes the `MultiValueLimit` record. - A TODO on `ForwardIndexConfig` tracks making the cap configurable and enforcing it for the immutable forward index as well. Behavior note: a multi-value column whose forward index is disabled or supplied by a non-default implementation is now validated against the cap too. Pinot's own paths are unaffected, since every enabled MV forward index in a mutable segment is a `FixedByteMVMutableForwardIndex`. ##### Was generative AI tooling used to co-author this PR? - [x] Yes -- 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]
