xiangfu0 commented on code in PR #19282:
URL: https://github.com/apache/pinot/pull/19282#discussion_r3800584756
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/forward/FixedByteChunkSVForwardIndexTest.java:
##########
@@ -60,6 +60,73 @@ public static Object[][] combinations() {
.toArray(Object[][]::new);
}
+ @DataProvider(name = "deltaCompressions")
+ public static Object[][] deltaCompressions() {
+ return new Object[][]{
+ {ChunkCompressionType.DELTA},
+ {ChunkCompressionType.DELTADELTA}
+ };
+ }
+
+ @Test(dataProvider = "deltaCompressions")
+ public void testDeltaIntChunkCaching(ChunkCompressionType compressionType)
+ throws Exception {
+ int[] expected = {101, 103, 107, 109, 211, 223, 227, 229, 307};
+ File outputFile = new File(TEST_FILE + "-int-" + compressionType);
+ FileUtils.deleteQuietly(outputFile);
+
+ try {
+ try (FixedByteChunkForwardIndexWriter writer = new
FixedByteChunkForwardIndexWriter(outputFile,
+ compressionType, expected.length, 4, Integer.BYTES,
FixedBytePower2ChunkSVForwardIndexReader.VERSION)) {
Review Comment:
Addressed in c78ad6b79a. The INT and LONG regressions now run DELTA and
DELTADELTA across writer versions 2, 3, and 4, selecting
FixedByteChunkSVForwardIndexReader for versions 2 and 3 and
FixedBytePower2ChunkSVForwardIndexReader for version 4. The full
FixedByteChunkSVForwardIndexTest passes 87 tests.
##########
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);
Review Comment:
Addressed in c78ad6b79a. The new regression caches chunk 0, corrupts chunk 1
so DELTA partially writes the shared buffer and then fails on an impossible
compressed size, asserts the context chunk id is reset to -1, and rereads chunk
0 to prove it is decoded again instead of returning the mutated buffer. The
full test class passes 87 tests.
--
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]
