xiangfu0 commented on PR #19307: URL: https://github.com/apache/pinot/pull/19307#issuecomment-5545029585
Rebased onto master after #19397 merged (`950c034`), then pushed review follow-up `feb515f1dc` as a separate commit so the delta is easy to see: - **Reader no longer walks every chunk frame at segment load.** The constructor now validates only header-resident structure (offset table strictly increasing with room for each frame's header, bounded by the pipeline's encoded-size limit) plus the final frame's extent, so truncated/concatenated files still fail at load without faulting in every data page. Each frame's exact size is verified in `loadChunk` on first access, matching how the legacy readers behave. - `Context.close()` resets the cached chunk id and `loadChunk` guards against a closed context, so a post-close read fails with `IllegalStateException` instead of touching freed direct memory. - `MAX_CODEC_SPEC_LENGTH_BYTES` is a frozen `4096` literal (byte-identical to the current DSL limit), and table-config validation now checks the canonical spec's byte length against it. - Dropped the write-only per-instance executor map in `ForwardIndexHandler`; the canonical spec is computed inline. - Javadoc: `getCodecSpec()` now says rollback requires regenerating or re-pushing segments (reload alone only rewrites the server-local copy) and lists the chunk-sizing knobs V7 ignores; `computeColumnOperations` documents that RAW codec reconciliation runs alongside standalone-dictionary toggles. - Tests (kept compact, no new classes): a 10,009-doc multi-chunk round trip per codec spec with a partial final chunk; writer/reader guard rejections; the malformed-file provider now has ten rows split by load-time vs read-time detection (new `gap` and `offsetOrder` cases pin the moved checks); and a reload that enables/disables a standalone dictionary while changing `codecSpec` in one pass. `ForwardIndexCreatorFactoryTest`, `ForwardIndexHandlerTest`, `ForwardIndexHandlerCompressionStatsTest`, and `TableConfigUtilsTest` pass locally (172 tests); Spotless, Checkstyle, and license checks pass on both modules. #19308 and #19309 are rebased onto this tip. PR description updated to match. -- 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]
