Remove PersistAbortTransactionPayload 17/82217/1
authorTomas Cere <tomas.cere@pantheon.tech>
Tue, 14 May 2019 10:56:04 +0000 (12:56 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 23 May 2019 10:55:55 +0000 (12:55 +0200)
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)

opendaylight/md-sal/sal-cluster-admin-impl/src/test/java/org/opendaylight/controller/cluster/datastore/admin/ClusterAdminRpcServiceTest.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/FrontendMetadata.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/ShardTransaction.java
opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/messages/PersistAbortTransactionPayload.java [deleted file]
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/DistributedDataStoreRemotingIntegrationTest.java
opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTest.java

index 8f9f2b2c27495bcaaaf662b27f1c7b91b38830a2..8411cd4b71081efaeb65d0d9aeb4abcb49a87bcb 100644 (file)
@@ -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);
index e11778da87183bcd7033d28d81e8d175054ca228..f4db772c6b896eb92940b3c88b84cd5dff0dced6 100644 (file)
@@ -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())) {
index 55f932b2f01bca0a582ace6d40ae6f6055be7bc7..be22efaa100150d506fe9253bee60c3e3191283c 100644 (file)
@@ -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 javax.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()));
             }
         }
     }
@@ -874,9 +868,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);
         }
@@ -1036,8 +1036,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);
     }
index 8961a1a7553edcf91691f9e29f40a4056c41b2b5..87dd5e3e2638fff856352b2f012523ad83631c82 100644 (file)
@@ -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
+++ /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;
-    }
-}
index ddd43afa1e89329a30ab94df24916e20fdd273a5..e1c0b7b4c51ada7f8e5ed5a2c3f822f567216f78 100644 (file)
@@ -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.
index b3e8de2d9b70af550bfa0017ed3a0d873125da83..9d4381e145282c547ca4ed326402e104163118bc 100644 (file)
@@ -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