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>

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 b87c07e186910fcd4c30a19e08a61f86360a04cf..a29df3f4b6353374f170a57a0bd5ab19c4f0188a 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 f651efa3fab3c14933aa3a386ea7b8c8b8477576..efb0fdc04beb351845e6a4465298a705f5ba8156 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
@@ -132,7 +132,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 718b0641dacaedd33af3852497a074ec236a84fa..8e00aa6091adafb24c4ef2c8636cfaae7f34e091 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
@@ -30,6 +30,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 org.eclipse.jdt.annotation.NonNull;
@@ -77,16 +78,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;
@@ -365,12 +363,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)) {
@@ -461,6 +453,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()));
             }
         }
     }
@@ -871,9 +865,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);
         }
@@ -1032,8 +1032,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 3943e7ee563325f2fb371182ef6d18413b1dd7f9..40285102243c64d01486d1f30f97e3319168df0c 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 b3e8de2d9b70af550bfa0017ed3a0d873125da83..9d4381e145282c547ca4ed326402e104163118bc 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
@@ -551,7 +551,7 @@ public class ShardTest extends AbstractShardTest {
 
         verifyOuterListEntry(shard, 1);
 
-        verifyLastApplied(shard, 5);
+        verifyLastApplied(shard, 6);
     }
 
     @Test
@@ -1026,9 +1026,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());
     }
 
     @Test