21671 direct io + readahead exception fix - #5152
Open
rustyrazorblade wants to merge 3 commits into
Open
Conversation
added 3 commits
September 9, 2026 19:31
…calReadAheadBuffer Block Block objects are cached in a static thread-local map keyed by file path and shared across all ThreadLocalReadAheadBuffer instances. bufferSize is a per-instance field initialised only inside the block.buffer == null branch of getBlock(). When a second instance, on the same thread and for the same file path, reused an already-allocated cached Block, that branch was skipped and its bufferSize stayed -1. fill() then computed Math.min(remaining, -1) == -1 and called ByteBuffer.limit(-1), throwing IllegalArgumentException: newLimit < 0 and aborting compaction. Move the bufferSize initialisation out of the block.buffer == null guard so any instance that observes a Block, fresh or reused, initialises bufferSize from the buffer capacity. Add a regression test that reproduces the two-instance, same-path, same-thread scenario.
Second defect in the same read-ahead Direct IO family as Bug #01. The static
per-thread, per-path Block cache in ThreadLocalReadAheadBuffer is shared across
instances with different buffer ownership:
- DirectThreadLocalReadAheadBuffer allocates an aligned SLICE via
BufferUtil.allocateDirectAligned (no Cleaner; attachment = backing
DirectByteBuffer) and previously overrode cleanBuffer() to clean the
attachment.
- Base ThreadLocalReadAheadBuffer allocates an OWNED buffer (has a Cleaner)
and cleanBuffer() calls MemoryUtil.clean(buffer) directly.
CompressedChunkReader.Direct uses the Direct subclass; .Standard uses the base.
Both can open over the same file path on the same thread and share the same
cached Block. When a Direct instance populates the Block with a slice and a
Standard instance over the same path reuses it (the reuse path Bug #01 exposed),
the Standard instance frees the slice through the base cleanBuffer, calling
MemoryUtil.clean(slice). The hardened MemoryUtil.clean rejects a buffer with no
cleaner and a non-null attachment (a latent double-free pre-hardening), throwing
IllegalArgumentException and aborting BTI compaction.
Make cleanup type-safe in the base class: cleanBuffer() resolves a direct buffer
with no cleaner but a ByteBuffer attachment to its root allocation before calling
MemoryUtil.clean, so any instance frees any buffer correctly. This does not weaken
MemoryUtil.clean's guard; it resolves to the root before calling it. The
DirectThreadLocalReadAheadBuffer.cleanBuffer override is now redundant and is
removed so both paths use the single correct base implementation. The Bug #01
bufferSize fix is unchanged.
Add a regression test that reproduces the Direct-slice/base-free scenario.
rustyrazorblade
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes direct io + read ahead exception, CASSANRA-21671