Stefan Miklosovic created CASSANDRA-21680:
---------------------------------------------
Summary: 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
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]
