[
https://issues.apache.org/jira/browse/CASSANDRA-21680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18116802#comment-18116802
]
Yifan Cai commented on CASSANDRA-21680:
---------------------------------------
+1
> ZstdCompressionDictionary reference leak when opening dictionary-compressed
> SSTables concurrently
> -------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-21680
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21680
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Feature/Compression
> Reporter: Stefan Miklosovic
> Assignee: Stefan Miklosovic
> Priority: Normal
> Fix For: 6.x, 7.x
>
>
> h2. Symptoms
>
> * Repeated \{{ERROR [Reference-Reaper] Ref.java:283 - LEAK DETECTED ...
> ZstdCompressionDictionary$Tidy}} in \{{system.log}}, in bursts on the
> reaper's GC cycle.
> * Volume scales with the number of dictionary-compressed SSTables opened
> concurrently — e.g. bulk open on startup via \{{SSTableReader.openAll}} on
> the \{{SSTableBatchOpen}} pool — roughly one leaked reference per
> concurrently-opened SSTable that shares a dictionary id.
> * Native/off-heap memory growth on long-lived nodes; each leaked dictionary
> keeps its pooled \{{ZstdCompressCtx}}/\{{ZstdDecompressCtx}} and dictionary
> tables alive.
>
> h2. Root cause
>
> With \{{-Dcassandra.debugrefcount=true}} the allocate trace points at the
> SSTable-open path:
>
> {noformat}
> ERROR [Reference-Reaper] Ref.java:283 - LEAK DETECTED: a reference
> (...ZstdCompressionDictionary$Tidy...) was not released before the reference
> was garbage collected
> ERROR [Reference-Reaper] Ref.java:323 - Allocate trace
> ...ZstdCompressionDictionary$Tidy...:
> Thread[SSTableBatchOpen:4]
> at org.apache.cassandra.utils.concurrent.Ref.<init>(Ref.java:143)
> at
> org.apache.cassandra.db.compression.ZstdCompressionDictionary.initRefLazily(ZstdCompressionDictionary.java:323)
> at
> org.apache.cassandra.db.compression.ZstdCompressionDictionary.tryRef(ZstdCompressionDictionary.java:299)
> at
> org.apache.cassandra.io.compress.CompressionMetadata.buildCloseableArray(CompressionMetadata.java:164)
> at
> org.apache.cassandra.io.compress.CompressionMetadata.<init>(CompressionMetadata.java:149)
> at
> org.apache.cassandra.io.compress.CompressionMetadata.open(CompressionMetadata.java:133)
> at
> org.apache.cassandra.io.sstable.format.CompressionInfoComponent.load(CompressionInfoComponent.java:59)
> at
> org.apache.cassandra.io.sstable.format.big.BigSSTableReaderLoadingBuilder.openComponents(BigSSTableReaderLoadingBuilder.java:143)
> at
> org.apache.cassandra.io.sstable.format.SSTableReader.lambda$openAll$4(SSTableReader.java:437)
> {noformat}
>
> The leak is a race between \{{CompressionDictionary.deserialize}} and
> \{{CompressionDictionaryCache.add}}:
>
> # On a cache miss, \{{deserialize}} creates a new \{{CompressionDictionary}}
> instance, calls \{{manager.add(...)}}, then *returns its own created instance
> regardless of which instance actually won the cache*.
> # \{{CompressionDictionaryCache.add}} stores via \{{cache.get(dictId,
> mappingFunction)}}; the mapping function — which calls \{{initRefLazily()}}
> to make the cache the owner of the dictionary's \{{selfRef}} — runs *only on
> a genuine miss*.
> # Under concurrent opens (multiple \{{SSTableBatchOpen}} threads opening
> SSTables that share one dictionary id), two or more threads all miss for the
> same id, each creates a separate instance, and each calls \{{add}}. Exactly
> one instance wins the cache; its \{{selfRef}} is owned by the cache and
> released on eviction via the removal listener (\{{dictionary.close()}}). The
> *losing* instances are returned to their callers.
> # \{{CompressionMetadata.buildCloseableArray}} then calls \{{tryRef()}} on
> the returned losing instance, lazily creating a \{{selfRef}} the cache never
> owns and never releases. When that \{{CompressionMetadata}} is later closed
> the reference is not released; the instance becomes unreachable and the
> reaper reports the leak.
>
> This is why the leak scales with concurrent opens, spikes on bulk open, and
> does not occur when dictionary SSTables are opened through the
> cursor-compaction path rather than \{{openAll}}.
>
> h2. Fix
>
> Make \{{ICompressionDictionaryCache.add}} (and the
> \{{CompressionDictionaryManager}} delegate and
> \{{CompressionDictionaryCache}} implementation) return the *canonical* cached
> instance — the value from \{{cache.get(key, mappingFunction)}}, which is
> either the argument (on a miss) or the pre-existing winner (on a hit/race).
> \{{CompressionDictionary.deserialize}} then adopts it:
>
> {code:java}
> CompressionDictionary dictionary = kind.createDictionary(dictId, dict,
> checksum);
> if (manager != null)
> {
> CompressionDictionary canonical = manager.add(dictionary);
> if (canonical != null)
> dictionary = canonical;
> }
> return dictionary;
> {code}
>
> Every caller now references the single cache-owned instance, and any
> redundant loser is discarded before it is ever \{{tryRef}}'d — so its
> \{{selfRef}} is never created and it is collected cleanly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
