[
https://issues.apache.org/jira/browse/CASSANDRA-21671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18114936#comment-18114936
]
Jon Haddad commented on CASSANDRA-21671:
----------------------------------------
Spoke to Sam on Slack, we agreed to remove the thread pool. The buffers are
allocated once per input sstable, and they don't provide any measurable
improvement, so removing the complexity is the right move.
> direct io + read ahead buffer exception
> ---------------------------------------
>
> Key: CASSANDRA-21671
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21671
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Local/Compaction
> Reporter: Jon Haddad
> Assignee: Jon Haddad
> Priority: Normal
>
> Under sustained compaction with the BTI SSTable format and Direct IO reads,
> compaction tasks abort with two distinct {{{}IllegalArgumentException{}}}s
> that share one root cause. Both are new in 6.0 and unreleased.
> {{ThreadLocalReadAheadBuffer}} caches its per-thread read-ahead {{Block}} in
> a _static_ {{FastThreadLocal<Map<String, Block>>}} keyed by file path. The
> cached {{Block}} is shared across {{ThreadLocalReadAheadBuffer}} instances
> that do not have identical buffer semantics. On a single compaction thread, a
> second instance over the same file path reuses an already-populated {{Block}}
> and skips first-time setup. This produces two failures.
> h3. Failure 1: {{newLimit < 0: (-1 < 0)}}
> {{bufferSize}} is a per-instance field initialised to {{{}-1{}}}, assigned
> only inside the {{block.buffer == null}} branch of {{{}getBlock(){}}}. A
> second instance that reuses a cached {{Block}} never enters that branch, so
> its {{bufferSize}} stays {{{}-1{}}}. In {{{}fill(){}}}, {{Math.min(remaining,
> -1)}} is {{{}-1{}}}, which is passed to {{ByteBuffer.limit(-1)}} and throws.
> {{{}java.lang.IllegalArgumentException: newLimit < 0: (-1 < 0)
> at java.base/java.nio.MappedByteBuffer.limit(MappedByteBuffer.java:73)
> at
> org.apache.cassandra.io.util.ThreadLocalReadAheadBuffer.loadBlock(ThreadLocalReadAheadBuffer.java:130)
> at
> org.apache.cassandra.io.util.ThreadLocalReadAheadBuffer.fill(ThreadLocalReadAheadBuffer.java:119)
> at
> org.apache.cassandra.io.util.CompressedChunkReader$ScanCompressedReader.read(CompressedChunkReader.java:229)
> ...
> at
> org.apache.cassandra.io.sstable.SSTableRewriter.moveStarts(SSTableRewriter.java:240)
> at
> org.apache.cassandra.io.sstable.format.bti.BtiTableWriter.lambda$openEarly$1(BtiTableWriter.java:130)
> at
> org.apache.cassandra.db.compaction.CompactionTask.runMayThrow(CompactionTask.java:292){}}}Copy
> BTI compaction early-open triggers it: {{SSTableRewriter.moveStarts}} ->
> {{BtiTableReader.firstKeyBeyond}} opens a fresh reader over a file the same
> compaction thread already read through.
> Introduced by {{6f5fe8c06d5}} (2026-01-16, "Add Direct IO support for
> compaction reads"), which made {{bufferSize}} a lazily initialised instance
> field inside the {{buffer == null}} branch. The read-ahead scaffold is
> {{a09d5240abc}} (2024-04-17).
> h3. Failure 2: {{Cannot clean a buffer with no cleaner}}
> Once Failure 1 is fixed, a second exception surfaces (it was previously
> suppressed under the {{newLimit}} primary during failed-reader cleanup).
> {{{}java.lang.IllegalArgumentException: Cannot clean a buffer with no cleaner
> and attachment type
> java.nio.DirectByteBuffer; this buffer does not own its memory.
> For slices/duplicates, resolve to the root allocation before calling clean()
> at org.apache.cassandra.utils.memory.MemoryUtil.clean(MemoryUtil.java:346)
> at
> org.apache.cassandra.io.util.ThreadLocalReadAheadBuffer.cleanBuffer(ThreadLocalReadAheadBuffer.java:174)
> at
> org.apache.cassandra.io.util.ThreadLocalReadAheadBuffer.clear(ThreadLocalReadAheadBuffer.java:167)
> at
> org.apache.cassandra.io.util.CompressedChunkReader$ScanCompressedReader.deallocateResources(CompressedChunkReader.java:260)
> ...
> at
> org.apache.cassandra.io.util.RandomAccessReader.close(RandomAccessReader.java:189){}}}Copy
> {{ThreadLocalReadAheadBuffer}} (base, built by
> {{{}CompressedChunkReader.Standard{}}}) allocates a direct buffer that owns
> its memory. {{DirectThreadLocalReadAheadBuffer}} (built by
> {{{}CompressedChunkReader.Direct{}}}) allocates an aligned _slice_ that does
> not own its memory and overrides {{cleanBuffer()}} to free the attachment.
> Because both can share the same cached {{Block}} on one thread, a base
> instance can free a slice allocated by a Direct instance. Cleanup dispatches
> on the cleaning instance's class, not the buffer's actual ownership, so the
> base {{cleanBuffer()}} calls {{{}MemoryUtil.clean(slice){}}}; the slice has
> no cleaner and a {{DirectByteBuffer}} attachment, so the hardened
> {{MemoryUtil.clean()}} throws. Pre-hardening, this was a latent unsafe free /
> memory-corruption risk.
> h2. Environment
> * 3-node cluster, i4i-class local NVMe.
> * {{sstable.selected_format: bti}}
> * {{{}compaction_read_disk_access_mode: direct{}}},
> {{background_write_disk_access_mode: direct}}
> * {{{}default_compaction: UnifiedCompactionStrategy{}}}, LZ4 compression.
> * Reproduced on all nodes; thousands of occurrences per node over multi-hour
> windows.
> h2. Steps to reproduce
> # Configure BTI format and the Direct IO disk-access modes above.
> # Write enough data to a compressed table to trigger multi-input UCS
> compactions large enough for early-open
> ({{{}SSTableRewriter.maybeReopenEarly{}}}) to fire.
> # Watch {{system.log}} for {{newLimit < 0}} on {{CompactionExecutor}}
> threads; after the {{bufferSize}} fix, watch for {{{}Cannot clean a buffer
> with no cleaner{}}}.
> h2. Fix
> Two minimal changes in {{{}ThreadLocalReadAheadBuffer{}}}:
> # 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. The capacity is
> identical for every instance over the same file.
> # Base {{cleanBuffer()}} resolves a direct buffer that has no cleaner and a
> {{ByteBuffer}} attachment to its root allocation before calling
> {{{}MemoryUtil.clean{}}}. The redundant
> {{DirectThreadLocalReadAheadBuffer.cleanBuffer()}} override is deleted; the
> subclass inherits the single correct base implementation.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
