Fix sonar warnings in sal-distributed-datastore 63/63363/6
authorTom Pantelis <tompantelis@gmail.com>
Wed, 20 Sep 2017 18:00:31 +0000 (14:00 -0400)
committerRobert Varga <nite@hq.sk>
Wed, 11 Oct 2017 14:11:41 +0000 (14:11 +0000)
These come from squid:

- String literals should not be duplicated
- Modifiers should be declared in the correct order
- Lambdas and anonymous classes should not have too many lines
- Nested blocks of code should not be left empty
- Local variables should not shadow class fields
- Exception handlers should preserve the original exception
- Utility classes should not have public constructors
- Overriding methods should do more than simply call the same method in the super class
- Unused private fields should be removed

I fixed quite a few of them. Others we'd have to suppress or modify the sonar
config to be more lenient.

Change-Id: I7ce7b2a05feac9844fd9c37927de82b7b8b68ee5
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
55 files changed:
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/AbstractDOMBroker.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/ClientBackedTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/ConcurrentDOMDataBroker.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractDataStoreClientBehavior.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/AbstractShardBackendResolver.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/ClientTransactionCursor.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadOnlyProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/actors/dds/LocalReadWriteProxyTransaction.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/databroker/actors/dds/RemoteProxyTransaction.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/AbstractShardDataTreeTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DatastoreContextConfigAdminOverlay.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DatastoreContextIntrospector.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DefaultShardDataChangeListenerPublisher.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DefaultShardDataTreeChangeListenerPublisher.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreFactory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendReadWriteTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/RemoteTransactionContextSupport.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTreeMetadata.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/config/ConfigurationImpl.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/config/PrefixShardConfiguration.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/AbstractEntityOwnerChangeListener.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/CandidateListChangeListener.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnersModel.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/selectionstrategy/EntityOwnerSelectionStrategyConfigReader.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardMBeanFactory.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/ActorInitialized.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/DataChanged.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/DataChangedReply.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/LocalPrimaryShardFound.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/PrimaryShardInfo.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/ShardLeaderStateChanged.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/SuccessReply.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/AbstractVersionException.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/DataTreeCandidateInputOutput.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/DatastoreSnapshot.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/ShardDataTreeSnapshot.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/ShardDataTreeSnapshotMetadata.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/persisted/ShardManagerSnapshot.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardManager.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/shardmanager/ShardManagerSnapshot.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/utils/ClusterUtils.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/utils/CompositeOnComplete.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/utils/PrimaryShardInfoFutureCache.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/sharding/DOMDataTreeShardCreationFailedException.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/sharding/DistributedShardChangePublisher.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/sharding/ShardProxyProducer.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/sharding/ShardProxyTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/sharding/ShardedDataTreeActor.java

index 68191fb..6e6ddf6 100644 (file)
@@ -54,9 +54,7 @@ abstract class AbstractDOMBroker extends AbstractDOMTransactionFactory<DOMStore>
                 @Override
                 public <L extends DOMDataTreeChangeListener> ListenerRegistration<L> registerDataTreeChangeListener(
                         final DOMDataTreeIdentifier treeId, final L listener) {
-                    DOMStore store = getTxFactories().get(treeId.getDatastoreType());
-                    checkState(store != null, "Requested logical data store is not available.");
-
+                    DOMStore store = getDOMStore(treeId.getDatastoreType());
                     return ((DOMStoreTreeChangePublisher) store).registerTreeChangeListener(
                             treeId.getRootIdentifier(), listener);
                 }
@@ -68,9 +66,7 @@ abstract class AbstractDOMBroker extends AbstractDOMTransactionFactory<DOMStore>
                 @Override
                 public <T extends DOMDataTreeCommitCohort> DOMDataTreeCommitCohortRegistration<T> registerCommitCohort(
                         org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier path, T cohort) {
-                    DOMStore store = getTxFactories().get(toLegacy(path.getDatastoreType()));
-                    checkState(store != null, "Requested logical data store is not available.");
-
+                    DOMStore store = getDOMStore(toLegacy(path.getDatastoreType()));
                     return ((org.opendaylight.mdsal.dom.api.DOMDataTreeCommitCohortRegistry) store)
                             .registerCommitCohort(path, cohort);
                 }
@@ -129,8 +125,7 @@ abstract class AbstractDOMBroker extends AbstractDOMTransactionFactory<DOMStore>
     public ListenerRegistration<DOMDataChangeListener> registerDataChangeListener(final LogicalDatastoreType store,
             final YangInstanceIdentifier path, final DOMDataChangeListener listener,
             final DataChangeScope triggeringScope) {
-        DOMStore potentialStore = getTxFactories().get(store);
-        checkState(potentialStore != null, "Requested logical data store is not available.");
+        DOMStore potentialStore = getDOMStore(store);
         return potentialStore.registerChangeListener(path, listener, triggeringScope);
     }
 
@@ -154,4 +149,10 @@ abstract class AbstractDOMBroker extends AbstractDOMTransactionFactory<DOMStore>
                 backingChains);
         return new DOMBrokerTransactionChain(chainId, backingChains, this, listener);
     }
+
+    private DOMStore getDOMStore(final LogicalDatastoreType type) {
+        DOMStore store = getTxFactories().get(type);
+        checkState(store != null, "Requested logical data store is not available.");
+        return store;
+    }
 }
index 5b6a451..4d979cd 100644 (file)
@@ -45,7 +45,8 @@ abstract class ClientBackedTransaction<T extends AbstractClientHandle<?>> extend
             this.allocationContext = allocationContext;
         }
 
-        static @Nonnull <T extends AbstractClientHandle<?>> T recordTransaction(
+        @Nonnull
+        static <T extends AbstractClientHandle<?>> T recordTransaction(
                 @Nonnull final ClientBackedTransaction<T> referent, @Nonnull final T transaction,
                 @Nullable final Throwable allocationContext) {
             FINALIZERS.add(new Finalizer(referent, transaction, allocationContext));
index 71cd2dc..7e92787 100644 (file)
@@ -7,6 +7,10 @@
  */
 package org.opendaylight.controller.cluster.databroker;
 
+import static org.opendaylight.controller.md.sal.dom.broker.impl.TransactionCommitFailedExceptionMapper.CAN_COMMIT_ERROR_MAPPER;
+import static org.opendaylight.controller.md.sal.dom.broker.impl.TransactionCommitFailedExceptionMapper.COMMIT_ERROR_MAPPER;
+import static org.opendaylight.controller.md.sal.dom.broker.impl.TransactionCommitFailedExceptionMapper.PRE_COMMIT_MAPPER;
+
 import com.google.common.annotations.Beta;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.AbstractFuture;
@@ -89,8 +93,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
 
         doCanCommit(clientSubmitFuture, transaction, cohorts);
 
-        return MappingCheckedFuture.create(clientSubmitFuture,
-                TransactionCommitFailedExceptionMapper.COMMIT_ERROR_MAPPER);
+        return MappingCheckedFuture.create(clientSubmitFuture, COMMIT_ERROR_MAPPER);
     }
 
     private void doCanCommit(final AsyncNotifyingSettableFuture clientSubmitFuture,
@@ -106,25 +109,19 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
             @Override
             public void onSuccess(Boolean result) {
                 if (result == null || !result) {
-                    handleException(clientSubmitFuture, transaction, cohorts,
-                            CAN_COMMIT, TransactionCommitFailedExceptionMapper.CAN_COMMIT_ERROR_MAPPER,
-                            new TransactionCommitFailedException(
-                                            "Can Commit failed, no detailed cause available."));
+                    handleException(clientSubmitFuture, transaction, cohorts, CAN_COMMIT, CAN_COMMIT_ERROR_MAPPER,
+                            new TransactionCommitFailedException("Can Commit failed, no detailed cause available."));
+                } else if (!cohortIterator.hasNext()) {
+                    // All cohorts completed successfully - we can move on to the preCommit phase
+                    doPreCommit(startTime, clientSubmitFuture, transaction, cohorts);
                 } else {
-                    if (!cohortIterator.hasNext()) {
-                        // All cohorts completed successfully - we can move on to the preCommit phase
-                        doPreCommit(startTime, clientSubmitFuture, transaction, cohorts);
-                    } else {
-                        ListenableFuture<Boolean> canCommitFuture = cohortIterator.next().canCommit();
-                        Futures.addCallback(canCommitFuture, this, MoreExecutors.directExecutor());
-                    }
+                    Futures.addCallback(cohortIterator.next().canCommit(), this, MoreExecutors.directExecutor());
                 }
             }
 
             @Override
             public void onFailure(Throwable failure) {
-                handleException(clientSubmitFuture, transaction, cohorts, CAN_COMMIT,
-                        TransactionCommitFailedExceptionMapper.CAN_COMMIT_ERROR_MAPPER, failure);
+                handleException(clientSubmitFuture, transaction, cohorts, CAN_COMMIT, CAN_COMMIT_ERROR_MAPPER, failure);
             }
         };
 
@@ -153,8 +150,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
 
             @Override
             public void onFailure(Throwable failure) {
-                handleException(clientSubmitFuture, transaction, cohorts, PRE_COMMIT,
-                        TransactionCommitFailedExceptionMapper.PRE_COMMIT_MAPPER, failure);
+                handleException(clientSubmitFuture, transaction, cohorts, PRE_COMMIT, PRE_COMMIT_MAPPER, failure);
             }
         };
 
@@ -185,8 +181,7 @@ public class ConcurrentDOMDataBroker extends AbstractDOMBroker {
 
             @Override
             public void onFailure(Throwable throwable) {
-                handleException(clientSubmitFuture, transaction, cohorts, COMMIT,
-                        TransactionCommitFailedExceptionMapper.COMMIT_ERROR_MAPPER, throwable);
+                handleException(clientSubmitFuture, transaction, cohorts, COMMIT, COMMIT_ERROR_MAPPER, throwable);
             }
         };
 
index 007f387..77db4e6 100644 (file)
@@ -21,6 +21,8 @@ import org.opendaylight.controller.cluster.access.client.BackendInfoResolver;
 import org.opendaylight.controller.cluster.access.client.ClientActorBehavior;
 import org.opendaylight.controller.cluster.access.client.ClientActorContext;
 import org.opendaylight.controller.cluster.access.client.ConnectedClientConnection;
