From: Tomas Cere Date: Tue, 21 May 2019 11:42:47 +0000 (+0200) Subject: Fix write transaction shard stats X-Git-Tag: release/sodium~76 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=c9ca1a93d44cfb7681079b7ca3c56ea89ac3ef1b;ds=sidebyside Fix write transaction shard stats With the changes in ask protocol due to to tell based integration, shards stats had incorrectly reported write transaction counts. Since backend doesnt care about the difference between write-only and read-write transactions and all are read-write from backend point of view we can safely remove writeOnly counter. Change-Id: I18046ad3b229755bbd0daa788a3fe62f59e4140d Signed-off-by: Tomas Cere --- diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java index d22e028663..8a3140b43f 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardDataTree.java @@ -547,6 +547,8 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { } ReadOnlyShardDataTreeTransaction newReadOnlyTransaction(final TransactionIdentifier txId) { + shard.getShardMBean().incrementReadOnlyTransactionCount(); + if (txId.getHistoryId().getHistoryId() == 0) { return new ReadOnlyShardDataTreeTransaction(this, txId, dataTree.takeSnapshot()); } @@ -555,6 +557,8 @@ public class ShardDataTree extends ShardDataTreeTransactionParent { } ReadWriteShardDataTreeTransaction newReadWriteTransaction(final TransactionIdentifier txId) { + shard.getShardMBean().incrementReadWriteTransactionCount(); + if (txId.getHistoryId().getHistoryId() == 0) { return new ReadWriteShardDataTreeTransaction(ShardDataTree.this, txId, dataTree.takeSnapshot() .newModification()); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java index 3cd09ab6ef..26c4086c2e 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/ShardTransactionActorFactory.java @@ -68,15 +68,10 @@ class ShardTransactionActorFactory { switch (type) { case READ_ONLY: transaction = dataTree.newReadOnlyTransaction(transactionID); - shardMBean.incrementReadOnlyTransactionCount(); break; case READ_WRITE: - transaction = dataTree.newReadWriteTransaction(transactionID); - shardMBean.incrementReadWriteTransactionCount(); - break; case WRITE_ONLY: transaction = dataTree.newReadWriteTransaction(transactionID); - shardMBean.incrementWriteOnlyTransactionCount(); break; default: throw new IllegalArgumentException("Unsupported transaction type " + type); diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStats.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStats.java index 7f81370a87..954032a323 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStats.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStats.java @@ -44,8 +44,6 @@ public class ShardStats extends AbstractMXBean implements ShardStatsMXBean { private long readOnlyTransactionCount; - private long writeOnlyTransactionCount; - private long readWriteTransactionCount; private long lastCommittedTransactionTime; @@ -113,11 +111,6 @@ public class ShardStats extends AbstractMXBean implements ShardStatsMXBean { return readOnlyTransactionCount; } - @Override - public long getWriteOnlyTransactionCount() { - return writeOnlyTransactionCount; - } - @Override public long getReadWriteTransactionCount() { return readWriteTransactionCount; @@ -221,10 +214,6 @@ public class ShardStats extends AbstractMXBean implements ShardStatsMXBean { return ++readOnlyTransactionCount; } - public long incrementWriteOnlyTransactionCount() { - return ++writeOnlyTransactionCount; - } - public long incrementReadWriteTransactionCount() { return ++readWriteTransactionCount; } @@ -264,8 +253,6 @@ public class ShardStats extends AbstractMXBean implements ShardStatsMXBean { readOnlyTransactionCount = 0; - writeOnlyTransactionCount = 0; - readWriteTransactionCount = 0; lastCommittedTransactionTime = 0; diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStatsMXBean.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStatsMXBean.java index bdaf26aea0..ac76891b69 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStatsMXBean.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/datastore/jmx/mbeans/shard/ShardStatsMXBean.java @@ -28,8 +28,6 @@ public interface ShardStatsMXBean { long getReadOnlyTransactionCount(); - long getWriteOnlyTransactionCount(); - long getReadWriteTransactionCount(); long getLastLogIndex(); 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 3943e7ee56..0fb3a41c7d 100644 --- 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 @@ -843,7 +843,7 @@ public class DistributedDataStoreRemotingIntegrationTest extends AbstractTest { readWriteTx.write(CarsModel.newCarPath("car" + carIndex), cars.getLast()); IntegrationTestKit.verifyShardStats(leaderDistributedDataStore, "cars", - stats -> assertEquals("getReadWriteTransactionCount", 1, stats.getReadWriteTransactionCount())); + stats -> assertEquals("getReadWriteTransactionCount", 5, stats.getReadWriteTransactionCount())); // Disable elections on the leader so it switches to follower. diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java index 65ca16a0f5..2eec393304 100644 --- a/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java +++ b/opendaylight/md-sal/sal-distributed-datastore/src/test/java/org/opendaylight/controller/cluster/datastore/ShardTransactionFailureTest.java @@ -7,12 +7,15 @@ */ package org.opendaylight.controller.cluster.datastore; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + import akka.actor.ActorRef; import akka.actor.Props; import akka.testkit.TestActorRef; import java.util.concurrent.TimeUnit; +import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; import org.opendaylight.controller.cluster.access.concepts.MemberName; import org.opendaylight.controller.cluster.datastore.identifiers.ShardIdentifier; import org.opendaylight.controller.cluster.datastore.jmx.mbeans.shard.ShardStats; @@ -37,7 +40,7 @@ public class ShardTransactionFailureTest extends AbstractActorTest { private static final TransactionType RO = TransactionType.READ_ONLY; private static final TransactionType RW = TransactionType.READ_WRITE; - private static final Shard MOCK_SHARD = Mockito.mock(Shard.class); + private static final Shard MOCK_SHARD = mock(Shard.class); private static final ShardDataTree STORE = new ShardDataTree(MOCK_SHARD, TEST_SCHEMA_CONTEXT, TreeType.OPERATIONAL); @@ -55,6 +58,12 @@ public class ShardTransactionFailureTest extends AbstractActorTest { return shard; } + @Before + public void setup() { + ShardStats stats = mock(ShardStats.class); + when(MOCK_SHARD.getShardMBean()).thenReturn(stats); + } + @Test(expected = ReadFailedException.class) public void testNegativeReadWithReadOnlyTransactionClosed() throws Exception {