[
https://issues.apache.org/jira/browse/CASSANDRA-21671?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Caleb Rackliffe updated CASSANDRA-21671:
----------------------------------------
Attachment: (was: result_details.tar-1.gz)
> Fix compressed scan read-ahead buffer Block leak and chunk-cache pollution by
> scans
> -----------------------------------------------------------------------------------
>
> 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
> Fix For: 5.0.x, 6.0-alpha3, 7.x
>
> Time Spent: 3h 50m
> Remaining Estimate: 0h
>
> 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]