+import org.opendaylight.controller.cluster.access.client.ConnectionEntry;
+import org.opendaylight.controller.cluster.access.client.ReconnectForwarder;
 import org.opendaylight.controller.cluster.access.concepts.LocalHistoryIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.slf4j.Logger;
@@ -139,29 +141,33 @@ abstract class AbstractDataStoreClientBehavior extends ClientActorBehavior<Shard
             startReconnect(h, newConn, cohorts);
         }
 
-        return previousEntries -> {
+        return previousEntries -> finishReconnect(newConn, stamp, cohorts, previousEntries);
+    }
+
+    private ReconnectForwarder finishReconnect(final ConnectedClientConnection<ShardBackendInfo> newConn,
+            final long stamp, final Collection<HistoryReconnectCohort> cohorts,
+            final Collection<ConnectionEntry> previousEntries) {
+        try {
+            // Step 2: Collect previous successful requests from the cohorts. We do not want to expose
+            //         the non-throttling interface to the connection, hence we use a wrapper consumer
+            for (HistoryReconnectCohort c : cohorts) {
+                c.replayRequests(previousEntries);
+            }
+
+            // Step 3: Install a forwarder, which will forward requests back to affected cohorts. Any outstanding
+            //         requests will be immediately sent to it and requests being sent concurrently will get
+            //         forwarded once they hit the new connection.
+            return BouncingReconnectForwarder.forCohorts(newConn, cohorts);
+        } finally {
             try {
-                // Step 2: Collect previous successful requests from the cohorts. We do not want to expose
-                //         the non-throttling interface to the connection, hence we use a wrapper consumer
+                // Step 4: Complete switchover of the connection. The cohorts can resume normal operations.
                 for (HistoryReconnectCohort c : cohorts) {
-                    c.replayRequests(previousEntries);
+                    c.close();
                 }
-
-                // Step 3: Install a forwarder, which will forward requests back to affected cohorts. Any outstanding
-                //         requests will be immediately sent to it and requests being sent concurrently will get
-                //         forwarded once they hit the new connection.
-                return BouncingReconnectForwarder.forCohorts(newConn, cohorts);
             } finally {
-                try {
-                    // Step 4: Complete switchover of the connection. The cohorts can resume normal operations.
-                    for (HistoryReconnectCohort c : cohorts) {
-                        c.close();
-                    }
-                } finally {
-                    lock.unlockWrite(stamp);
-                }
+                lock.unlockWrite(stamp);
             }
-        };
+        }
     }
 
     private static void startReconnect(final AbstractClientHistory history,
index 51f5281..8e09411 100644 (file)
@@ -413,7 +413,7 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
         }
     }
 
-    final void recordSuccessfulRequest(final @Nonnull TransactionRequest<?> req) {
+    final void recordSuccessfulRequest(@Nonnull final TransactionRequest<?> req) {
         successfulRequests.add(Verify.verifyNotNull(req));
     }
 
@@ -449,7 +449,7 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
             } else if (t instanceof RequestFailure) {
                 ret.voteNo(((RequestFailure<?, ?>) t).getCause().unwrap());
             } else {
-                ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
+                ret.voteNo(unhandledResponseException(t));
             }
 
             // This is a terminal request, hence we do not need to record it
@@ -507,7 +507,7 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
                             ret.setException(cause);
                         }
                     } else {
-                        ret.setException(new IllegalStateException("Unhandled response " + t.getClass()));
+                        ret.setException(unhandledResponseException(t));
                     }
 
                     // This is a terminal request, hence we do not need to record it
@@ -538,7 +538,7 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
                     } else if (t instanceof RequestFailure) {
                         ret.voteNo(((RequestFailure<?, ?>) t).getCause().unwrap());
                     } else {
-                        ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
+                        ret.voteNo(unhandledResponseException(t));
                     }
 
                     recordSuccessfulRequest(req);
@@ -569,7 +569,7 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
             } else if (t instanceof RequestFailure) {
                 ret.voteNo(((RequestFailure<?, ?>) t).getCause().unwrap());
             } else {
-                ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
+                ret.voteNo(unhandledResponseException(t));
             }
 
             onPreCommitComplete(req);
@@ -602,7 +602,7 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
             } else if (t instanceof RequestFailure) {
                 ret.voteNo(((RequestFailure<?, ?>) t).getCause().unwrap());
             } else {
-                ret.voteNo(new IllegalStateException("Unhandled response " + t.getClass()));
+                ret.voteNo(unhandledResponseException(t));
             }
 
             LOG.debug("Transaction {} doCommit completed", this);
@@ -687,13 +687,13 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
             for (Object obj : successfulRequests) {
                 if (obj instanceof TransactionRequest) {
                     LOG.debug("Forwarding successful request {} to successor {}", obj, successor);
-                    successor.doReplayRequest((TransactionRequest<?>) obj, resp -> { }, now);
+                    successor.doReplayRequest((TransactionRequest<?>) obj, resp -> { /*NOOP*/ }, now);
                 } else {
                     Verify.verify(obj instanceof IncrementSequence);
                     final IncrementSequence increment = (IncrementSequence) obj;
                     successor.doReplayRequest(new IncrementTransactionSequenceRequest(getIdentifier(),
-                        increment.getSequence(), localActor(), isSnapshotOnly(), increment.getDelta()), resp -> { },
-                        now);
+                        increment.getSequence(), localActor(), isSnapshotOnly(),
+                        increment.getDelta()), resp -> { /*NOOP*/ }, now);
                     LOG.debug("Incrementing sequence {} to successor {}", obj, successor);
                 }
             }
@@ -846,6 +846,10 @@ abstract class AbstractProxyTransaction implements Identifiable<TransactionIdent
     abstract void handleReplayedRemoteRequest(TransactionRequest<?> request,
             @Nullable Consumer<Response<?, ?>> callback, long enqueuedTicks);
 
+    private static IllegalStateException unhandledResponseException(Response<?, ?> resp) {
+        return new IllegalStateException("Unhandled response " + resp.getClass());
+    }
+
     @Override
     public final String toString() {
         return MoreObjects.toStringHelper(this).add("identifier", getIdentifier()).add("state", state).toString();
index 46e035d..0ce4df0 100644 (file)
@@ -137,27 +137,32 @@ abstract class AbstractShardBackendResolver extends BackendInfoResolver<ShardBac
 
         FutureConverters.toJava(ExplicitAsk.ask(info.getPrimaryShardActor(), connectFunction, CONNECT_TIMEOUT))
             .whenComplete((response, failure) -> {
-                if (failure != null) {
-                    LOG.debug("Connect attempt to {} failed, will retry", shardName, failure);
-                    future.completeExceptionally(wrap("Connection attempt failed", failure));
-                    return;
-                }
-                if (response instanceof RequestFailure) {
-                    final Throwable cause = ((RequestFailure<?, ?>) response).getCause().unwrap();
-                    LOG.debug("Connect attempt to {} failed to process", shardName, cause);
-                    final Throwable result = cause instanceof NotLeaderException
-                            ? wrap("Leader moved during establishment", cause) : cause;
-                    future.completeExceptionally(result);
-                    return;
-                }
-
-                LOG.debug("Resolved backend information to {}", response);
-                Preconditions.checkArgument(response instanceof ConnectClientSuccess, "Unhandled response %s",
-                    response);
-                final ConnectClientSuccess success = (ConnectClientSuccess) response;
-                future.complete(new ShardBackendInfo(success.getBackend(), nextSessionId.getAndIncrement(),
-                    success.getVersion(), shardName, UnsignedLong.fromLongBits(cookie), success.getDataTree(),
-                    success.getMaxMessages()));
+                onConnectResponse(shardName, cookie, future, response, failure);
             });
     }
+
+    private void onConnectResponse(final String shardName, final long cookie,
+            final CompletableFuture<ShardBackendInfo> future, final Object response, final Throwable failure) {
+        if (failure != null) {
+            LOG.debug("Connect attempt to {} failed, will retry", shardName, failure);
+            future.completeExceptionally(wrap("Connection attempt failed", failure));
+            return;
+        }
+        if (response instanceof RequestFailure) {
+            final Throwable cause = ((RequestFailure<?, ?>) response).getCause().unwrap();
+            LOG.debug("Connect attempt to {} failed to process", shardName, cause);
+            final Throwable result = cause instanceof NotLeaderException
+                    ? wrap("Leader moved during establishment", cause) : cause;
+            future.completeExceptionally(result);
+            return;
+        }
+
+        LOG.debug("Resolved backend information to {}", response);
+        Preconditions.checkArgument(response instanceof ConnectClientSuccess, "Unhandled response %s",
+            response);
+        final ConnectClientSuccess success = (ConnectClientSuccess) response;
+        future.complete(new ShardBackendInfo(success.getBackend(), nextSessionId.getAndIncrement(),
+            success.getVersion(), shardName, UnsignedLong.fromLongBits(cookie), success.getDataTree(),
+            success.getMaxMessages()));
+    }
 }
