Minor sal-distributed-datastore cleanups 14/98314/3
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 7 Nov 2021 00:19:18 +0000 (01:19 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Sun, 7 Nov 2021 07:36:24 +0000 (08:36 +0100)
Corrent a @GuardedBy annotations, add @NonNull annotations, fix javadocs
and make a few methods final.

Change-Id: Icb93aae229fb0aece5ece223a759ce9f36ee0297
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHandle.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractClientHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ProxyHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/AbstractFrontendHistory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/LeaderFrontendState.java

index 90971b3..aa4779c 100644 (file)
@@ -55,6 +55,7 @@ public abstract class AbstractClientHandle<T extends AbstractProxyTransaction> e
     }
 
     @Override
+    // Non-final for mocking
     public TransactionIdentifier getIdentifier() {
         return transactionId;
     }
@@ -64,6 +65,7 @@ public abstract class AbstractClientHandle<T extends AbstractProxyTransaction> e
      *
      * @return True if this transaction became closed during this call
      */
+    // Non-final for mocking
     public boolean abort() {
         if (commonAbort()) {
             parent.onTransactionAbort(this);
index d445d3c..d306d13 100644 (file)
@@ -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<ShardBackendInfo> newConn) {
+    final HistoryReconnectCohort startReconnect(final ConnectedClientConnection<ShardBackendInfo> 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
             }
         };
     }
-
 }
index 9b4c5a9..f5855c2 100644 (file)
@@ -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<AbstractProxyTransac
 
     public DOMStoreThreePhaseCommitCohort ready() {
         final Collection<AbstractProxyTransaction> 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<AbstractProxyTransac
                 cohort = new EmptyTransactionCommitCohort(parent(), getIdentifier());
                 break;
             case 1:
-                cohort = new DirectTransactionCommitCohort(parent(), getIdentifier(),
-                    Iterables.getOnlyElement(toReady));
+                cohort = new DirectTransactionCommitCohort(parent(), getIdentifier(), toReady.iterator().next());
                 break;
             default:
                 cohort = new ClientTransactionCommitCohort(parent(), getIdentifier(), toReady);
index 92284e0..fb87640 100644 (file)
@@ -8,10 +8,11 @@
 package org.opendaylight.controller.cluster.databroker.actors.dds;
 
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.base.Verify.verify;
+import static com.google.common.base.Verify.verifyNotNull;
 import static java.util.Objects.requireNonNull;
 
 import akka.actor.ActorRef;
-import com.google.common.base.Verify;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.Iterator;
@@ -130,7 +131,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
 
         @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<LocalHistoryIdentifier> {
                 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<LocalHistoryIdentifier> {
                 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<LocalHistoryIdentifier> {
             }
         }
 
-        @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<LocalHistoryIdentifier> {
     }
 
     @Override
+    // Non-final for mocking
     public LocalHistoryIdentifier getIdentifier() {
         return identifier;
     }
@@ -377,6 +379,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         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<LocalHistoryIdentifier> {
         }
     }
 
-    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<LocalHistoryIdentifier> {
     abstract ProxyHistory createSuccessor(AbstractClientConnection<ShardBackendInfo> connection);
 
     @SuppressFBWarnings(value = "UL_UNRELEASED_LOCK", justification = "Lock is released asynchronously via the cohort")
-    ProxyReconnectCohort startReconnect(final ConnectedClientConnection<ShardBackendInfo> newConnection) {
+    final ProxyReconnectCohort startReconnect(final ConnectedClientConnection<ShardBackendInfo> newConnection) {
         lock.lock();
         if (successor != null) {
             lock.unlock();
@@ -506,6 +509,7 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         // No-op for most implementations
     }
 
+    @Holding("lock")
     void onTransactionSealed(final AbstractProxyTransaction tx) {
         // No-op on most implementations
     }
index b102e8c..22536cc 100644 (file)
@@ -77,7 +77,7 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
     final @Nullable TransactionSuccess<?> 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<LocalHistoryIdent
         return tx.handleRequest(request, envelope, now);
     }
 
-    private TransactionSuccess<?> 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<LocalHistoryIdent
 
     abstract FrontendTransaction createOpenTransaction(TransactionIdentifier id);
 
-    abstract FrontendTransaction createReadyTransaction(TransactionIdentifier id, DataTreeModification mod)
-        ;
+    abstract FrontendTransaction createReadyTransaction(TransactionIdentifier id, DataTreeModification mod);
 
     abstract ShardDataTreeCohort createFailedCohort(TransactionIdentifier id, DataTreeModification mod,
             Exception failure);
@@ -213,8 +212,11 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
             Optional<SortedSet<String>> 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();
     }
 }
index ba64bad..e086e51 100644 (file)
@@ -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<ClientIdentifier> {
                 } 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<ClientIdentifier> {
 
     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;