xiangfu0 commented on PR #19307: URL: https://github.com/apache/pinot/pull/19307#issuecomment-5828393434
Rebased onto current master (`d4365386d8`; the stack was 9 commits behind, clean rebase, all commits patch-identical, no file overlap) and re-reviewed the whole PR against the new base. One finding was worth a code change, pushed as `28fdf9c522`: - **V7 writer `close()` no longer throws on partial writes.** Closing with fewer docs than declared is the state of every creator when a segment build aborts; throwing there replaced the original ingestion error in the driver's catch-and-close path and stopped sibling creators from being closed (`FileUtils.close` only catches `IOException`). The legacy writer never threw here. It now logs a warning and leaves the file header-less, which the reader rejects at load. `close()` is idempotent and writes after close throw. Direct buffers are now allocated after the file is opened so a failed open leaks nothing. - **Reader:** dropped the per-row docId bounds check in favour of a once-per-chunk check, matching the legacy fixed-byte readers; byte-range prefetch hints are documented as deferred for V7. - **Small cleanups:** dead null-check in `ForwardIndexCreatorFactory`; plain record component names in `ForwardIndexHandler`. - **Tests:** corrupt payload bytes now exercise the codec-decode failure path and prove the context's cached chunk is not poisoned (the V7 analogue of `testFailedDeltaDecodeInvalidatesCachedChunk`); early close leaves an unreadable file; a legacy `compressionCodec` change is applied in the same reload as a standalone-dictionary toggle; leaving V7 with an explicit legacy codec lands on that codec. PR description updated to note the legacy-table reload behaviour and the close semantics, and `release-notes` added. #19308 and #19309 are restacked on this tip. The Unit Test Set 2 failure on the previous head was a runner-side `protoc did not exit cleanly` in `pinot-confluent-protobuf` during the build step (nothing in this PR touches protobuf); master passed the same job minutes earlier. -- 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]
