From: Robert Varga Date: Sun, 7 Nov 2021 00:19:18 +0000 (+0100) Subject: Minor sal-distributed-datastore cleanups X-Git-Tag: v4.0.6~1 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=27e0cf91aaba3d2a9f9e3f33e31f0faece502cd4 Minor sal-distributed-datastore cleanups Corrent a @GuardedBy annotations, add @NonNull annotations, fix javadocs and make a few methods final. Change-Id: Icb93aae229fb0aece5ece223a759ce9f36ee0297 Signed-off-by: Robert Varga --- 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 90971b31ce..aa4779c898 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 @@ -55,6 +55,7 @@ public abstract class AbstractClientHandle e } @Override + // Non-final for mocking public TransactionIdentifier getIdentifier() { return transactionId; } @@ -64,6 +65,7 @@ public abstract class AbstractClientHandle e * * @return True if this transaction became closed during this call */ + // Non-final for mocking public boolean abort() { if (commonAbort()) { parent.onTransactionAbort(this); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java index d445d3c2f6..d306d13e2e 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java @@ -104,7 +104,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id } @Override - public LocalHistoryIdentifier getIdentifier() { + public final LocalHistoryIdentifier getIdentifier() { return identifier; } @@ -135,6 +135,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id /** * Create a new history proxy for a given shard. * + * @param shard Shard cookie * @throws InversibleLockException if the shard is being reconnected */ @Holding("lock") @@ -203,6 +204,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id * @throws DOMTransactionChainClosedException if this history is closed * @throws IllegalStateException if a previous dependent transaction has not been closed */ + // Non-final for mocking public @NonNull ClientTransaction createTransaction() { checkNotClosed(); @@ -220,6 +222,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id * @throws DOMTransactionChainClosedException if this history is closed * @throws IllegalStateException if a previous dependent transaction has not been closed */ + // Non-final for mocking public ClientSnapshot takeSnapshot() { checkNotClosed(); @@ -239,7 +242,7 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id /** * Callback invoked from {@link ClientTransaction} when a child transaction readied for submission. * - * @param txId Transaction identifier + * @param tx Client transaction * @param cohort Transaction commit cohort */ synchronized AbstractTransactionCommitCohort onTransactionReady(final ClientTransaction tx, @@ -274,13 +277,14 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id * * @param txId transaction identifier */ + // Non-final for mocking synchronized void onTransactionComplete(final TransactionIdentifier txId) { if (readyTransactions.remove(txId) == null) { LOG.warn("Could not find completed transaction {}", txId); } } - HistoryReconnectCohort startReconnect(final ConnectedClientConnection newConn) { + final HistoryReconnectCohort startReconnect(final ConnectedClientConnection newConn) { /* * This looks ugly and unusual and there is a reason for that, as the locking involved is in multiple places. * @@ -328,5 +332,4 @@ public abstract class AbstractClientHistory extends LocalAbortable implements Id } }; } - } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java index 9b4c5a962f..f5855c279a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java @@ -7,9 +7,9 @@ */ package org.opendaylight.controller.cluster.databroker.actors.dds; +import static com.google.common.base.Preconditions.checkState; + import com.google.common.annotations.Beta; -import com.google.common.base.Preconditions; -import com.google.common.collect.Iterables; import com.google.common.util.concurrent.FluentFuture; import java.util.Collection; import java.util.Optional; @@ -80,7 +80,7 @@ public class ClientTransaction extends AbstractClientHandle toReady = ensureClosed(); - Preconditions.checkState(toReady != null, "Attempted to submit a closed transaction %s", this); + checkState(toReady != null, "Attempted to submit a closed transaction %s", this); toReady.forEach(AbstractProxyTransaction::seal); final AbstractTransactionCommitCohort cohort; @@ -89,8 +89,7 @@ public class ClientTransaction extends AbstractClientHandle { @Override void onTransactionCompleted(final AbstractProxyTransaction tx) { - Verify.verify(tx instanceof LocalProxyTransaction); + verify(tx instanceof LocalProxyTransaction, "Unexpected transaction %s", tx); if (tx instanceof LocalReadWriteProxyTransaction && LAST_SEALED_UPDATER.compareAndSet(this, (LocalReadWriteProxyTransaction) tx, null)) { LOG.debug("Completed last sealed transaction {}", tx); @@ -229,7 +230,7 @@ abstract class ProxyHistory implements Identifiable { final ConnectionEntry e = it.next(); final Request req = e.getRequest(); if (identifier.equals(req.getTarget())) { - Verify.verify(req instanceof LocalHistoryRequest); + verify(req instanceof LocalHistoryRequest, "Unexpected request %s", req); if (req instanceof CreateLocalHistoryRequest) { successor.connection.enqueueRequest(req, e.getCallback(), e.getEnqueuedTicks()); it.remove(); @@ -249,7 +250,7 @@ abstract class ProxyHistory implements Identifiable { final ConnectionEntry e = it.next(); final Request req = e.getRequest(); if (identifier.equals(req.getTarget())) { - Verify.verify(req instanceof LocalHistoryRequest); + verify(req instanceof LocalHistoryRequest, "Unexpected request %s", req); if (req instanceof DestroyLocalHistoryRequest) { successor.connection.enqueueRequest(req, e.getCallback(), e.getEnqueuedTicks()); it.remove(); @@ -259,10 +260,10 @@ abstract class ProxyHistory implements Identifiable { } } - @GuardedBy("lock") + @Holding("lock") @Override ProxyHistory finishReconnect() { - final ProxyHistory ret = Verify.verifyNotNull(successor); + final ProxyHistory ret = verifyNotNull(successor); for (AbstractProxyTransaction t : proxies.values()) { t.finishReconnect(); @@ -352,6 +353,7 @@ abstract class ProxyHistory implements Identifiable { } @Override + // Non-final for mocking public LocalHistoryIdentifier getIdentifier() { return identifier; } @@ -377,6 +379,7 @@ abstract class ProxyHistory implements Identifiable { return createTransactionProxy(txId, snapshotOnly, false); } + // Non-final for mocking AbstractProxyTransaction createTransactionProxy(final TransactionIdentifier txId, final boolean snapshotOnly, final boolean isDone) { lock.lock(); @@ -417,7 +420,7 @@ abstract class ProxyHistory implements Identifiable { } } - void purgeTransaction(final AbstractProxyTransaction tx) { + final void purgeTransaction(final AbstractProxyTransaction tx) { lock.lock(); try { proxies.remove(tx.getIdentifier()); @@ -462,7 +465,7 @@ abstract class ProxyHistory implements Identifiable { abstract ProxyHistory createSuccessor(AbstractClientConnection connection); @SuppressFBWarnings(value = "UL_UNRELEASED_LOCK", justification = "Lock is released asynchronously via the cohort") - ProxyReconnectCohort startReconnect(final ConnectedClientConnection newConnection) { + final ProxyReconnectCohort startReconnect(final ConnectedClientConnection newConnection) { lock.lock(); if (successor != null) { lock.unlock(); @@ -506,6 +509,7 @@ abstract class ProxyHistory implements Identifiable { // No-op for most implementations } + @Holding("lock") void onTransactionSealed(final AbstractProxyTransaction tx) { // No-op on most implementations } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java index b102e8c45b..22536cc50a 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java @@ -77,7 +77,7 @@ abstract class AbstractFrontendHistory implements Identifiable handleTransactionRequest(final TransactionRequest request, final RequestEnvelope envelope, final long now) throws RequestException { if (request instanceof TransactionPurgeRequest) { - return handleTransactionPurgeRequest(request, envelope, now); + return handleTransactionPurgeRequest((TransactionPurgeRequest) request, envelope, now); } final TransactionIdentifier id = request.getTarget(); @@ -117,7 +117,7 @@ abstract class AbstractFrontendHistory implements Identifiable handleTransactionPurgeRequest(final TransactionRequest request, + private TransactionPurgeResponse handleTransactionPurgeRequest(final TransactionPurgeRequest request, final RequestEnvelope envelope, final long now) { final TransactionIdentifier id = request.getTarget(); final long txidBits = id.getTransactionId(); @@ -203,8 +203,7 @@ abstract class AbstractFrontendHistory implements Identifiable> participatingShardNames); @Override - public String toString() { - return MoreObjects.toStringHelper(this).omitNullValues().add("identifier", getIdentifier()) - .add("persistenceId", persistenceId).add("transactions", transactions).toString(); + public final String toString() { + return MoreObjects.toStringHelper(this).omitNullValues() + .add("identifier", getIdentifier()) + .add("persistenceId", persistenceId) + .add("transactions", transactions) + .toString(); } } diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java index ba64bad55d..e086e51a66 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java @@ -14,6 +14,7 @@ import com.google.common.base.MoreObjects.ToStringHelper; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.controller.cluster.access.commands.CreateLocalHistoryRequest; import org.opendaylight.controller.cluster.access.commands.DeadHistoryException; @@ -99,7 +100,7 @@ abstract class LeaderFrontendState implements Identifiable { } else if (request instanceof DestroyLocalHistoryRequest) { return handleDestroyHistory((DestroyLocalHistoryRequest) request, envelope, now); } else if (request instanceof PurgeLocalHistoryRequest) { - return handlePurgeHistory((PurgeLocalHistoryRequest)request, envelope, now); + return handlePurgeHistory((PurgeLocalHistoryRequest) request, envelope, now); } else { LOG.warn("{}: rejecting unsupported request {}", persistenceId(), request); throw new UnsupportedRequestException(request); @@ -236,9 +237,9 @@ abstract class LeaderFrontendState implements Identifiable { private static final Logger LOG = LoggerFactory.getLogger(LeaderFrontendState.class); - private final ShardDataTree tree; - private final ClientIdentifier clientId; - private final String persistenceId; + private final @NonNull ClientIdentifier clientId; + private final @NonNull String persistenceId; + private final @NonNull ShardDataTree tree; private long lastConnectTicks; private long lastSeenTicks;