BUG-9054: do not use BatchedModifications needlessly

Transaction identifier, which is a required parameter for
BatchedModifications is a resource tracked on the backend and is
assumed to be allocated contiguously. Using BatchedModifications
to transport only a list of modifications means we are allocating
transactions IDs which we then never use.

This patch reworks the logic so it tracks modifications in a list
and allocates BatchedModifications only when we are ready to actually
commit something.

Change-Id: I3f71511cfd68e96e80790e69d28d083f195e5e12
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 71a4b6377ba598b18c64b89b6b16538751d2d116)

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java
index 1dfe03d3d22c3aa6aa144e67baee081d83037bd3..56b169c5562a30f2703f8321595961e332608a05 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShard.java
@@ -38,6 +38,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -60,6 +61,7 @@ import org.opendaylight.controller.cluster.datastore.messages.PeerUp;
 import org.opendaylight.controller.cluster.datastore.messages.SuccessReply;
 import org.opendaylight.controller.cluster.datastore.modification.DeleteModification;
 import org.opendaylight.controller.cluster.datastore.modification.MergeModification;
+import org.opendaylight.controller.cluster.datastore.modification.Modification;
 import org.opendaylight.controller.cluster.datastore.modification.WriteModification;
 import org.opendaylight.controller.cluster.raft.RaftState;
 import org.opendaylight.mdsal.eos.dom.api.DOMEntity;
@@ -469,7 +471,7 @@ class EntityOwnershipShard extends Shard {
     }
 
     private void selectNewOwnerForEntitiesOwnedBy(final Set<String> ownedBy) {
-        final BatchedModifications modifications = commitCoordinator.newBatchedModifications();
+        final List<Modification> modifications = new ArrayList<>();
         searchForEntitiesOwnedBy(ownedBy, (entityTypeNode, entityNode) -> {
             YangInstanceIdentifier entityPath = YangInstanceIdentifier.builder(ENTITY_TYPES_PATH)
                     .node(entityTypeNode.getIdentifier()).node(ENTITY_NODE_ID).node(entityNode.getIdentifier())
@@ -480,8 +482,8 @@ class EntityOwnershipShard extends Shard {
             if (!newOwner.isEmpty()) {
                 LOG.debug("{}: Found entity {}, writing new owner {}", persistenceId(), entityPath, newOwner);
 
-                modifications.addModification(new WriteModification(entityPath,
-                        ImmutableNodes.leafNode(ENTITY_OWNER_NODE_ID, newOwner)));
+                modifications.add(new WriteModification(entityPath,
+                    ImmutableNodes.leafNode(ENTITY_OWNER_NODE_ID, newOwner)));
 
             } else {
                 LOG.debug("{}: Found entity {} but no other candidates - not clearing owner", persistenceId(),
@@ -538,7 +540,7 @@ class EntityOwnershipShard extends Shard {
     }
 
     private void removeCandidateFromEntities(final MemberName member) {
-        final BatchedModifications modifications = commitCoordinator.newBatchedModifications();
+        final List<Modification> modifications = new ArrayList<>();
         searchForEntities((entityTypeNode, entityNode) -> {
             if (hasCandidate(entityNode, member)) {
                 YangInstanceIdentifier entityId =
@@ -550,7 +552,7 @@ class EntityOwnershipShard extends Shard {
                 LOG.info("{}: Found entity {}, removing candidate {}, path {}", persistenceId(), entityId,
                         member, candidatePath);
 
-                modifications.addModification(new DeleteModification(candidatePath));
+                modifications.add(new DeleteModification(candidatePath));
             }
         });
 

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardCommitCoordinator.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardCommitCoordinator.java
index a6958297443e9cb7e635f300a96c1b3346e715e6..6058e7e997c60a652086cf2ad6a63e1aa39c4fc6 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardCommitCoordinator.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/entityownership/EntityOwnershipShardCommitCoordinator.java
@@ -13,8 +13,10 @@ import akka.actor.ActorRef;
 import akka.actor.Cancellable;
 import akka.actor.Status.Failure;
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import java.util.Iterator;
 import java.util.LinkedList;
+import java.util.List;
 import java.util.Queue;
 import javax.annotation.Nullable;
 import org.opendaylight.controller.cluster.access.concepts.ClientIdentifier;
@@ -158,13 +160,11 @@ class EntityOwnershipShardCommitCoordinator {
     }
 
     void commitModification(Modification modification, EntityOwnershipShard shard) {
-        BatchedModifications modifications = newBatchedModifications();
-        modifications.addModification(modification);
-        commitModifications(modifications, shard);
+        commitModifications(ImmutableList.of(modification), shard);
     }
 
-    void commitModifications(BatchedModifications modifications, EntityOwnershipShard shard) {
-        if (modifications.getModifications().isEmpty()) {
+    void commitModifications(List<Modification> modifications, EntityOwnershipShard shard) {
+        if (modifications.isEmpty()) {
             return;
         }
 
@@ -175,9 +175,10 @@ class EntityOwnershipShardCommitCoordinator {
                         inflightCommit != null ? "A commit is inflight" : "No shard leader");
             }
 
-            pendingModifications.addAll(modifications.getModifications());
+            pendingModifications.addAll(modifications);
         } else {
-            inflightCommit = modifications;
+            inflightCommit = newBatchedModifications();
+            inflightCommit.addModifications(modifications);
             shard.tryCommitModifications(inflightCommit);
         }
     }
@@ -270,7 +271,7 @@ class EntityOwnershipShardCommitCoordinator {
         inflightCommit = newBatchedModifications;
     }
 
-    BatchedModifications newBatchedModifications() {
+    private BatchedModifications newBatchedModifications() {
         BatchedModifications modifications = new BatchedModifications(
             new TransactionIdentifier(historyId, ++transactionIDCounter), DataStoreVersions.CURRENT_VERSION);
         modifications.setDoCommitOnReady(true);