natas_hw
/
cassandra
mirror of https://github.com/apache/cassandra
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove strong ref loop in LogTransaction.SSTableTidier 

Patch by Josh McKenzie; reviewed by Benedict Elliott Smith for CASSANDRA-17205
This commit is contained in:
Josh McKenzie 2021-12-14 12:01:47 -05:00
parent bf1446cd85
commit f4a2135c5b
2 changed files with 25 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
CHANGES.txt
View File

@ -1,7 +1,8 @@
4.1
* Remove self-reference in SSTableTidier (CASSANDRA-17205)
* Add guardrail for query page size (CASSANDRA-17189)
* Allow column_index_size_in_kb to be configurable through nodetool (CASSANDRA-17121)
* Emit a metric for number of local read and write calls
* Emit a metric for number of local read and write calls
* Add non-blocking mode for CDC writes (CASSANDRA-17001)
* Add guardrails framework (CASSANDRA-17147)
* Harden resource management on SSTable components to prevent future leaks (CASSANDRA-17174)
@ -228,7 +229,7 @@ Merged from 3.0:
* Promote protocol V5 out of beta (CASSANDRA-14973)
* Fix incorrect encoding for strings can be UTF8 (CASSANDRA-16429)
* Fix node unable to join when RF > N in multi-DC with added warning (CASSANDRA-16296)
* Add an option to nodetool tablestats to check sstable location correctness (CASSANDRA-16344)
* Add an option to nodetool tablestats to check sstable location correctness (CASSANDRA-16344)
* Unable to ALTER KEYSPACE while decommissioned/assassinated nodes are in gossip (CASSANDRA-16422)
* Metrics backward compatibility restored after CASSANDRA-15066 (CASSANDRA-16083)
* Reduce new reserved keywords introduced since 3.0 (CASSANDRA-16439)
@ -768,7 +769,7 @@ Merged from 3.0:
* Use standard Amazon naming for datacenter and rack in Ec2Snitch (CASSANDRA-7839)
* Abstract write path for pluggable storage (CASSANDRA-14118)
* nodetool describecluster should be more informative (CASSANDRA-13853)
* Compaction performance improvements (CASSANDRA-14261)
* Compaction performance improvements (CASSANDRA-14261)
* Refactor Pair usage to avoid boxing ints/longs (CASSANDRA-14260)
* Add options to nodetool tablestats to sort and limit output (CASSANDRA-13889)
* Rename internals to reflect CQL vocabulary (CASSANDRA-14354)
@ -1153,7 +1154,7 @@ Merged from 2.1:
3.11.2
* Fix ReadCommandTest (CASSANDRA-14234)
* Remove trailing period from latency reports at keyspace level (CASSANDRA-14233)
* Remove dependencies on JVM internal classes from JMXServerUtils (CASSANDRA-14173)
* Remove dependencies on JVM internal classes from JMXServerUtils (CASSANDRA-14173)
* Add DEFAULT, UNSET, MBEAN and MBEANS to `ReservedKeywords` (CASSANDRA-14205)
* Print correct snitch info from nodetool describecluster (CASSANDRA-13528)
* Enable CDC unittest (CASSANDRA-14141)

29
src/java/org/apache/cassandra/db/lifecycle/LogTransaction.java
View File

@ -30,12 +30,12 @@ import java.util.function.Predicate;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.Runnables;
import org.apache.cassandra.io.util.File;
import com.codahale.metrics.Counter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.concurrent.ScheduledExecutors;
import org.apache.cassandra.schema.TableMetadata;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.db.Directories;
import org.apache.cassandra.db.SystemKeyspace;
import org.apache.cassandra.db.compaction.OperationType;
@ -46,7 +46,9 @@ import org.apache.cassandra.io.sstable.Descriptor;
import org.apache.cassandra.io.sstable.SSTable;
import org.apache.cassandra.io.sstable.SnapshotDeletingTask;
import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.io.util.File;
import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.schema.TableMetadata;
import org.apache.cassandra.service.StorageService;
import org.apache.cassandra.utils.*;
import org.apache.cassandra.utils.concurrent.Ref;
@ -349,21 +351,25 @@ class LogTransaction extends Transactional.AbstractTransactional implements Tran
// must not retain a reference to the SSTableReader, else leak detection cannot kick in
private final Descriptor desc;
private final long sizeOnDisk;
private final Tracker tracker;
private final boolean wasNew;
private final Object lock;
private final Ref<LogTransaction> parentRef;
private final UUID txnId;
private final Counter totalDiskSpaceUsed;
public SSTableTidier(SSTableReader referent, boolean wasNew, LogTransaction parent)
{
this.desc = referent.descriptor;
this.sizeOnDisk = referent.bytesOnDisk();
this.tracker = parent.tracker;
this.wasNew = wasNew;
this.lock = parent.lock;
this.parentRef = parent.selfRef.tryRef();
this.txnId = parent.id();
// While the parent cfs may be dropped in the interim of us taking a reference to this and using it, at worst
// we'll be updating a metric for a now dropped ColumnFamilyStore. We do not hold a reference to the tracker or
// cfs as that would create a strong ref loop and violate our ability to do leak detection.
totalDiskSpaceUsed = parent.tracker != null && parent.tracker.cfstore != null ?
parent.tracker.cfstore.metric.totalDiskSpaceUsed :
null;
if (this.parentRef == null)
throw new IllegalStateException("Transaction already completed");
@ -371,7 +377,9 @@ class LogTransaction extends Transactional.AbstractTransactional implements Tran
public void run()
{
if (tracker != null && !tracker.isDummy())
// While this may be a dummy tracker w/out information in the metrics table, we attempt to delete regardless
// and allow the delete to silently fail if this is an invalid ks + cf combination at time of tidy run.
if (DatabaseDescriptor.isDaemonInitialized())
SystemKeyspace.clearSSTableReadMeter(desc.ksname, desc.cfname, desc.generation);
synchronized (lock)
@ -399,8 +407,11 @@ class LogTransaction extends Transactional.AbstractTransactional implements Tran
return;
}
if (tracker != null && tracker.cfstore != null && !wasNew)
tracker.cfstore.metric.totalDiskSpaceUsed.dec(sizeOnDisk);
// It's possible we're the last one's holding a ref to this metric if it's already been released in the
// parent TableMetrics; we run this regardless rather than holding a ref to that CFS or Tracker and thus
// creating a strong ref loop
if (DatabaseDescriptor.isDaemonInitialized() && totalDiskSpaceUsed != null && !wasNew)
totalDiskSpaceUsed.dec(sizeOnDisk);
// release the referent to the parent so that the all transaction files can be released
parentRef.release();