index bbd8534..41d2cb8 100644 (file)
@@ -44,9 +44,9 @@ final class ClientTransactionCursor implements DOMDataTreeWriteCursor {
 
     @Override
     public void exit() {
-        final YangInstanceIdentifier parent = current.getParent();
-        Preconditions.checkState(parent != null);
-        current = parent;
+        final YangInstanceIdentifier currentParent = current.getParent();
+        Preconditions.checkState(currentParent != null);
+        current = currentParent;
     }
 
     @Override
index f34abff..b6e9bfa 100644 (file)
@@ -69,7 +69,8 @@ abstract class LocalProxyTransaction extends AbstractProxyTransaction {
         return identifier;
     }
 
-    abstract @Nonnull DataTreeSnapshot readOnlyView();
+    @Nonnull
+    abstract DataTreeSnapshot readOnlyView();
 
     abstract void applyForwardedModifyTransactionRequest(ModifyTransactionRequest request,
             @Nullable Consumer<Response<?, ?>> callback);
@@ -103,7 +104,7 @@ abstract class LocalProxyTransaction extends AbstractProxyTransaction {
     }
 
     private boolean handleReadRequest(final TransactionRequest<?> request,
-            final @Nullable Consumer<Response<?, ?>> callback) {
+            @Nullable final Consumer<Response<?, ?>> callback) {
         // Note we delay completion of read requests to limit the scope at which the client can run, as they have
         // listeners, which we do not want to execute while we are reconnecting.
         if (request instanceof ReadTransactionRequest) {
@@ -133,7 +134,7 @@ abstract class LocalProxyTransaction extends AbstractProxyTransaction {
 
     @Override
     void handleReplayedRemoteRequest(final TransactionRequest<?> request,
-            final @Nullable Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
+            @Nullable final Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
         if (request instanceof ModifyTransactionRequest) {
             replayModifyTransactionRequest((ModifyTransactionRequest) request, callback, enqueuedTicks);
         } else if (handleReadRequest(request, callback)) {
@@ -206,7 +207,7 @@ abstract class LocalProxyTransaction extends AbstractProxyTransaction {
         } else if (request instanceof ModifyTransactionRequest) {
             successor.handleForwardedRequest(request, callback);
         } else {
-            throw new IllegalArgumentException("Unhandled request" + request);
+            throwUnhandledRequest(request);
         }
     }
 
@@ -218,12 +219,16 @@ abstract class LocalProxyTransaction extends AbstractProxyTransaction {
         } else if (request instanceof TransactionPurgeRequest) {
             successor.enqueuePurge(callback);
         } else {
-            throw new IllegalArgumentException("Unhandled request" + request);
+            throwUnhandledRequest(request);
         }
 
         LOG.debug("Forwarded request {} to successor {}", request, successor);
     }
 
+    private static void throwUnhandledRequest(final TransactionRequest<?> request) {
+        throw new IllegalArgumentException("Unhandled request" + request);
+    }
+
     void sendAbort(final TransactionRequest<?> request, final Consumer<Response<?, ?>> callback) {
         sendRequest(request, callback);
     }
index 00e2940..ee5889d 100644 (file)
@@ -55,22 +55,22 @@ final class LocalReadOnlyProxyTransaction extends LocalProxyTransaction {
 
     @Override
     void doDelete(final YangInstanceIdentifier path) {
-        throw new UnsupportedOperationException("Read-only snapshot");
+        throw new UnsupportedOperationException("doDelete");
     }
 
     @Override
     void doMerge(final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
-        throw new UnsupportedOperationException("Read-only snapshot");
+        throw new UnsupportedOperationException("doMerge");
     }
 
     @Override
     void doWrite(final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
-        throw new UnsupportedOperationException("Read-only snapshot");
+        throw new UnsupportedOperationException("doWrite");
     }
 
     @Override
     CommitLocalTransactionRequest commitRequest(final boolean coordinated) {
-        throw new UnsupportedOperationException("Read-only snapshot");
+        throw new UnsupportedOperationException("commitRequest");
     }
 
     @Override
index 76ad672..c4db9d8 100644 (file)
@@ -228,18 +228,18 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
 
     @Override
     void applyForwardedModifyTransactionRequest(final ModifyTransactionRequest request,
-            final @Nullable Consumer<Response<?, ?>> callback) {
+            @Nullable final Consumer<Response<?, ?>> callback) {
         commonModifyTransactionRequest(request, callback, this::sendRequest);
     }
 
     @Override
     void replayModifyTransactionRequest(final ModifyTransactionRequest request,
-            final @Nullable Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
+            @Nullable final Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
         commonModifyTransactionRequest(request, callback, (req, cb) -> enqueueRequest(req, cb, enqueuedTicks));
     }
 
     private void commonModifyTransactionRequest(final ModifyTransactionRequest request,
-            final @Nullable Consumer<Response<?, ?>> callback,
+            @Nullable final Consumer<Response<?, ?>> callback,
             final BiConsumer<TransactionRequest<?>, Consumer<Response<?, ?>>> sendMethod) {
         for (final TransactionModification mod : request.getModifications()) {
             if (mod instanceof TransactionWrite) {
@@ -291,7 +291,7 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
 
     @Override
     void handleReplayedRemoteRequest(final TransactionRequest<?> request,
-            final @Nullable Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
+            @Nullable final Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
         LOG.debug("Applying replayed request {}", request);
 
         if (request instanceof TransactionPreCommitRequest) {
@@ -347,7 +347,8 @@ final class LocalReadWriteProxyTransaction extends LocalProxyTransaction {
         closedException = this::abortedException;
     }
 
-    private @Nonnull CursorAwareDataTreeModification getModification() {
+    @Nonnull
+    private CursorAwareDataTreeModification getModification() {
         if (closedException != null) {
             throw closedException.get();
         }
index e26e00f..61b45ed 100644 (file)
@@ -127,10 +127,9 @@ abstract class ProxyHistory implements Identifiable<LocalHistoryIdentifier> {
         @Override
         void onTransactionCompleted(final AbstractProxyTransaction tx) {
             Verify.verify(tx instanceof LocalProxyTransaction);
-            if (tx instanceof LocalReadWriteProxyTransaction) {
-                if (LAST_SEALED_UPDATER.compareAndSet(this, (LocalReadWriteProxyTransaction) tx, null)) {
-                    LOG.debug("Completed last sealed transaction {}", tx);
-                }
+            if (tx instanceof LocalReadWriteProxyTransaction
+                    && LAST_SEALED_UPDATER.compareAndSet(this, (LocalReadWriteProxyTransaction) tx, null)) {
+                LOG.debug("Completed last sealed transaction {}", tx);
             }
         }
 
index 5a6b539..3120f6f 100644 (file)
@@ -295,52 +295,7 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
 
     void handleForwardedRequest(final TransactionRequest<?> request, final Consumer<Response<?, ?>> callback) {
         if (request instanceof ModifyTransactionRequest) {
-            final ModifyTransactionRequest req = (ModifyTransactionRequest) request;
-
-            req.getModifications().forEach(this::appendModification);
-
-            final java.util.Optional<PersistenceProtocol> maybeProto = req.getPersistenceProtocol();
-            if (maybeProto.isPresent()) {
-                // Persistence protocol implies we are sealed, propagate the marker, but hold off doing other actions
-                // until we know what we are going to do.
-                if (markSealed()) {
-                    sealOnly();
-                }
-
-                final TransactionRequest<?> tmp;
-                switch (maybeProto.get()) {
-                    case ABORT:
-                        tmp = abortRequest();
-                        sendRequest(tmp, resp -> {
-                            completeModify(tmp, resp);
-                            callback.accept(resp);
-                        });
-                        break;
-                    case SIMPLE:
-                        tmp = commitRequest(false);
-                        sendRequest(tmp, resp -> {
-                            completeModify(tmp, resp);
-                            callback.accept(resp);
-                        });
-                        break;
-                    case THREE_PHASE:
-                        tmp = commitRequest(true);
-                        sendRequest(tmp, resp -> {
-                            recordSuccessfulRequest(tmp);
-                            callback.accept(resp);
-                        });
-                        break;
-                    case READY:
-                        tmp = readyRequest();
-                        sendRequest(tmp, resp -> {
-                            recordSuccessfulRequest(tmp);
-                            callback.accept(resp);
-                        });
-                        break;
-                    default:
-                        throw new IllegalArgumentException("Unhandled protocol " + maybeProto.get());
-                }
-            }
+            handleForwardedModifyTransactionRequest(callback, (ModifyTransactionRequest) request);
         } else if (request instanceof ReadTransactionRequest) {
             ensureFlushedBuider();
             sendRequest(new ReadTransactionRequest(getIdentifier(), nextSequence(), localActor(),
@@ -376,6 +331,54 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
         }
     }
 
+    private void handleForwardedModifyTransactionRequest(final Consumer<Response<?, ?>> callback,
+            final ModifyTransactionRequest req) {
+        req.getModifications().forEach(this::appendModification);
+
+        final java.util.Optional<PersistenceProtocol> maybeProto = req.getPersistenceProtocol();
+        if (maybeProto.isPresent()) {
+            // Persistence protocol implies we are sealed, propagate the marker, but hold off doing other actions
+            // until we know what we are going to do.
+            if (markSealed()) {
+                sealOnly();
+            }
+
+            final TransactionRequest<?> tmp;
+            switch (maybeProto.get()) {
+                case ABORT:
+                    tmp = abortRequest();
+                    sendRequest(tmp, resp -> {
+                        completeModify(tmp, resp);
+                        callback.accept(resp);
+                    });
+                    break;
+                case SIMPLE:
+                    tmp = commitRequest(false);
+                    sendRequest(tmp, resp -> {
+                        completeModify(tmp, resp);
+                        callback.accept(resp);
+                    });
+                    break;
+                case THREE_PHASE:
+                    tmp = commitRequest(true);
+                    sendRequest(tmp, resp -> {
+                        recordSuccessfulRequest(tmp);
+                        callback.accept(resp);
+                    });
+                    break;
+                case READY:
+                    tmp = readyRequest();
+                    sendRequest(tmp, resp -> {
+                        recordSuccessfulRequest(tmp);
+                        callback.accept(resp);
+                    });
+                    break;
+                default:
+                    throw new IllegalArgumentException("Unhandled protocol " + maybeProto.get());
+            }
+        }
+    }
+
     @Override
     void forwardToLocal(final LocalProxyTransaction successor, final TransactionRequest<?> request,
             final Consumer<Response<?, ?>> callback) {
@@ -421,58 +424,12 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
 
     @Override
     void handleReplayedRemoteRequest(final TransactionRequest<?> request,
-            final @Nullable Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
-        final Consumer<Response<?, ?>> cb = callback != null ? callback : resp -> { };
+            @Nullable final Consumer<Response<?, ?>> callback, final long enqueuedTicks) {
+        final Consumer<Response<?, ?>> cb = callback != null ? callback : resp -> { /* NOOP */ };
         final Optional<Long> optTicks = Optional.of(Long.valueOf(enqueuedTicks));
 
         if (request instanceof ModifyTransactionRequest) {
-            final ModifyTransactionRequest req = (ModifyTransactionRequest) request;
-            for (TransactionModification mod : req.getModifications()) {
-                appendModification(mod, optTicks);
-            }
-
-            final java.util.Optional<PersistenceProtocol> maybeProto = req.getPersistenceProtocol();
-            if (maybeProto.isPresent()) {
-                // Persistence protocol implies we are sealed, propagate the marker, but hold off doing other actions
-                // until we know what we are going to do.
-                if (markSealed()) {
-                    sealOnly();
-                }
-
-                final TransactionRequest<?> tmp;
-                switch (maybeProto.get()) {
-                    case ABORT:
-                        tmp = abortRequest();
-                        enqueueRequest(tmp, resp -> {
-                            completeModify(tmp, resp);
-                            cb.accept(resp);
-                        }, enqueuedTicks);
-                        break;
-                    case SIMPLE:
-                        tmp = commitRequest(false);
-                        enqueueRequest(tmp, resp -> {
-                            completeModify(tmp, resp);
-                            cb.accept(resp);
-                        }, enqueuedTicks);
-                        break;
-                    case THREE_PHASE:
-                        tmp = commitRequest(true);
-                        enqueueRequest(tmp, resp -> {
-                            recordSuccessfulRequest(tmp);
-                            cb.accept(resp);
-                        }, enqueuedTicks);
-                        break;
-                    case READY:
-                        tmp = readyRequest();
-                        enqueueRequest(tmp, resp -> {
-                            recordSuccessfulRequest(tmp);
-                            cb.accept(resp);
-                        }, enqueuedTicks);
-                        break;
-                    default:
-                        throw new IllegalArgumentException("Unhandled protocol " + maybeProto.get());
-                }
-            }
+            handleReplayedModifyTransactionRequest(enqueuedTicks, cb, (ModifyTransactionRequest) request);
         } else if (request instanceof ReadTransactionRequest) {
             ensureFlushedBuider(optTicks);
             enqueueRequest(new ReadTransactionRequest(getIdentifier(), nextSequence(), localActor(),
@@ -514,4 +471,52 @@ final class RemoteProxyTransaction extends AbstractProxyTransaction {
             throw new IllegalArgumentException("Unhandled request {}" + request);
         }
     }
+
+    private void handleReplayedModifyTransactionRequest(final long enqueuedTicks, final Consumer<Response<?, ?>> cb,
+            final ModifyTransactionRequest req) {
+        req.getModifications().forEach(this::appendModification);
+
+        final java.util.Optional<PersistenceProtocol> maybeProto = req.getPersistenceProtocol();
+        if (maybeProto.isPresent()) {
+            // Persistence protocol implies we are sealed, propagate the marker, but hold off doing other actions
+            // until we know what we are going to do.
+            if (markSealed()) {
+                sealOnly();
+            }
+
+            final TransactionRequest<?> tmp;
+            switch (maybeProto.get()) {
+                case ABORT:
+                    tmp = abortRequest();
+                    enqueueRequest(tmp, resp -> {
+                        completeModify(tmp, resp);
+                        cb.accept(resp);
+                    }, enqueuedTicks);
+                    break;
+                case SIMPLE:
+                    tmp = commitRequest(false);
+                    enqueueRequest(tmp, resp -> {
+                        completeModify(tmp, resp);
+                        cb.accept(resp);
+                    }, enqueuedTicks);
+                    break;
+                case THREE_PHASE:
+                    tmp = commitRequest(true);
+                    enqueueRequest(tmp, resp -> {
+                        recordSuccessfulRequest(tmp);
+                        cb.accept(resp);
+                    }, enqueuedTicks);
+                    break;
+                case READY:
+                    tmp = readyRequest();
+                    enqueueRequest(tmp, resp -> {
+                        recordSuccessfulRequest(tmp);
+                        cb.accept(resp);
+                    }, enqueuedTicks);
+                    break;
+                default:
+                    throw new IllegalArgumentException("Unhandled protocol " + maybeProto.get());
+            }
+        }
+    }
 }
index c1d2db3..89dd59a 100644 (file)
@@ -73,53 +73,15 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
         return tree.readTime();
     }
 
-    final @Nullable TransactionSuccess<?> handleTransactionRequest(final TransactionRequest<?> request,
+    @Nullable
+    final TransactionSuccess<?> handleTransactionRequest(final TransactionRequest<?> request,
             final RequestEnvelope envelope, final long now) throws RequestException {
-        final TransactionIdentifier id = request.getTarget();
-        final UnsignedLong ul = UnsignedLong.fromLongBits(id.getTransactionId());
-
         if (request instanceof TransactionPurgeRequest) {
-            if (purgedTransactions.contains(ul)) {
-                // Retransmitted purge request: nothing to do
-                LOG.debug("{}: transaction {} already purged", persistenceId, id);
-                return new TransactionPurgeResponse(id, request.getSequence());
-            }
-
-            // We perform two lookups instead of a straight remove, because once the map becomes empty we switch it
-            // to an ImmutableMap, which does not allow remove().
-            if (closedTransactions.containsKey(ul)) {
-                tree.purgeTransaction(id, () -> {
-                    closedTransactions.remove(ul);
-                    if (closedTransactions.isEmpty()) {
-                        closedTransactions = ImmutableMap.of();
-                    }
-
-                    purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));
-                    LOG.debug("{}: finished purging inherited transaction {}", persistenceId(), id);
-                    envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);
-                });
-                return null;
-            }
-
-            final FrontendTransaction tx = transactions.get(id);
-            if (tx == null) {
-                // This should never happen because the purge callback removes the transaction and puts it into
-                // purged transactions in one go. If it does, we warn about the situation and
-                LOG.warn("{}: transaction {} not tracked in {}, but not present in active transactions", persistenceId,
-                    id, purgedTransactions);
-                purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));
-                return new TransactionPurgeResponse(id, request.getSequence());
-            }
-
-            tree.purgeTransaction(id, () -> {
-                purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));
-                transactions.remove(id);
-                LOG.debug("{}: finished purging transaction {}", persistenceId(), id);
-                envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);
-            });
-            return null;
+            return handleTransactionPurgeRequest(request, envelope, now);
         }
 
+        final TransactionIdentifier id = request.getTarget();
+        final UnsignedLong ul = UnsignedLong.fromLongBits(id.getTransactionId());
         if (purgedTransactions.contains(ul)) {
             LOG.warn("{}: Request {} is contained purged transactions {}", persistenceId, request, purgedTransactions);
             throw new DeadTransactionException(purgedTransactions);
@@ -154,6 +116,52 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
         return tx.handleRequest(request, envelope, now);
     }
 
+    private TransactionSuccess<?> handleTransactionPurgeRequest(final TransactionRequest<?> request,
+            final RequestEnvelope envelope, final long now) {
+        final TransactionIdentifier id = request.getTarget();
+        final UnsignedLong ul = UnsignedLong.fromLongBits(id.getTransactionId());
+        if (purgedTransactions.contains(ul)) {
+            // Retransmitted purge request: nothing to do
+            LOG.debug("{}: transaction {} already purged", persistenceId, id);
+            return new TransactionPurgeResponse(id, request.getSequence());
+        }
+
+        // We perform two lookups instead of a straight remove, because once the map becomes empty we switch it
+        // to an ImmutableMap, which does not allow remove().
+        if (closedTransactions.containsKey(ul)) {
+            tree.purgeTransaction(id, () -> {
+                closedTransactions.remove(ul);
+                if (closedTransactions.isEmpty()) {
+                    closedTransactions = ImmutableMap.of();
+                }
+
+                purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));
+                LOG.debug("{}: finished purging inherited transaction {}", persistenceId(), id);
+                envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);
+            });
+            return null;
+        }
+
+        final FrontendTransaction tx = transactions.get(id);
+        if (tx == null) {
+            // This should never happen because the purge callback removes the transaction and puts it into
+            // purged transactions in one go. If it does, we warn about the situation and
+            LOG.warn("{}: transaction {} not tracked in {}, but not present in active transactions", persistenceId,
+                id, purgedTransactions);
+            purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));
+            return new TransactionPurgeResponse(id, request.getSequence());
+        }
+
+        tree.purgeTransaction(id, () -> {
+            purgedTransactions.add(Range.closedOpen(ul, UnsignedLong.ONE.plus(ul)));
+            transactions.remove(id);
+            LOG.debug("{}: finished purging transaction {}", persistenceId(), id);
+            envelope.sendSuccess(new TransactionPurgeResponse(id, request.getSequence()), readTime() - now);
+        });
+
+        return null;
+    }
+
     final void destroy(final long sequence, final RequestEnvelope envelope, final long now) {
         LOG.debug("{}: closing history {}", persistenceId(), getIdentifier());
         tree.closeTransactionChain(getIdentifier(),
@@ -178,12 +186,11 @@ abstract class AbstractFrontendHistory implements Identifiable<LocalHistoryIdent
             tree.getStats().incrementReadWriteTransactionCount();
             return createReadyTransaction(id, ((CommitLocalTransactionRequest) request).getModification());
         }
-        if (request instanceof AbstractReadTransactionRequest) {
-            if (((AbstractReadTransactionRequest<?>) request).isSnapshotOnly()) {
-                LOG.debug("{}: allocating new open snapshot {}", persistenceId(), id);
-                tree.getStats().incrementReadOnlyTransactionCount();
-                return createOpenSnapshot(id);
-            }
+        if (request instanceof AbstractReadTransactionRequest
+                && ((AbstractReadTransactionRequest<?>) request).isSnapshotOnly()) {
+            LOG.debug("{}: allocating new open snapshot {}", persistenceId(), id);
+            tree.getStats().incrementReadOnlyTransactionCount();
+            return createOpenSnapshot(id);
         }
 
         LOG.debug("{}: allocating new open transaction {}", persistenceId(), id);
index 1a5b968..cde7da2 100644 (file)
@@ -14,8 +14,6 @@ import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier
 import org.opendaylight.controller.cluster.datastore.persisted.AbortTransactionPayload;
 import org.opendaylight.yangtools.concepts.Identifiable;
 import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Abstract base for transactions running on SharrdDataTree.
@@ -25,8 +23,6 @@ import org.slf4j.LoggerFactory;
 @NotThreadSafe
 abstract class AbstractShardDataTreeTransaction<T extends DataTreeSnapshot>
         implements Identifiable<TransactionIdentifier> {
-    private static final Logger LOG = LoggerFactory.getLogger(AbstractShardDataTreeTransaction.class);
-
     private final ShardDataTreeTransactionParent parent;
     private final TransactionIdentifier id;
     private final T snapshot;
index abf07ec..23fcf19 100644 (file)
@@ -60,7 +60,8 @@ public class DatastoreContextConfigAdminOverlay implements AutoCloseable {
         this.listener = listener;
     }
 
-    @SuppressWarnings("checkstyle:IllegalCatch")
+    @SuppressWarnings({"checkstyle:IllegalCatch",
+        "squid:S1166" /*  Exception handlers should preserve the original exception */})
     private void overlaySettings(ServiceReference<ConfigurationAdmin> configAdminServiceReference) {
         try {
             ConfigurationAdmin configAdmin = bundleContext.getService(configAdminServiceReference);
@@ -71,10 +72,8 @@ public class DatastoreContextConfigAdminOverlay implements AutoCloseable {
 
                 LOG.debug("Overlaying settings: {}", properties);
 
-                if (introspector.update(properties)) {
-                    if (listener != null) {
-                        listener.onDatastoreContextUpdated(introspector.newContextFactory());
-                    }
+                if (introspector.update(properties) && listener != null) {
+                    listener.onDatastoreContextUpdated(introspector.newContextFactory());
                 }
             } else {
                 LOG.debug("No Configuration found for {}", CONFIG_ID);
index 6685f64..aa54d5b 100644 (file)
@@ -18,6 +18,7 @@ import java.beans.Introspector;
 import java.beans.MethodDescriptor;
 import java.beans.PropertyDescriptor;
 import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -66,16 +67,18 @@ public class DatastoreContextIntrospector {
      * constructor that takes a single String argument. For primitive wrappers, this constructor
      * converts from a String representation.
      */
-    @SuppressWarnings("checkstyle:IllegalCatch")
+    // Disables "Either log or rethrow this exception" sonar warning
+    @SuppressWarnings("squid:S1166")
     private static void introspectPrimitiveTypes() {
-
         Set<Class<?>> primitives = ImmutableSet.<Class<?>>builder().addAll(
                 Primitives.allWrapperTypes()).add(String.class).build();
         for (Class<?> primitive: primitives) {
             try {
                 processPropertyType(primitive);
-            } catch (Exception e) {
+            } catch (NoSuchMethodException e) {
                 // Ignore primitives that can't be constructed from a String, eg Character and Void.
+            } catch (SecurityException | IntrospectionException e) {
+                LOG.error("Error introspect primitive type {}", primitive, e);
             }
         }
     }
@@ -139,7 +142,8 @@ public class DatastoreContextIntrospector {
      * Finds the appropriate constructor for the specified type that we will use to construct
      * instances.
      */
-    private static void processPropertyType(Class<?> propertyType) throws Exception {
+    private static void processPropertyType(Class<?> propertyType) throws NoSuchMethodException, SecurityException,
+            IntrospectionException {
         Class<?> wrappedType = Primitives.wrap(propertyType);
         if (CONSTRUCTORS.containsKey(wrappedType)) {
             return;
@@ -169,8 +173,7 @@ public class DatastoreContextIntrospector {
     /**
      * Finds the getter method on a yang-generated type for the specified property name.
      */
-    private static void findYangTypeGetter(Class<?> type, String propertyName)
-            throws Exception {
+    private static void findYangTypeGetter(Class<?> type, String propertyName) throws IntrospectionException {
         for (PropertyDescriptor desc: Introspector.getBeanInfo(type).getPropertyDescriptors()) {
             if (desc.getName().equals(propertyName)) {
                 YANG_TYPE_GETTERS.put(type, desc.getReadMethod());
@@ -178,7 +181,7 @@ public class DatastoreContextIntrospector {
             }
         }
 
-        throw new IllegalArgumentException(String.format(
+        throw new IntrospectionException(String.format(
                 "Getter method for constructor property %s not found for YANG type %s",
                 propertyName, type));
     }
@@ -303,7 +306,8 @@ public class DatastoreContextIntrospector {
                     Primitives.wrap(setter.getParameterTypes()[0]), value.toString()));
 
             return true;
-        } catch (Exception e) {
+        } catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException
+                | InstantiationException e) {
             LOG.error("Error converting value ({}) for property {}", inValue, key, e);
         }
 
@@ -321,7 +325,8 @@ public class DatastoreContextIntrospector {
         return StringUtils.uncapitalize(str);
     }
 
-    private Object convertValue(String name, Object from) throws Exception {
+    private Object convertValue(String name, Object from)
+            throws InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException {
         Class<?> propertyType = DATA_STORE_PROP_TYPES.get(name);
         if (propertyType == null) {
             LOG.debug("Property not found for {}", name);
@@ -345,7 +350,8 @@ public class DatastoreContextIntrospector {
         return converted;
     }
 
-    private Object constructorValueRecursively(Class<?> toType, Object fromValue) throws Exception {
+    private Object constructorValueRecursively(Class<?> toType, Object fromValue)
+            throws InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException {
         LOG.trace("convertValueRecursively - toType: {}, fromValue {} ({})",
                 toType.getSimpleName(), fromValue, fromValue.getClass().getSimpleName());
 
index 89da5b2..6d581f8 100644 (file)
@@ -86,7 +86,7 @@ final class DefaultShardDataChangeListenerPublisher implements ShardDataChangeLi
             final AsyncDataChangeListener<YangInstanceIdentifier,NormalizedNode<?, ?>> listener,
             final DataChangeScope scope, final DataTreeCandidate initialState, String logContext) {
         DefaultShardDataChangeListenerPublisher publisher = new DefaultShardDataChangeListenerPublisher(logContext);
-        publisher.registerDataChangeListener(path, listener, scope, Optional.absent(), noop -> { });
+        publisher.registerDataChangeListener(path, listener, scope, Optional.absent(), noop -> { /* NOOP */ });
         publisher.publishChanges(initialState);
     }
 }
index 449b620..2757c2d 100644 (file)
@@ -88,7 +88,7 @@ final class DefaultShardDataTreeChangeListenerPublisher extends AbstractDOMStore
         DefaultShardDataTreeChangeListenerPublisher publisher =
                 new DefaultShardDataTreeChangeListenerPublisher(logContext);
         publisher.logContext = logContext;
-        publisher.registerTreeChangeListener(treeId, listener, Optional.absent(), noop -> { });
+        publisher.registerTreeChangeListener(treeId, listener, Optional.absent(), noop -> { /* NOOP */ });
         publisher.publishChanges(state);
     }
 
index 11bbe1b..a4256a7 100644 (file)
@@ -24,6 +24,9 @@ public class DistributedDataStoreFactory {
     private static final String DEFAULT_MODULE_SHARDS_PATH = "./configuration/initial/module-shards.conf";
     private static final String DEFAULT_MODULES_PATH = "./configuration/initial/modules.conf";
 
+    private DistributedDataStoreFactory() {
+    }
+
     /**
      * Create a data store instance.
      *
index 50e9130..2b444a6 100644 (file)
@@ -247,7 +247,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction {
             case READY:
                 throw new IllegalStateException("Attempted to preCommit in stage " + ready.stage);
             default:
-                throw new IllegalStateException("Unhandled commit stage " + ready.stage);
+                throwUnhandledCommitStage(ready);
         }
     }
 
@@ -306,7 +306,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction {
             case READY:
                 throw new IllegalStateException("Attempted to doCommit in stage " + ready.stage);
             default:
-                throw new IllegalStateException("Unhandled commit stage " + ready.stage);
+                throwUnhandledCommitStage(ready);
         }
     }
 
@@ -395,7 +395,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction {
             case PRE_COMMIT_PENDING:
                 throw new IllegalStateException("Attempted to canCommit in stage " + ready.stage);
             default:
-                throw new IllegalStateException("Unhandled commit stage " + ready.stage);
+                throwUnhandledCommitStage(ready);
         }
     }
 
@@ -442,7 +442,7 @@ final class FrontendReadWriteTransaction extends FrontendTransaction {
                 });
                 break;
             default:
-                throw new IllegalStateException("Unhandled commit stage " + ready.stage);
+                throwUnhandledCommitStage(ready);
         }
     }
 
@@ -558,7 +558,8 @@ final class FrontendReadWriteTransaction extends FrontendTransaction {
         }
     }
 
-    private @Nullable TransactionSuccess<?> handleModifyTransaction(final ModifyTransactionRequest request,
+    @Nullable
+    private TransactionSuccess<?> handleModifyTransaction(final ModifyTransactionRequest request,
             final RequestEnvelope envelope, final long now) throws RequestException {
         // We need to examine the persistence protocol first to see if this is an idempotent request. If there is no
         // protocol, there is nothing for us to do.
@@ -636,4 +637,8 @@ final class FrontendReadWriteTransaction extends FrontendTransaction {
             state);
         return ((Sealed) state).sealedModification;
     }
+
+    private static void throwUnhandledCommitStage(final Ready ready) {
+        throw new IllegalStateException("Unhandled commit stage " + ready.stage);
+    }
 }
index 5bafef8..22ba497 100644 (file)
@@ -166,30 +166,29 @@ final class RemoteTransactionContextSupport {
         // the cached remote leader actor is no longer available.
         boolean retryCreateTransaction = primaryShardInfo != null
                 && (failure instanceof NoShardLeaderException || failure instanceof AskTimeoutException);
-        if (retryCreateTransaction) {
-            // Schedule a retry unless we're out of retries. Note: totalCreateTxTimeout is volatile as it may
-            // be written by different threads however not concurrently, therefore decrementing it
-            // non-atomically here is ok.
-            if (totalCreateTxTimeout > 0) {
-                long scheduleInterval = CREATE_TX_TRY_INTERVAL_IN_MS;
-                if (failure instanceof AskTimeoutException) {
-                    // Since we use the createTxMessageTimeout for the CreateTransaction request and it timed
-                    // out, subtract it from the total timeout. Also since the createTxMessageTimeout period
-                    // has already elapsed, we can immediately schedule the retry (10 ms is virtually immediate).
-                    totalCreateTxTimeout -= createTxMessageTimeout.duration().toMillis();
-                    scheduleInterval = 10;
-                }
-
-                totalCreateTxTimeout -= scheduleInterval;
-
-                LOG.debug("Tx {}: create tx on shard {} failed with exception \"{}\" - scheduling retry in {} ms",
-                        getIdentifier(), shardName, failure, scheduleInterval);
-
-                getActorContext().getActorSystem().scheduler().scheduleOnce(
-                    FiniteDuration.create(scheduleInterval, TimeUnit.MILLISECONDS),
-                        this::tryFindPrimaryShard, getActorContext().getClientDispatcher());
-                return;
+
+        // Schedule a retry unless we're out of retries. Note: totalCreateTxTimeout is volatile as it may
+        // be written by different threads however not concurrently, therefore decrementing it
+        // non-atomically here is ok.
+        if (retryCreateTransaction && totalCreateTxTimeout > 0) {
+            long scheduleInterval = CREATE_TX_TRY_INTERVAL_IN_MS;
+            if (failure instanceof AskTimeoutException) {
+                // Since we use the createTxMessageTimeout for the CreateTransaction request and it timed
+                // out, subtract it from the total timeout. Also since the createTxMessageTimeout period
+                // has already elapsed, we can immediately schedule the retry (10 ms is virtually immediate).
+                totalCreateTxTimeout -= createTxMessageTimeout.duration().toMillis();
+                scheduleInterval = 10;
             }
+
+            totalCreateTxTimeout -= scheduleInterval;
+
+            LOG.debug("Tx {}: create tx on shard {} failed with exception \"{}\" - scheduling retry in {} ms",
+                    getIdentifier(), shardName, failure, scheduleInterval);
+
+            getActorContext().getActorSystem().scheduler().scheduleOnce(
+                    FiniteDuration.create(scheduleInterval, TimeUnit.MILLISECONDS),
+                    this::tryFindPrimaryShard, getActorContext().getClientDispatcher());
+            return;
         }
 
         createTransactionContext(failure, response);
index 1789f09..402bf48 100644 (file)
@@ -502,7 +502,8 @@ public class Shard extends RaftActor {
         throw new OutOfSequenceEnvelopeException(0);
     }
 
-    private static @Nonnull ABIVersion selectVersion(final ConnectClientRequest message) {
+    @Nonnull
+    private static ABIVersion selectVersion(final ConnectClientRequest message) {
         final Range<ABIVersion> clientRange = Range.closed(message.getMinVersion(), message.getMaxVersion());
         for (ABIVersion v : SUPPORTED_ABIVERSIONS) {
             if (clientRange.contains(v)) {
@@ -550,7 +551,8 @@ public class Shard extends RaftActor {
         }
     }
 
-    private @Nullable RequestSuccess<?, ?> handleRequest(final RequestEnvelope envelope, final long now)
+    @Nullable
+    private RequestSuccess<?, ?> handleRequest(final RequestEnvelope envelope, final long now)
             throws RequestException {
         // We are not the leader, hence we want to fail-fast.
         if (!isLeader() || paused || !isLeaderActive()) {
@@ -683,7 +685,7 @@ public class Shard extends RaftActor {
             ActorSelection leader = getLeader();
             if (!isLeaderActive || leader == null) {
                 messageRetrySupport.addMessageToRetry(batched, getSender(),
-                        "Could not commit transaction " + batched.getTransactionId());
+                        "Could not process BatchedModifications " + batched.getTransactionId());
             } else {
                 // If this is not the first batch and leadership changed in between batched messages,
                 // we need to reconstruct previous BatchedModifications from the transaction
@@ -736,7 +738,7 @@ public class Shard extends RaftActor {
             ActorSelection leader = getLeader();
             if (!isLeaderActive || leader == null) {
                 messageRetrySupport.addMessageToRetry(message, getSender(),
-                        "Could not commit transaction " + message.getTransactionId());
+                        "Could not process ready local transaction " + message.getTransactionId());
             } else {
                 LOG.debug("{}: Forwarding ReadyLocalTransaction to leader {}", persistenceId(), leader);
                 message.setRemoteVersion(getCurrentBehavior().getLeaderPayloadVersion());
@@ -755,7 +757,7 @@ public class Shard extends RaftActor {
             ActorSelection leader = getLeader();
             if (!isLeaderActive || leader == null) {
                 messageRetrySupport.addMessageToRetry(forwardedReady, getSender(),
-                        "Could not commit transaction " + forwardedReady.getTransactionId());
+                        "Could not process forwarded ready transaction " + forwardedReady.getTransactionId());
             } else {
                 LOG.debug("{}: Forwarding ForwardedReadyTransaction to leader {}", persistenceId(), leader);
 
index 37e65f6..e399cd4 100644 (file)
@@ -299,7 +299,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
      * @param snapshot Snapshot that needs to be applied
      * @throws DataValidationFailedException when the snapshot fails to apply
      */
-    void applyRecoverySnapshot(final @Nonnull ShardDataTreeSnapshot snapshot) throws DataValidationFailedException {
+    void applyRecoverySnapshot(@Nonnull final ShardDataTreeSnapshot snapshot) throws DataValidationFailedException {
         applySnapshot(snapshot, this::wrapWithPruning);
     }
 
@@ -333,7 +333,7 @@ public class ShardDataTree extends ShardDataTreeTransactionParent {
      * @throws IOException when the snapshot fails to deserialize
      * @throws DataValidationFailedException when the snapshot fails to apply
      */
-    void applyRecoveryPayload(final @Nonnull Payload payload) throws IOException, DataValidationFailedException {
+    void applyRecoveryPayload(@Nonnull final Payload payload) throws IOException, DataValidationFailedException {
         if (payload instanceof CommitTransactionPayload) {
             final Entry<TransactionIdentifier, DataTreeCandidate> e =
                     ((CommitTransactionPayload) payload).getCandidate();
index 7db3a22..e2c1b27 100644 (file)
@@ -44,14 +44,16 @@ abstract class ShardDataTreeMetadata<T extends ShardDataTreeSnapshotMetadata<T>>
      *
      * @return Metadata type
      */
-    abstract @Nonnull Class<T> getSupportedType();
+    @Nonnull
+    abstract Class<T> getSupportedType();
 
     /**
      * Take a snapshot of current metadata state.
      *
      * @return Metadata snapshot, or null if the metadata is empty.
      */
-    abstract @Nullable T toSnapshot();
+    @Nullable
+    abstract T toSnapshot();
 
     // Lifecycle events
 
index dfd60af..0e82116 100644 (file)
@@ -40,7 +40,8 @@ public abstract class ShardTransaction extends AbstractUntypedActorWithMetering
 
     protected ShardTransaction(final ActorRef shardActor, final ShardStats shardStats,
             final TransactionIdentifier transactionId) {
-        super("shard-tx"); //actor name override used for metering. This does not change the "real" actor name
+        // actor name override used for metering. This does not change the "real" actor name
+        super("shard-tx");
         this.shardActor = shardActor;
         this.shardStats = shardStats;
         this.transactionId = Preconditions.checkNotNull(transactionId);
index c1f687b..0007d09 100644 (file)
@@ -142,7 +142,7 @@ public class ConfigurationImpl implements Configuration {
 
     @Override
     public Collection<MemberName> getMembersFromShardName(final String shardName) {
-        Preconditions.checkNotNull(shardName, "shardName should not be null");
+        checkNotNullShardName(shardName);
 
         for (ModuleConfig moduleConfig: moduleConfigMap.values()) {
             ShardConfig shardConfig = moduleConfig.getShardConfig(shardName);
@@ -160,6 +160,10 @@ public class ConfigurationImpl implements Configuration {
         return Collections.emptyList();
     }
 
+    private static void checkNotNullShardName(final String shardName) {
+        Preconditions.checkNotNull(shardName, "shardName should not be null");
+    }
+
     @Override
     public Set<String> getAllShardNames() {
         return allShardNames;
@@ -234,13 +238,13 @@ public class ConfigurationImpl implements Configuration {
 
     @Override
     public boolean isShardConfigured(String shardName) {
-        Preconditions.checkNotNull(shardName, "shardName should not be null");
+        checkNotNullShardName(shardName);
         return allShardNames.contains(shardName);
     }
 
     @Override
     public void addMemberReplicaForShard(String shardName, MemberName newMemberName) {
-        Preconditions.checkNotNull(shardName, "shardName should not be null");
+        checkNotNullShardName(shardName);
         Preconditions.checkNotNull(newMemberName, "MemberName should not be null");
 
         for (ModuleConfig moduleConfig: moduleConfigMap.values()) {
@@ -256,7 +260,7 @@ public class ConfigurationImpl implements Configuration {
 
     @Override
     public void removeMemberReplicaForShard(String shardName, MemberName newMemberName) {
-        Preconditions.checkNotNull(shardName, "shardName should not be null");
+        checkNotNullShardName(shardName);
         Preconditions.checkNotNull(newMemberName, "MemberName should not be null");
 
         for (ModuleConfig moduleConfig: moduleConfigMap.values()) {
index 629418f..c387fe4 100644 (file)
@@ -52,16 +52,17 @@ public class PrefixShardConfiguration implements Serializable {
 
         @Override
         public void readExternal(final ObjectInput objectInput) throws IOException, ClassNotFoundException {
-            final DOMDataTreeIdentifier prefix =  (DOMDataTreeIdentifier) objectInput.readObject();
-            final String strategyName = (String) objectInput.readObject();
+            final DOMDataTreeIdentifier localPrefix = (DOMDataTreeIdentifier) objectInput.readObject();
+            final String localStrategyName = (String) objectInput.readObject();
 
             final int size = objectInput.readInt();
-            final Collection<MemberName> shardMemberNames = new ArrayList<>(size);
+            final Collection<MemberName> localShardMemberNames = new ArrayList<>(size);
             for (int i = 0; i < size; i++) {
-                shardMemberNames.add(MemberName.readFrom(objectInput));
+                localShardMemberNames.add(MemberName.readFrom(objectInput));
             }
 
-            prefixShardConfiguration = new PrefixShardConfiguration(prefix, strategyName, shardMemberNames);
+            prefixShardConfiguration = new PrefixShardConfiguration(localPrefix, localStrategyName,
+                    localShardMemberNames);
         }
 
         private Object readResolve() {
index 6094e24..7d872b4 100644 (file)
@@ -25,7 +25,7 @@ public abstract class AbstractEntityOwnerChangeListener implements DOMDataTreeCh
             .node(ENTITY_OWNER_QNAME).build();
 
     void init(ShardDataTree shardDataTree) {
-        shardDataTree.registerTreeChangeListener(EOS_PATH, this, Optional.absent(), noop -> { });
+        shardDataTree.registerTreeChangeListener(EOS_PATH, this, Optional.absent(), noop -> { /* NOOP */ });
     }
 
     protected static String extractOwner(LeafNode<?> ownerLeaf) {
index 6b6717c..729b7d8 100644 (file)
@@ -60,8 +60,8 @@ class CandidateListChangeListener implements DOMDataTreeChangeListener {
 
     void init(ShardDataTree shardDataTree) {
         shardDataTree.registerTreeChangeListener(YangInstanceIdentifier.builder(ENTITY_OWNERS_PATH)
-                .node(EntityType.QNAME).node(EntityType.QNAME).node(ENTITY_QNAME).node(ENTITY_QNAME)
-                    .node(Candidate.QNAME).node(Candidate.QNAME).build(), this, Optional.absent(), noop -> { });
+            .node(EntityType.QNAME).node(EntityType.QNAME).node(ENTITY_QNAME).node(ENTITY_QNAME)
+                .node(Candidate.QNAME).node(Candidate.QNAME).build(), this, Optional.absent(), noop -> { /* NOOP */ });
     }
 
     @Override
index 1b5e512..a392049 100644 (file)
@@ -49,6 +49,9 @@ public final class EntityOwnersModel {
     static final YangInstanceIdentifier ENTITY_TYPES_PATH =
             YangInstanceIdentifier.of(EntityOwners.QNAME).node(EntityType.QNAME);
 
+    private EntityOwnersModel() {
+    }
+
     static YangInstanceIdentifier entityPath(String entityType, YangInstanceIdentifier entityId) {
         return YangInstanceIdentifier.builder(ENTITY_OWNERS_PATH).node(EntityType.QNAME)
                 .nodeWithKey(EntityType.QNAME, ENTITY_TYPE_QNAME, entityType).node(ENTITY_QNAME)
index 29ace24..126324f 100644 (file)
@@ -64,7 +64,7 @@ public final class EntityOwnerSelectionStrategyConfigReader {
 
             LOG.debug("Could not read strategy configuration file, will use default configuration");
         } catch (IOException e1) {
-            LOG.warn("Failed to get configuration for {}, starting up empty", CONFIG_ID);
+            LOG.warn("Failed to get configuration for {}, starting up empty", CONFIG_ID, e1);
             return builder.build();
         } finally {
             try {
@@ -127,7 +127,7 @@ public final class EntityOwnerSelectionStrategyConfigReader {
         try {
             clazz = EntityOwnerSelectionStrategyConfigReader.class.getClassLoader().loadClass(strategyClassAndDelay);
         } catch (ClassNotFoundException e) {
-            throw new IllegalArgumentException("Failed to load strategy " + strategyClassAndDelay);
+            throw new IllegalArgumentException("Failed to load strategy " + strategyClassAndDelay, e);
         }
 
         Preconditions.checkArgument(EntityOwnerSelectionStrategy.class.isAssignableFrom(clazz),
index 08632a8..8b46987 100644 (file)
@@ -17,6 +17,9 @@ import org.opendaylight.controller.cluster.datastore.Shard;
  */
 public class ShardMBeanFactory {
 
+    private ShardMBeanFactory() {
+    }
+
     public static ShardStats getShardStatsMBean(final String shardName, final String mxBeanType,
             @Nonnull final Shard shard) {
         String finalMXBeanType = mxBeanType != null ? mxBeanType : "DistDataStore";
index 3cc6d04..c753ad2 100644 (file)
@@ -43,7 +43,8 @@ public class DataChanged implements Externalizable {
 
     @Override
     public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
-        in.readShort(); // Read the version
+        // Read the version
+        in.readShort();
 
         NormalizedNodeDataInput streamReader = NormalizedNodeInputOutput.newDataInputWithoutValidation(in);
 
index 707f50b..0ca4f64 100644 (file)
@@ -27,11 +27,13 @@ public class LocalPrimaryShardFound {
         this.localShardDataTree = Preconditions.checkNotNull(localShardDataTree);
     }
 
-    public @Nonnull String getPrimaryPath() {
+    @Nonnull
+    public String getPrimaryPath() {
         return primaryPath;
     }
 
-    public @Nonnull DataTree getLocalShardDataTree() {
+    @Nonnull
+    public DataTree getLocalShardDataTree() {
         return localShardDataTree;
     }
 
index 27d2475..f739e1e 100644 (file)
@@ -39,7 +39,8 @@ public class PrimaryShardInfo {
     /**
      * Returns an ActorSelection representing the primary shard actor.
      */
-    public @Nonnull ActorSelection getPrimaryShardActor() {
+    @Nonnull
+    public ActorSelection getPrimaryShardActor() {
         return primaryShardActor;
     }
 
@@ -54,7 +55,8 @@ public class PrimaryShardInfo {
      * Returns an Optional whose value contains the primary shard's DataTree if the primary shard is local
      * to the caller. Otherwise the Optional value is absent.
      */
-    public @Nonnull Optional<DataTree> getLocalShardDataTree() {
+    @Nonnull
+    public Optional<DataTree> getLocalShardDataTree() {
         return Optional.ofNullable(localShardDataTree);
     }
 }
index dbd0310..0b3b6b8 100644 (file)
@@ -37,7 +37,8 @@ public class ShardLeaderStateChanged extends LeaderStateChanged {
         this.localShardDataTree = null;
     }
 
-    public @Nonnull Optional<DataTree> getLocalShardDataTree() {
+    @Nonnull
+    public Optional<DataTree> getLocalShardDataTree() {
         return Optional.ofNullable(localShardDataTree);
     }
 }
index 709217d..997fa45 100644 (file)
@@ -42,7 +42,8 @@ public abstract class AbstractVersionException extends Exception {
      *
      * @return Closest supported {@link PayloadVersion}
      */
-    public final @Nonnull PayloadVersion getClosestVersion() {
+    @Nonnull
+    public final PayloadVersion getClosestVersion() {
         return closestVersion;
     }
 
index fb9b07a..bc1fca1 100644 (file)
@@ -168,7 +168,7 @@ public final class DataTreeCandidateInputOutput {
                 out.writeByte(UNMODIFIED);
                 break;
             default:
-                throw new IllegalArgumentException("Unhandled node type " + node.getModificationType());
+                throwUnhandledNodeType(node);
         }
     }
 
@@ -201,8 +201,12 @@ public final class DataTreeCandidateInputOutput {
                     writer.writeNormalizedNode(node.getDataAfter().get());
                     break;
                 default:
-                    throw new IllegalArgumentException("Unhandled node type " + node.getModificationType());
+                    throwUnhandledNodeType(node);
             }
         }
     }
+
+    private static void throwUnhandledNodeType(final DataTreeCandidateNode node) {
+        throw new IllegalArgumentException("Unhandled node type " + node.getModificationType());
+    }
 }
index b37fb4d..37d4125 100644 (file)
@@ -57,16 +57,16 @@ public class DatastoreSnapshot implements Serializable {
 
         @Override
         public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
-            String type = (String)in.readObject();
-            ShardManagerSnapshot shardManagerSnapshot = (ShardManagerSnapshot) in.readObject();
+            String localType = (String)in.readObject();
+            ShardManagerSnapshot localShardManagerSnapshot = (ShardManagerSnapshot) in.readObject();
 
             int size = in.readInt();
-            List<ShardSnapshot> shardSnapshots = new ArrayList<>(size);
+            List<ShardSnapshot> localShardSnapshots = new ArrayList<>(size);
             for (int i = 0; i < size; i++) {
-                shardSnapshots.add((ShardSnapshot) in.readObject());
+                localShardSnapshots.add((ShardSnapshot) in.readObject());
             }
 
-            datastoreSnapshot = new DatastoreSnapshot(type, shardManagerSnapshot, shardSnapshots);
+            datastoreSnapshot = new DatastoreSnapshot(localType, localShardManagerSnapshot, localShardSnapshots);
         }
 
         private Object readResolve() {
index fd35046..7a8bd46 100644 (file)
@@ -13,8 +13,6 @@ import java.io.ObjectInput;
 import java.io.ObjectOutput;
 import java.util.Optional;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Abstract base class for snapshots of the ShardDataTree.
@@ -23,8 +21,6 @@ import org.slf4j.LoggerFactory;
  */
 @Beta
 public abstract class ShardDataTreeSnapshot {
-    private static final Logger LOG = LoggerFactory.getLogger(ShardDataTreeSnapshot.class);
-
     ShardDataTreeSnapshot() {
         // Hidden to prevent subclassing from outside of this package
     }
index 0ddc785..3ba5a91 100644 (file)
@@ -46,7 +46,8 @@ public abstract class ShardDataTreeSnapshotMetadata<T extends ShardDataTreeSnaps
      *
      * @return Externalizable proxy, may not be null
      */
-    protected abstract @Nonnull Externalizable externalizableProxy();
+    @Nonnull
+    protected abstract Externalizable externalizableProxy();
 
     public abstract Class<T> getType();
 }
index 9dc95c0..0c1969b 100644 (file)
@@ -63,19 +63,19 @@ public class ShardManagerSnapshot implements Serializable {
         @Override
         public void readExternal(final ObjectInput in) throws IOException, ClassNotFoundException {
             int size = in.readInt();
-            List<String> shardList = new ArrayList<>(size);
+            List<String> localShardList = new ArrayList<>(size);
             for (int i = 0; i < size; i++) {
-                shardList.add((String) in.readObject());
+                localShardList.add((String) in.readObject());
             }
 
             size = in.readInt();
-            Map<DOMDataTreeIdentifier, PrefixShardConfiguration> prefixShardConfiguration = new HashMap<>(size);
+            Map<DOMDataTreeIdentifier, PrefixShardConfiguration> localPrefixShardConfiguration = new HashMap<>(size);
             for (int i = 0; i < size; i++) {
-                prefixShardConfiguration.put((DOMDataTreeIdentifier) in.readObject(),
+                localPrefixShardConfiguration.put((DOMDataTreeIdentifier) in.readObject(),
                         (PrefixShardConfiguration) in.readObject());
             }
 
-            snapshot = new ShardManagerSnapshot(shardList, prefixShardConfiguration);
+            snapshot = new ShardManagerSnapshot(localShardList, localPrefixShardConfiguration);
         }
 
         private Object readResolve() {
index 992786a..e3d3a60 100644 (file)
@@ -336,7 +336,7 @@ class ShardManager extends AbstractUntypedPersistentActorWithMetering {
     private void onInitConfigListener() {
         LOG.debug("{}: Initializing config listener on {}", persistenceId(), cluster.getCurrentMemberName());
 
-        final org.opendaylight.mdsal.common.api.LogicalDatastoreType type =
+        final org.opendaylight.mdsal.common.api.LogicalDatastoreType datastoreType =
                 org.opendaylight.mdsal.common.api.LogicalDatastoreType
                         .valueOf(datastoreContextFactory.getBaseDatastoreContext().getLogicalStoreType().name());
 
@@ -345,7 +345,7 @@ class ShardManager extends AbstractUntypedPersistentActorWithMetering {
         }
 
         configUpdateHandler = new PrefixedShardConfigUpdateHandler(self(), cluster.getCurrentMemberName());
-        configUpdateHandler.initListener(dataStore, type);
+        configUpdateHandler.initListener(dataStore, datastoreType);
     }
 
     private void onShutDown() {
@@ -849,7 +849,8 @@ class ShardManager extends AbstractUntypedPersistentActorWithMetering {
         final ActorRef sender = getSender();
 
         if (sender == null) {
-            return; //why is a non-actor sending this message? Just ignore.
+            // why is a non-actor sending this message? Just ignore.
+            return;
         }
 
         String actorName = sender.path().name();
@@ -1252,7 +1253,8 @@ class ShardManager extends AbstractUntypedPersistentActorWithMetering {
             }
         }
 
-        restoreFromSnapshot = null; // null out to GC
+        // null out to GC
+        restoreFromSnapshot = null;
 
         for (String shardName : memberShardNames) {
             ShardIdentifier shardId = getShardIdentifier(memberName, shardName);
index 9899aeb..5322420 100644 (file)
@@ -39,7 +39,7 @@ public final class ShardManagerSnapshot implements Serializable {
      *             org.opendaylight.controller.cluster.datastore.ShardManagerSnapshot is removed.
      */
     @Deprecated
-    public static ShardManagerSnapshot forShardList(final @Nonnull List<String> shardList) {
+    public static ShardManagerSnapshot forShardList(@Nonnull final List<String> shardList) {
         return new ShardManagerSnapshot(shardList);
     }
 
index 06bb712..a877430 100644 (file)
@@ -43,6 +43,9 @@ public class ClusterUtils {
     public static final YangInstanceIdentifier SHARD_LIST_PATH =
             PREFIX_SHARDS_PATH.node(SHARD_LIST_QNAME).toOptimized();
 
+    private ClusterUtils() {
+    }
+
     public static ShardIdentifier getShardIdentifier(final MemberName memberName, final DOMDataTreeIdentifier prefix) {
         final String type;
         switch (prefix.getDatastoreType()) {
index 0ae3fef..6e7d2bf 100644 (file)
@@ -29,7 +29,7 @@ public abstract class CompositeOnComplete<T> extends OnComplete<T> {
         onCompleteTasks.add(task);
     }
 
-    @SuppressWarnings("checkstyle:IllegalCatch")
+    @SuppressWarnings({ "checkstyle:IllegalCatch", "squid:S1181" /*  Throwable and Error should not be caught */ })
     protected void notifyOnCompleteTasks(Throwable failure, T result) {
         for (OnComplete<T> task: onCompleteTasks) {
             try {
index 70d9b94..3d4476a 100644 (file)
@@ -23,7 +23,8 @@ import scala.concurrent.Future;
 public class PrimaryShardInfoFutureCache {
     private final Cache<String, Future<PrimaryShardInfo>> primaryShardInfoCache = CacheBuilder.newBuilder().build();
 
-    public @Nullable Future<PrimaryShardInfo> getIfPresent(@Nonnull String shardName) {
+    @Nullable
+    public Future<PrimaryShardInfo> getIfPresent(@Nonnull String shardName) {
         return primaryShardInfoCache.getIfPresent(shardName);
     }
 
index f6628ad..eda1153 100644 (file)
@@ -18,11 +18,11 @@ import javax.annotation.Nonnull;
 public class DOMDataTreeShardCreationFailedException extends Exception {
     private static final long serialVersionUID = 1L;
 
-    public DOMDataTreeShardCreationFailedException(final @Nonnull String message) {
+    public DOMDataTreeShardCreationFailedException(@Nonnull final String message) {
         super(message);
     }
 
-    public DOMDataTreeShardCreationFailedException(final @Nonnull String message, final @Nonnull Throwable cause) {
+    public DOMDataTreeShardCreationFailedException(@Nonnull final String message, @Nonnull final Throwable cause) {
         super(message, cause);
     }
 }
index 0eef92a..57e7513 100644 (file)
@@ -57,9 +57,6 @@ public class DistributedShardChangePublisher
     private final AbstractDataStore distributedDataStore;
     private final YangInstanceIdentifier shardPath;
 
-    // This will be useful for signaling back pressure
-    private final DataStoreClient client;
-
     private final Map<DOMDataTreeIdentifier, ChildShardContext> childShards;
 
     @GuardedBy("this")
@@ -69,7 +66,6 @@ public class DistributedShardChangePublisher
                                            final AbstractDataStore distributedDataStore,
                                            final DOMDataTreeIdentifier prefix,
                                            final Map<DOMDataTreeIdentifier, ChildShardContext> childShards) {
-        this.client = client;
         this.distributedDataStore = distributedDataStore;
         // TODO keeping the whole dataTree thats contained in subshards doesn't seem like a good idea
         // maybe the whole listener logic would be better in the backend shards where we have direct access to the
@@ -306,7 +302,7 @@ public class DistributedShardChangePublisher
                 // data tree yet. Postpone processing of these changes till we
                 // receive changes from current shard.
                 LOG.debug("Validation for modification built from subshard {} changes {} failed, current data tree {}.",
-                        pathFromRoot, changes, dataTree);
+                        pathFromRoot, changes, dataTree, e);
                 stashedDataTreeCandidates.addAll(newCandidates);
             }
         }
index 3c8db5f..b78836b 100644 (file)
@@ -11,25 +11,17 @@ package org.opendaylight.controller.cluster.sharding;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import java.util.Collection;
-import java.util.concurrent.atomic.AtomicLong;
 import javax.annotation.Nonnull;
 import org.opendaylight.controller.cluster.databroker.actors.dds.ClientLocalHistory;
 import org.opendaylight.controller.cluster.databroker.actors.dds.DataStoreClient;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
 import org.opendaylight.mdsal.dom.spi.shard.DOMDataTreeShardProducer;
 import org.opendaylight.mdsal.dom.spi.shard.DOMDataTreeShardWriteTransaction;
-import org.opendaylight.mdsal.dom.store.inmemory.InMemoryDOMDataTreeShard;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Proxy producer implementation that creates transactions that forward all calls to {@link DataStoreClient}.
  */
 class ShardProxyProducer implements DOMDataTreeShardProducer {
-
-    private static final Logger LOG = LoggerFactory.getLogger(InMemoryDOMDataTreeShard.class);
-    private static final AtomicLong COUNTER = new AtomicLong();
-
     private final DOMDataTreeIdentifier shardRoot;
     private final Collection<DOMDataTreeIdentifier> prefixes;
     private final ClientLocalHistory history;
index 52630eb..a614f92 100644 (file)
@@ -124,7 +124,7 @@ class ShardProxyTransaction implements DOMDataTreeShardWriteTransaction {
     public ListenableFuture<Void> submit() {
         LOG.debug("Submitting transaction for shard {}", shardRoot);
 
-        Preconditions.checkState(!cohorts.isEmpty(), "Transaction not readied yet");
+        checkTransactionReadied();
 
         final AsyncFunction<Boolean, Void> validateFunction = input -> prepare();
         final AsyncFunction<Void, Void> prepareFunction = input -> commit();
@@ -136,11 +136,15 @@ class ShardProxyTransaction implements DOMDataTreeShardWriteTransaction {
         return Futures.transformAsync(prepareFuture, prepareFunction, MoreExecutors.directExecutor());
     }
 
+    private void checkTransactionReadied() {
+        Preconditions.checkState(!cohorts.isEmpty(), "Transaction not readied yet");
+    }
+
     @Override
     public ListenableFuture<Boolean> validate() {
         LOG.debug("Validating transaction for shard {}", shardRoot);
 
-        Preconditions.checkState(!cohorts.isEmpty(), "Transaction not readied yet");
+        checkTransactionReadied();
         final List<ListenableFuture<Boolean>> futures =
                 cohorts.stream().map(DOMStoreThreePhaseCommitCohort::canCommit).collect(Collectors.toList());
         final SettableFuture<Boolean> ret = SettableFuture.create();
@@ -164,7 +168,7 @@ class ShardProxyTransaction implements DOMDataTreeShardWriteTransaction {
     public ListenableFuture<Void> prepare() {
         LOG.debug("Preparing transaction for shard {}", shardRoot);
 
-        Preconditions.checkState(!cohorts.isEmpty(), "Transaction not readied yet");
+        checkTransactionReadied();
         final List<ListenableFuture<Void>> futures =
                 cohorts.stream().map(DOMStoreThreePhaseCommitCohort::preCommit).collect(Collectors.toList());
         final SettableFuture<Void> ret = SettableFuture.create();
@@ -188,7 +192,7 @@ class ShardProxyTransaction implements DOMDataTreeShardWriteTransaction {
     public ListenableFuture<Void> commit() {
         LOG.debug("Committing transaction for shard {}", shardRoot);
 
-        Preconditions.checkState(!cohorts.isEmpty(), "Transaction not readied yet");
+        checkTransactionReadied();
         final List<ListenableFuture<Void>> futures =
                 cohorts.stream().map(DOMStoreThreePhaseCommitCohort::commit).collect(Collectors.toList());
         final SettableFuture<Void> ret = SettableFuture.create();
index 0f55831..3efbbab 100644 (file)
@@ -17,7 +17,6 @@ import akka.actor.PoisonPill;
 import akka.actor.Props;
 import akka.actor.Status;
 import akka.actor.Status.Success;
-import akka.cluster.Cluster;
 import akka.cluster.ClusterEvent;
 import akka.cluster.ClusterEvent.MemberExited;
 import akka.cluster.ClusterEvent.MemberRemoved;
@@ -98,11 +97,6 @@ public class ShardedDataTreeActor extends AbstractUntypedPersistentActor {
     private final int lookupTaskMaxRetries;
 
     private final Map<DOMDataTreeIdentifier, ActorProducerRegistration> idToProducer = new HashMap<>();
-    private final Map<DOMDataTreeIdentifier, ShardFrontendRegistration> idToShardRegistration = new HashMap<>();
-
-    private final Cluster cluster;
-
-    private final Map<DOMDataTreeIdentifier, PrefixShardConfiguration> currentConfiguration = new HashMap<>();
 
     ShardedDataTreeActor(final ShardedDataTreeActorCreator builder) {
         LOG.debug("Creating ShardedDataTreeActor on {}", builder.getClusterWrapper().getCurrentMemberName());
@@ -118,7 +112,6 @@ public class ShardedDataTreeActor extends AbstractUntypedPersistentActor {
                 DistributedShardedDOMDataTree.ACTOR_ID, clusterWrapper.getCurrentMemberName());
 
         clusterWrapper.subscribeToMemberEvents(self());
-        cluster = Cluster.get(actorSystem);
     }
 
     @Override

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.