Remove PersistAbortTransactionPayload

With the metadata tracking disabled for ask based protocol
there is no need to track aborts for read only transactions
on the backend.

JIRA: CONTROLLER-1879
Change-Id: I189ae3231bb2f3c0eaa0bbe21a14342446708c5f
Signed-off-by: Tomas Cere <tomas.cere@pantheon.tech>
(cherry picked from commit 7011afc1380ca25ad3de4cc8865f12fe3f5e09cf)
(cherry picked from commit f437308703b0386e870f3525f19a3c3b997e8baa)

diff --git a/opendaylight/md-sal/sal-cluster-admin-impl/src/test/java/org/opendaylight/controller/cluster/datastore/admin/ClusterAdminRpcServiceTest.java b/opendaylight/md-sal/sal-cluster-admin-impl/src/test/java/org/opendaylight/controller/cluster/datastore/admin/ClusterAdminRpcServiceTest.java
index 8f9f2b2c27495bcaaaf662b27f1c7b91b38830a2..8411cd4b71081efaeb65d0d9aeb4abcb49a87bcb 100644 (file)
--- a/opendaylight/md-sal/sal-cluster-admin-impl/src/test/java/org/opendaylight/controller/cluster/datastore/admin/ClusterAdminRpcServiceTest.java
+++ b/opendaylight/md-sal/sal-cluster-admin-impl/src/test/java/org/opendaylight/controller/cluster/datastore/admin/ClusterAdminRpcServiceTest.java
@@ -389,8 +389,8 @@ public class ClusterAdminRpcServiceTest {
         // 2 ServerConfigurationPayload entries,  the transaction payload entry plus a purge payload.
 
         RaftStateVerifier verifier = raftState -> {
-            assertEquals("Commit index", 3, raftState.getCommitIndex());
-            assertEquals("Last applied index", 3, raftState.getLastApplied());
+            assertEquals("Commit index", 5, raftState.getCommitIndex());
+            assertEquals("Last applied index", 5, raftState.getLastApplied());
         };
 
         verifyRaftState(leaderNode1.configDataStore(), "cars", verifier);

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java
index e11778da87183bcd7033d28d81e8d175054ca228..f4db772c6b896eb92940b3c88b84cd5dff0dced6 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.java
@@ -134,7 +134,10 @@ final class FrontendMetadata extends ShardDataTreeMetadata<FrontendShardDataTree
         final FrontendIdentifier frontendId = clientId.getFrontendId();
         final FrontendClientMetadataBuilder client = clients.get(frontendId);
         if (client == null) {
-            LOG.debug("{}: disableTracking {} does not match any client, ignoring", shardName, clientId);
+            // When we havent seen the client before, we still need to disable tracking for him since this only gets
+            // triggered once.
+            LOG.debug("{}: disableTracking {} does not match any client, pre-disabling client.", shardName, clientId);
+            clients.put(frontendId, new FrontendClientMetadataBuilder.Disabled(shardName, clientId));
             return;
         }
         if (!clientId.equals(client.getIdentifier())) {

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
index 9aa5a43fed8335db9325a5a007072ade273b7d22..15dc43d96fec044d56007d19c995f9466b181500 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/Shard.java
@@ -31,6 +31,7 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 import javax.annotation.Nonnull;
@@ -78,16 +79,13 @@ import org.opendaylight.controller.cluster.datastore.messages.GetShardDataTree;
 import org.opendaylight.controller.cluster.datastore.messages.MakeLeaderLocal;
 import org.opendaylight.controller.cluster.datastore.messages.OnDemandShardState;
 import org.opendaylight.controller.cluster.datastore.messages.PeerAddressResolved;
-import org.opendaylight.controller.cluster.datastore.messages.PersistAbortTransactionPayload;
 import org.opendaylight.controller.cluster.datastore.messages.ReadyLocalTransaction;
 import org.opendaylight.controller.cluster.datastore.messages.RegisterDataTreeChangeListener;
 import org.opendaylight.controller.cluster.datastore.messages.ShardLeaderStateChanged;
 import org.opendaylight.controller.cluster.datastore.messages.UpdateSchemaContext;
-import org.opendaylight.controller.cluster.datastore.persisted.AbortTransactionPayload;
 import org.opendaylight.controller.cluster.datastore.persisted.DatastoreSnapshot;
 import org.opendaylight.controller.cluster.datastore.persisted.DatastoreSnapshot.ShardSnapshot;
 import org.opendaylight.controller.cluster.datastore.persisted.DisableTrackingPayload;
-import org.opendaylight.controller.cluster.datastore.persisted.PurgeTransactionPayload;
 import org.opendaylight.controller.cluster.messaging.MessageAssembler;
 import org.opendaylight.controller.cluster.messaging.MessageSlicer;
 import org.opendaylight.controller.cluster.messaging.SliceOptions;
@@ -367,12 +365,6 @@ public class Shard extends RaftActor {
             } else if (message instanceof DataTreeCohortActorRegistry.CohortRegistryCommand) {
                 store.processCohortRegistryCommand(getSender(),
                         (DataTreeCohortActorRegistry.CohortRegistryCommand) message);
-            } else if (message instanceof PersistAbortTransactionPayload) {
-                final TransactionIdentifier txId = ((PersistAbortTransactionPayload) message).getTransactionId();
-                persistPayload(txId, AbortTransactionPayload.create(txId,
-                    datastoreContext.getInitialPayloadSerializedBufferCapacity()), true);
-                persistPayload(txId, PurgeTransactionPayload.create(txId,
-                    datastoreContext.getInitialPayloadSerializedBufferCapacity()), false);
             } else if (message instanceof MakeLeaderLocal) {
                 onMakeLeaderLocal();
             } else if (RESUME_NEXT_PENDING_TRANSACTION.equals(message)) {
@@ -463,6 +455,8 @@ public class Shard extends RaftActor {
                 }
             } else {
                 LOG.debug("{}: leader state for {} not found", persistenceId(), clientId);
+                knownFrontends.put(frontendId, new LeaderFrontendState.Disabled(persistenceId(), clientId,
+                    getDataStore()));
             }
         }
     }
