From 88a6b2f0a11ac2318795b0e9124ac596a386304a Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Wed, 30 Aug 2017 16:28:28 +0200 Subject: [PATCH] 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 (cherry picked from commit 71a4b6377ba598b18c64b89b6b16538751d2d116) --- .../entityownership/EntityOwnershipShard.java | 12 +++++++----- .../EntityOwnershipShardCommitCoordinator.java | 17 +++++++++-------- 2 files changed, 16 insertions(+), 13 deletions(-) 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 1dfe03d3d2..56b169c556 100644 --- 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 ownedBy) { - final BatchedModifications modifications = commitCoordinator.newBatchedModifications(); + final List 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 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 a695829744..6058e7e997 100644 --- 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 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); -- 2.36.6