Skip to content

Fix delta forward index chunk caching - #19282

Open
xiangfu0 wants to merge 1 commit into
masterfrom
xiangfu0/codex/codec-delta-chunk-cache-fix
Open

Fix delta forward index chunk caching#19282
xiangfu0 wants to merge 1 commit into
masterfrom
xiangfu0/codex/codec-delta-chunk-cache-fix

Conversation

@xiangfu0

@xiangfu0 xiangfu0 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Extracted from #18229 as an independent correctness prerequisite.

BaseChunkForwardIndexReader previously allocated a separate decoded buffer for DELTA and DELTADELTA without storing it in ChunkReaderContext. The first read returned correct data, but later reads in the same chunk reused the untouched context buffer. The extra direct allocation also escaped the context lifecycle.

This change decodes every compression type into the context-owned full-chunk buffer and invalidates the cached chunk id before decoding so failures cannot leave a false cache hit.

Tests:

  • FixedByteChunkSVForwardIndexTest covers INT and LONG with DELTA and DELTADELTA across writer versions 2, 3, and 4
  • same-chunk reads, cross-chunk transitions, partial final chunks, and chunk revisits
  • malformed-chunk recovery proves a failed partial decode invalidates the cache before the prior chunk is reread
  • the full test class passes 87 tests
  • pinot-segment-local Spotless, Checkstyle, license format, and license check

This PR does not add codec-pipeline behavior and can merge independently of the codec stack.

@codecov-commenter

codecov-commenter commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.12%. Comparing base (b6cd43b) to head (a5d3700).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #19282      +/-   ##
============================================
+ Coverage     67.09%   67.12%   +0.03%     
  Complexity     1424     1424              
============================================
  Files          3459     3459              
  Lines        219789   219784       -5     
  Branches      35007    35006       -1     
============================================
+ Hits         147457   147520      +63     
+ Misses        60543    60483      -60     
+ Partials      11789    11781       -8     
Flag Coverage Δ
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-25 67.12% <100.00%> (+0.03%) ⬆️
lane-a 100.00% <ø> (ø)
lane-b 0.00% <ø> (ø)
temurin 67.12% <100.00%> (+0.03%) ⬆️
unittests 67.11% <100.00%> (+0.03%) ⬆️
unittests1 57.75% <100.00%> (+0.01%) ⬆️
unittests2 39.21% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0
xiangfu0 requested a balanced review from Copilot August 18, 2026 00:25
@xiangfu0
xiangfu0 marked this pull request as ready for review August 18, 2026 00:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes decoded chunk caching for DELTA codecs in raw forward-index readers.

Changes:

  • Decodes chunks into the context-owned buffer.
  • Invalidates cache state before decompression.
  • Adds INT/LONG DELTA regression tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
BaseChunkForwardIndexReader.java Corrects buffer ownership and cache invalidation.
FixedByteChunkSVForwardIndexTest.java Tests DELTA chunk transitions and revisits.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@xiangfu0
xiangfu0 force-pushed the xiangfu0/codex/codec-delta-chunk-cache-fix branch from c78ad6b to a5d3700 Compare August 18, 2026 07:18
@xiangfu0
xiangfu0 requested a review from Jackie-Jiang August 18, 2026 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants