xiangfu0 commented on code in PR #19282:
URL: https://github.com/apache/pinot/pull/19282#discussion_r3808255160
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/forward/BaseChunkForwardIndexReader.java:
##########
@@ -211,17 +211,17 @@ protected ByteBuffer decompressChunk(int chunkId,
ChunkReaderContext context) {
ByteBuffer decompressedBuffer = context.getChunkBuffer();
decompressedBuffer.clear();
+ // Invalidate the cached chunk before decoding. If decompression fails, a
subsequent read must
+ // retry instead of returning a partially-mutated buffer as a cache hit.
+ context.setChunkId(-1);
try {
- if (_compressionType == ChunkCompressionType.DELTA || _compressionType
== ChunkCompressionType.DELTADELTA) {
- // For delta-based compression, pre-size the output using
decompressor's length calculation.
Review Comment:
Good question. I traced the special case to #15258 / commit 687770ee3e1. It
only pre-sized an exact output buffer using decompressedLength(); it did not
use different DELTA decoding semantics. The fixed-byte reader context was
already allocated for a complete decoded chunk, and the writer stores the same
docs-per-chunk and fixed entry size in the header, so a valid full INT/LONG
chunk exactly fits while a partial final chunk is smaller. ChunkDecompressor
only requires sufficient output capacity, and both DELTA decoders flip the
supplied buffer after writing, so decoding into the larger context buffer
preserves the same bytes and limit.
The old exact-size allocation was actually the cache bug: it was returned
for the first read but never installed in the context, while the context chunk
ID was marked cached. The next same-chunk read therefore used the untouched
context buffer. The added tests cover DELTA/DELTADELTA × INT/LONG × V2/V3/V4,
including full and partial chunks, same-chunk hits, cross-chunk reads,
revisits, and failed-decode recovery. So this does not break the original
sizing requirement; it removes the unnecessary direct allocation and restores
the reader-context contract.
--
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]
