From b0ea37067f7ac8d03cb785ce70616fa16a5ba5a4 Mon Sep 17 00:00:00 2001 From: "tadei.bilan" Date: Fri, 23 Oct 2020 14:29:50 +0300 Subject: [PATCH] Use java.lang.ref.Cleaner for ClientBackedTransaction Improve temporary file clean up by using a Cleaner to dispatch cleaning tasks. Since it gives us a Cleanable, we can dispense with a tracking map and removal from it -- Cleanable makes sure it is called exactly once. JIRA: CONTROLLER-1911 Change-Id: I02d2ee57a9fada2c54ba06008b6c28681a709748 Signed-off-by: tadei.bilan Signed-off-by: Robert Varga --- .../databroker/ClientBackedTransaction.java | 48 ++++++++----------- .../actors/dds/AbstractClientHandle.java | 7 +-- .../ClientBackedTransactionTest.java | 7 ++- 3 files changed, 28 insertions(+), 34 deletions(-) diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransaction.java index 9b65a881ec..a0f369638d 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransaction.java @@ -9,16 +9,13 @@ package org.opendaylight.controller.cluster.databroker; import static java.util.Objects.requireNonNull; -import com.google.common.base.FinalizablePhantomReference; -import com.google.common.base.FinalizableReferenceQueue; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; +import java.lang.ref.Cleaner; +import java.lang.ref.Cleaner.Cleanable; import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier; import org.opendaylight.controller.cluster.databroker.actors.dds.AbstractClientHandle; import org.opendaylight.controller.cluster.databroker.actors.dds.ClientTransaction; import org.opendaylight.mdsal.dom.spi.store.AbstractDOMStoreTransaction; +import org.opendaylight.mdsal.dom.spi.store.DOMStoreTransaction; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,50 +28,43 @@ import org.slf4j.LoggerFactory; */ abstract class ClientBackedTransaction> extends AbstractDOMStoreTransaction { - private static final class Finalizer extends FinalizablePhantomReference> { - private static final FinalizableReferenceQueue QUEUE = new FinalizableReferenceQueue(); - private static final Set FINALIZERS = ConcurrentHashMap.newKeySet(); - private static final Logger LOG = LoggerFactory.getLogger(Finalizer.class); - + private static final class Cleanup implements Runnable { private final AbstractClientHandle transaction; private final Throwable allocationContext; - private Finalizer(final ClientBackedTransaction referent, final AbstractClientHandle transaction, - final Throwable allocationContext) { - super(referent, QUEUE); - this.transaction = requireNonNull(transaction); + Cleanup(final AbstractClientHandle transaction, final Throwable allocationContext) { + this.transaction = transaction; this.allocationContext = allocationContext; } - static > @NonNull T recordTransaction( - final @NonNull ClientBackedTransaction referent, final @NonNull T transaction, - final @Nullable Throwable allocationContext) { - FINALIZERS.add(new Finalizer(referent, transaction, allocationContext)); - return transaction; - } - @Override - public void finalizeReferent() { - FINALIZERS.remove(this); + public void run() { if (transaction.abort()) { LOG.info("Aborted orphan transaction {}", transaction, allocationContext); } } } + private static final Logger LOG = LoggerFactory.getLogger(ClientBackedTransaction.class); + private static final Cleaner CLEANER = Cleaner.create(); + private final T delegate; + private final Cleanable cleanable; ClientBackedTransaction(final T delegate, final Throwable allocationContext) { super(delegate.getIdentifier()); - this.delegate = Finalizer.recordTransaction(this, delegate, allocationContext); - } - - final T delegate() { - return delegate; + this.delegate = requireNonNull(delegate); + this.cleanable = CLEANER.register(this, new Cleanup(delegate, allocationContext)); } @Override public void close() { delegate.abort(); + // Run cleaning immediate so the references is not stuck in cleaner queue + cleanable.clean(); + } + + final T delegate() { + return delegate; } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java index 90773359dc..842bc00d91 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java @@ -97,9 +97,10 @@ public abstract class AbstractClientHandle e * closed, too. */ final @Nullable Collection ensureClosed() { - @SuppressWarnings("unchecked") - final State local = STATE_UPDATER.getAndSet(this, null); - return local == null ? null : local.values(); + // volatile read and a conditional CAS. This ends up being better in the typical case when we are invoked more + // than once (see ClientBackedTransaction) than performing a STATE_UPDATER.getAndSet(). + final State local = state; + return local != null && STATE_UPDATER.compareAndSet(this, local, null) ? local.values() : null; } final T ensureProxy(final YangInstanceIdentifier path) { diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransactionTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransactionTest.java index 39e8d74dce..be140eb625 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransactionTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransactionTest.java @@ -7,8 +7,10 @@ */ package org.opendaylight.controller.cluster.databroker; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + import org.junit.Test; -import org.mockito.Mockito; import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier; import org.opendaylight.controller.cluster.access.concepts.FrontendIdentifier; import org.opendaylight.controller.cluster.access.concepts.FrontendType; @@ -31,6 +33,7 @@ public abstract class ClientBackedTransactionTest delegate = object().delegate(); object().close(); - Mockito.verify(delegate).abort(); + // Called twice because of immediate cleaning + verify(delegate, times(2)).abort(); } } \ No newline at end of file -- 2.36.6