@@ -876,9 +870,15 @@ public class Shard extends RaftActor {
 
     // Called on leader only
     private void askProtocolEncountered(final ClientIdentifier clientId) {
-        final LeaderFrontendState state = knownFrontends.get(clientId.getFrontendId());
-        if (state instanceof LeaderFrontendState.Enabled) {
+        final FrontendIdentifier frontend = clientId.getFrontendId();
+        final LeaderFrontendState state = knownFrontends.get(frontend);
+        if (!(state instanceof LeaderFrontendState.Disabled)) {
             LOG.debug("{}: encountered ask-based client {}, disabling transaction tracking", persistenceId(), clientId);
+            if (knownFrontends.isEmpty()) {
+                knownFrontends = new HashMap<>();
+            }
+            knownFrontends.put(frontend, new LeaderFrontendState.Disabled(persistenceId(), clientId, getDataStore()));
+
             persistPayload(clientId, DisableTrackingPayload.create(clientId,
                 datastoreContext.getInitialPayloadSerializedBufferCapacity()), false);
         }
@@ -1038,8 +1038,10 @@ public class Shard extends RaftActor {
         paused = true;
 
         // Tell-based protocol can replay transaction state, so it is safe to blow it up when we are paused.
-        knownFrontends.values().forEach(LeaderFrontendState::retire);
-        knownFrontends = ImmutableMap.of();
+        if (datastoreContext.isUseTellBasedProtocol()) {
+            knownFrontends.values().forEach(LeaderFrontendState::retire);
+            knownFrontends = ImmutableMap.of();
+        }
 
         store.setRunOnPendingTransactionsComplete(operation);
     }

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java
index 8961a1a7553edcf91691f9e29f40a4056c41b2b5..87dd5e3e2638fff856352b2f012523ad83631c82 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransaction.java
@@ -22,7 +22,6 @@ import org.opendaylight.controller.cluster.datastore.messages.CloseTransaction;
 import org.opendaylight.controller.cluster.datastore.messages.CloseTransactionReply;
 import org.opendaylight.controller.cluster.datastore.messages.DataExists;
 import org.opendaylight.controller.cluster.datastore.messages.DataExistsReply;
-import org.opendaylight.controller.cluster.datastore.messages.PersistAbortTransactionPayload;
 import org.opendaylight.controller.cluster.datastore.messages.ReadData;
 import org.opendaylight.controller.cluster.datastore.messages.ReadDataReply;
 import org.opendaylight.mdsal.common.api.ReadFailedException;
@@ -78,7 +77,6 @@ public abstract class ShardTransaction extends AbstractUntypedActorWithMetering
 
     private void closeTransaction(final boolean sendReply) {
         getDOMStoreTransaction().abortFromTransactionActor();
-        shardActor.tell(new PersistAbortTransactionPayload(transactionId), ActorRef.noSender());
 
         if (sendReply && returnCloseTransactionReply()) {
             getSender().tell(new CloseTransactionReply(), getSelf());

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/PersistAbortTransactionPayload.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/PersistAbortTransactionPayload.java
deleted file mode 100644 (file)
index b581ddb..0000000
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/PersistAbortTransactionPayload.java
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * Copyright (c) 2017 Cisco Systems, Inc. and others.  All rights reserved.
- *
- * This program and the accompanying materials are made available under the
- * terms of the Eclipse Public License v1.0 which accompanies this distribution,
- * and is available at http://www.eclipse.org/legal/epl-v10.html
- */
-package org.opendaylight.controller.cluster.datastore.messages;
-
-import com.google.common.base.Preconditions;
-import org.opendaylight.controller.cluster.access.concepts.TransactionIdentifier;
-
-/**
- * A request sent from {@link org.opendaylight.controller.cluster.datastore.ShardTransaction} to
- * {@link org.opendaylight.controller.cluster.datastore.Shard} to persist an
- * {@link org.opendaylight.controller.cluster.datastore.persisted.AbortTransactionPayload} after the transaction has
- * been closed by the frontend and internal backend state has been updated.
- *
- * <p>
- * Since the two are actors, we cannot perform a direct upcall, as that breaks actor containment and wreaks havoc into
- * Akka itself. This class does not need to be serializable, as both actors are guaranteed to be co-located.
- *
- * @author Robert Varga
- */
-public final class PersistAbortTransactionPayload {
-    private final TransactionIdentifier txId;
-
-    public PersistAbortTransactionPayload(final TransactionIdentifier txId) {
-        this.txId = Preconditions.checkNotNull(txId);
-    }
-
-    public TransactionIdentifier getTransactionId() {
-        return txId;
-    }
-}

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreRemotingIntegrationTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreRemotingIntegrationTest.java
index 6fd48c534127f2f7770e5aa5426d02b78494c0fb..cddd4375f48620a3a4e9f57b6bc484ebd9592439 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreRemotingIntegrationTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreRemotingIntegrationTest.java
@@ -787,10 +787,10 @@ public class DistributedDataStoreRemotingIntegrationTest extends AbstractTest {
         // Wait for the commit to be replicated to the follower.
 
         MemberNode.verifyRaftState(followerDistributedDataStore, "cars",
-            raftState -> assertEquals("getLastApplied", 1, raftState.getLastApplied()));
+            raftState -> assertEquals("getLastApplied", 2, raftState.getLastApplied()));
 
         MemberNode.verifyRaftState(followerDistributedDataStore, "people",
-            raftState -> assertEquals("getLastApplied", 1, raftState.getLastApplied()));
+            raftState -> assertEquals("getLastApplied", 2, raftState.getLastApplied()));
 
         // Prepare, ready and canCommit a WO tx that writes to 2 shards. This will become the current tx in
         // the leader shard.

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java
index 4dbdce0c079fbfa8e0b53b4304817184a73c2cfc..afc27bb7ae9c21d9cbd0ef0f097475556caf3608 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java
@@ -558,7 +558,7 @@ public class ShardTest extends AbstractShardTest {
 
                 verifyOuterListEntry(shard, 1);
 
-                verifyLastApplied(shard, 5);
+                verifyLastApplied(shard, 6);
             }
         };
     }
@@ -1067,9 +1067,8 @@ public class ShardTest extends AbstractShardTest {
                 // Committed transaction count should increase as usual
                 assertEquals(1, shardStats.getCommittedTransactionsCount());
 
-                // Commit index should advance as we do not have an empty
-                // modification
-                assertEquals(1, shardStats.getCommitIndex());
+                // Commit index should advance 2 to account for disabling metadata
+                assertEquals(2, shardStats.getCommitIndex());
             }
         };
     }