This is an automated email from the ASF dual-hosted git repository.
mkevo pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 5dde7d7 GEODE-8876: Statistics not collecting correct value for gets
when tra… (#6199)
5dde7d7 is described below
commit 5dde7d765c252ef20cfb16981b18e68903e32165
Author: Jakov Varenina <62134331+jvaren...@users.noreply.github.com>
AuthorDate: Mon Oct 4 09:45:40 2021 +0200
GEODE-8876: Statistics not collecting correct value for gets when tra…
(#6199)
* GEODE-8876: Statistics not collecting correct value for gets when
transaction is used
---
.../geode/management/CachePerfStatsDUnitTest.java | 298 +++++++++++++++++++++
.../management/PartitionedRegionStatsTest.java | 76 ------
.../org/apache/geode/cache/ProxyJUnitTest.java | 3 +-
.../apache/geode/internal/cache/BucketRegion.java | 10 +
.../geode/internal/cache/CachePerfStats.java | 4 +
.../apache/geode/internal/cache/LocalRegion.java | 4 +
.../geode/internal/cache/PartitionedRegion.java | 8 +-
.../apache/geode/internal/cache/RegionStats.java | 2 +
.../cache/SearchLoadAndWriteProcessor.java | 6 +
9 files changed, 330 insertions(+), 81 deletions(-)
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/management/CachePerfStatsDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/CachePerfStatsDUnitTest.java
new file mode 100644
index 0000000..0e34cc1
--- /dev/null
+++
b/geode-core/src/distributedTest/java/org/apache/geode/management/CachePerfStatsDUnitTest.java
@@ -0,0 +1,298 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+package org.apache.geode.management;
+
+import static org.apache.geode.test.dunit.VM.getHostName;
+import static org.apache.geode.test.dunit.VM.getVM;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.Serializable;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.CacheTransactionManager;
+import org.apache.geode.cache.PartitionAttributes;
+import org.apache.geode.cache.PartitionAttributesFactory;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientRegionFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.distributed.internal.ServerLocation;
+import org.apache.geode.internal.cache.CachePerfStats;
+import org.apache.geode.internal.cache.DistributedRegion;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.cache.PartitionedRegion;
+import org.apache.geode.internal.cache.PartitionedRegionDataStore;
+import org.apache.geode.internal.cache.TXManagerImpl;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.dunit.rules.CacheRule;
+import org.apache.geode.test.dunit.rules.ClientCacheRule;
+import org.apache.geode.test.dunit.rules.DistributedRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
+
+public class CachePerfStatsDUnitTest implements Serializable {
+
+ private static final int NON_EXISTENT_KEY = 1234;
+
+ private String hostName;
+ private String uniqueName;
+ private String regionNamePartitioned;
+ private String regionNameReplicated;
+ private VM server1;
+ private VM server2;
+ private int port1;
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @Rule
+ public CacheRule cacheRule = new CacheRule();
+
+ @Rule
+ public ClientCacheRule clientCacheRule = new ClientCacheRule();
+
+ @Rule
+ public SerializableTestName testName = new SerializableTestName();
+
+ @Before
+ public void setUp() {
+ server1 = getVM(0);
+ server2 = getVM(1);
+ hostName = getHostName();
+ uniqueName = getClass().getSimpleName() + "_" + testName.getMethodName();
+ regionNamePartitioned = uniqueName + "_partition_region";
+ regionNameReplicated = uniqueName + "_replicated_region";
+ }
+
+ @Test
+ public void getStatsAreIncrementedCorrectlyForPartitionedRegion() {
+ port1 = server1.invoke(this::createServerRegion);
+ createClientRegion(port1);
+
+ int numOfOps = 10;
+ int numOfMissedOps = 3;
+ putData(numOfOps, regionNamePartitioned);
+
+ getData(numOfOps, regionNamePartitioned);
+ getDataMissed(numOfMissedOps, regionNamePartitioned);
+
+ server1.invoke(
+ () -> validateGetsRates(regionNamePartitioned, numOfOps +
numOfMissedOps, numOfMissedOps));
+ }
+
+ @Test
+ public void getStatsAreIncrementedCorrectlyForReplicatedRegion() {
+ port1 = server1.invoke(this::createReplicatedServerRegion);
+ createClientReplicatedRegion(port1);
+
+ int numOfOps = 11;
+ int numOfMissedOps = 20;
+ putData(numOfOps, regionNameReplicated);
+
+ getData(numOfOps, regionNameReplicated);
+ getDataMissed(numOfMissedOps, regionNameReplicated);
+
+ server1.invoke(() -> validateGetsRatesReplicate(regionNameReplicated,
numOfOps + numOfMissedOps,
+ numOfMissedOps));
+ }
+
+ @Test
+ public void transactionGetStatsAreIncrementedCorrectlyForReplicatedRegion() {
+ port1 = server1.invoke(this::createReplicatedServerRegion);
+ createClientReplicatedRegion(port1);
+
+ CacheTransactionManager txManager =
+ clientCacheRule.getClientCache().getCacheTransactionManager();
+
+ int numOfOps = 10;
+ int numOfMissedOps = 3;
+
+ txManager.begin();
+ putData(numOfOps, regionNameReplicated);
+ txManager.commit();
+
+ txManager.begin();
+ getData(numOfOps, regionNameReplicated);
+ getDataMissed(numOfMissedOps, regionNameReplicated);
+ txManager.commit();
+
+ server1.invoke(() -> validateGetsRatesReplicate(regionNameReplicated,
numOfOps + numOfMissedOps,
+ numOfMissedOps));
+ }
+
+ @Test
+ public void transactionGetStatsAreIncrementedCorrectlyForPartitionRegion() {
+ port1 = server1.invoke(this::createServerRegion);
+ createClientRegion(port1);
+
+ CacheTransactionManager txManager =
+ clientCacheRule.getClientCache().getCacheTransactionManager();
+
+ int numOfOps = 10;
+ int numOfMissedOps = 3;
+
+ txManager.begin();
+ putData(numOfOps, regionNamePartitioned);
+ txManager.commit();
+
+ txManager.begin();
+ getData(numOfOps, regionNamePartitioned);
+ getDataMissed(numOfMissedOps, regionNamePartitioned);
+ txManager.commit();
+
+ server1.invoke(
+ () -> validateGetsRates(regionNamePartitioned, numOfOps +
numOfMissedOps, numOfMissedOps));
+ }
+
+ @Test
+ public void
transactionGetStatsAreIncrementedCorrectlyForPartitionRegionWhenRemoteGetIsPerformed()
{
+ port1 = server1.invoke(this::createServerRegion);
+ server2.invoke(this::createServerRegion);
+ createClientRegion(port1);
+
+ int numOfOps = 10;
+ putData(numOfOps, regionNamePartitioned);
+
+ getDataTransaction(numOfOps, regionNamePartitioned);
+
+ long numberOfMisses = server2.invoke(() -> {
+ InternalCache cache = cacheRule.getCache();
+ PartitionedRegion region = (PartitionedRegion)
cache.getRegion(regionNamePartitioned);
+ PartitionedRegionDataStore dataStore = region.getDataStore();
+
+ long numberOfEntries = 0;
+ for (int i = 0; i <= 113; i++) {
+ if (dataStore.getLocalBucketById(i) != null) {
+ numberOfEntries++;
+ }
+ }
+ return numberOfEntries;
+ });
+
+ server1.invoke(
+ () -> validateGetsRates(regionNamePartitioned, numOfOps, (int)
numberOfMisses));
+ }
+
+ private void putData(int numberOfEntries, String name) {
+ Region<Object, Object> region =
clientCacheRule.getClientCache().getRegion(name);
+ for (int key = 0; key < numberOfEntries; key++) {
+ String value = "value" + key;
+ region.put(key, value);
+ }
+ }
+
+ private void getData(int numberOfEntries, String name) {
+ Region<Object, Object> region =
clientCacheRule.getClientCache().getRegion(name);
+ for (int key = 0; key < numberOfEntries; key++) {
+ region.get(key);
+ }
+ }
+
+ private void getDataTransaction(int numberOfEntries, String name) {
+ TXManagerImpl txManager =
+ (TXManagerImpl)
clientCacheRule.getClientCache().getCacheTransactionManager();
+ Region<Object, Object> region =
clientCacheRule.getClientCache().getRegion(name);
+ for (int key = 0; key < numberOfEntries; key++) {
+ txManager.begin();
+ region.get(key);
+ txManager.commit();
+ }
+ }
+
+ private void getDataMissed(int numberOfMissedGets, String name) {
+ Region<Object, Object> region =
clientCacheRule.getClientCache().getRegion(name);
+ for (int key = 0; key < numberOfMissedGets; key++) {
+ region.get(NON_EXISTENT_KEY);
+ }
+ }
+
+ private int createServerRegion() throws Exception {
+ PartitionAttributesFactory<Object, Object> factory = new
PartitionAttributesFactory<>();
+ PartitionAttributes<Object, Object> partitionAttributes = factory.create();
+ cacheRule.getOrCreateCache().createRegionFactory(RegionShortcut.PARTITION)
+
.setPartitionAttributes(partitionAttributes).create(regionNamePartitioned);
+ CacheServer server = cacheRule.getCache().addCacheServer();
+ server.setPort(0);
+ server.start();
+ return server.getPort();
+ }
+
+ private int createReplicatedServerRegion() throws Exception {
+ cacheRule.getOrCreateCache().createRegionFactory(RegionShortcut.REPLICATE)
+ .create(regionNameReplicated);
+ CacheServer server = cacheRule.getCache().addCacheServer();
+ server.setPort(0);
+ server.start();
+ return server.getPort();
+ }
+
+ private void createClientRegion(int port) {
+ clientCacheRule.createClientCache();
+ PoolImpl pool = getPool(port);
+ ClientRegionFactory<Object, Object> crf =
+
clientCacheRule.getClientCache().createClientRegionFactory(ClientRegionShortcut.PROXY);
+ crf.setPoolName(pool.getName());
+ crf.create(regionNamePartitioned);
+ pool.acquireConnection(new ServerLocation(hostName, port));
+ }
+
+ private void createClientReplicatedRegion(int port) {
+ clientCacheRule.createClientCache();
+ PoolImpl pool = getPool(port);
+ ClientRegionFactory<Object, Object> crf =
+
clientCacheRule.getClientCache().createClientRegionFactory(ClientRegionShortcut.PROXY);
+ crf.setPoolName(pool.getName());
+ crf.create(regionNameReplicated);
+ pool.acquireConnection(new ServerLocation(hostName, port));
+ }
+
+ private PoolImpl getPool(int port) {
+ PoolFactory factory = PoolManager.createFactory();
+ factory.addServer(hostName, port);
+ return (PoolImpl) factory.create(uniqueName);
+ }
+
+ private void validateGetsRates(final String regionName, final int
expectedCount,
+ final int expectedMissed) {
+ InternalCache cache = cacheRule.getCache();
+ PartitionedRegion region = (PartitionedRegion) cache.getRegion(regionName);
+ CachePerfStats regionPerfStats = region.getCachePerfStats();
+ checkGetsStats(regionPerfStats, expectedCount, expectedMissed);
+ CachePerfStats cachePerfStats = cache.getCachePerfStats();
+ checkGetsStats(cachePerfStats, expectedCount, expectedMissed);
+ }
+
+ private void validateGetsRatesReplicate(final String regionName, final int
expectedCount,
+ final int expectedMissed) {
+ InternalCache cache = cacheRule.getCache();
+ DistributedRegion region = (DistributedRegion) cache.getRegion(regionName);
+ CachePerfStats regionPerfStats = region.getCachePerfStats();
+ checkGetsStats(regionPerfStats, expectedCount, expectedMissed);
+ CachePerfStats cachePerfStats = cache.getCachePerfStats();
+ checkGetsStats(cachePerfStats, expectedCount, expectedMissed);
+ }
+
+ private void checkGetsStats(CachePerfStats cachePerfStats, int expectedCount,
+ int expectedMissed) {
+ assertThat(cachePerfStats.getGets()).isEqualTo(expectedCount);
+ assertThat(cachePerfStats.getMisses()).isEqualTo(expectedMissed);
+ }
+}
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/management/PartitionedRegionStatsTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/management/PartitionedRegionStatsTest.java
deleted file mode 100644
index 578cd78..0000000
---
a/geode-core/src/distributedTest/java/org/apache/geode/management/PartitionedRegionStatsTest.java
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
- * agreements. See the NOTICE file distributed with this work for additional
information regarding
- * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the
License. You may obtain a
- * copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
distributed under the License
- * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
- * or implied. See the License for the specific language governing permissions
and limitations under
- * the License.
- */
-package org.apache.geode.management;
-
-import static org.apache.geode.security.SecurityTestUtil.createClientCache;
-import static org.apache.geode.security.SecurityTestUtil.createProxyRegion;
-import static org.assertj.core.api.Assertions.assertThat;
-
-import org.junit.Rule;
-import org.junit.Test;
-
-import org.apache.geode.cache.Region;
-import org.apache.geode.cache.RegionShortcut;
-import org.apache.geode.cache.client.ClientCache;
-import org.apache.geode.internal.cache.CachePerfStats;
-import org.apache.geode.internal.cache.PartitionedRegion;
-import org.apache.geode.test.dunit.Host;
-import org.apache.geode.test.dunit.VM;
-import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
-import org.apache.geode.test.junit.rules.ServerStarterRule;
-
-public class PartitionedRegionStatsTest extends JUnit4DistributedTestCase {
-
- private static String REGION_NAME = "testRegion";
- private final int numOfEntries = 10;
-
- final Host host = Host.getHost(0);
- final VM client1 = host.getVM(1);
-
- @Rule
- public ServerStarterRule server =
- new
ServerStarterRule().withAutoStart().withRegion(RegionShortcut.PARTITION,
REGION_NAME);
-
- @Test
- public void testGetsRate() throws Exception {
-
- client1.invoke(() -> {
-
- ClientCache clientCache = createClientCache("superUser", "123",
server.getPort());
- Region region = createProxyRegion(clientCache, REGION_NAME);
-
- for (int i = 1; i < numOfEntries; i++) {
- region.put("key" + i, "value" + i);
- region.get("key" + i);
- }
- region.get("key" + numOfEntries);
-
- });
-
- assertThat(server.getCache()).isNotNull();
-
- CachePerfStats regionStats =
- ((PartitionedRegion)
server.getCache().getRegion(REGION_NAME)).getCachePerfStats();
-
- CachePerfStats cacheStats = server.getCache().getCachePerfStats();
-
- assertThat(regionStats.getGets()).isEqualTo(numOfEntries);
- assertThat(regionStats.getMisses()).isEqualTo(1);
-
- assertThat(cacheStats.getGets()).isEqualTo(numOfEntries);
- assertThat(cacheStats.getMisses()).isEqualTo(1);
-
- }
-}
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java
index 6491653..af0e7a9 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/cache/ProxyJUnitTest.java
@@ -804,10 +804,9 @@ public class ProxyJUnitTest {
public void close() {}
};
r.getAttributesMutator().setCacheLoader(cl);
- r.get("key", cbArg);
+ assertEquals("loadedValue", r.get("key", cbArg));
gets++;
assertEquals(gets, getStats().getGets());
- misses++;
assertEquals(misses, getStats().getMisses());
expee.op = Operation.LOCAL_LOAD_CREATE;
expee.newValue = "loadedValue";
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
index fbabc2e..594704a 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java
@@ -294,6 +294,16 @@ public class BucketRegion extends DistributedRegion
implements Bucket {
}
}
+ @Override
+ protected long startGet() {
+ return 0;
+ }
+
+ @Override
+ protected void endGet(long start, boolean isMiss) {
+ // get stats are recorded by the PartitionedRegion
+ // so nothing is needed on the BucketRegion.
+ }
@Override
void initialized() {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
index 0e9f7ac..790cdc5 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/CachePerfStats.java
@@ -1131,6 +1131,10 @@ public class CachePerfStats {
}
}
+ public void incMisses() {
+ stats.incLong(missesId, 1L);
+ }
+
public void endGetForClient(long start, boolean miss) {}
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index af28668..31c3d6d 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -1381,6 +1381,7 @@ public class LocalRegion extends AbstractRegion
implements LoaderHelperFactory,
if (!returnTombstones && value == Token.TOMBSTONE) {
value = null;
}
+ isMiss = value == null;
} else {
// local scope with no loader, still might need to update stats
if (isCreate) {
@@ -2801,6 +2802,9 @@ public class LocalRegion extends AbstractRegion
implements LoaderHelperFactory,
holder = new VersionTagHolder();
value = mySRP.get(key, aCallbackArgument, holder);
fromServer = value != null;
+ if (fromServer) {
+ getCachePerfStats().incMisses();
+ }
}
/*
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
index e7c4f21..e695b1f 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
@@ -3315,13 +3315,12 @@ public class PartitionedRegion extends LocalRegion
@Override
protected long startGet() {
- return 0;
+ return getCachePerfStats().startGet();
}
@Override
protected void endGet(long start, boolean isMiss) {
- // get stats are recorded by the BucketRegion
- // so nothing is needed on the PartitionedRegion.
+ getCachePerfStats().endGet(start, isMiss);
}
public InternalDistributedMember getOrCreateNodeForBucketRead(int bucketId) {
@@ -4843,6 +4842,9 @@ public class PartitionedRegion extends LocalRegion
if (logger.isDebugEnabled()) {
logger.debug("getRemotely: got value {} for key {}", value, key);
}
+ if (value != null) {
+ getCachePerfStats().incMisses();
+ }
return value;
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java
index e5e76ad..dd097db 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/RegionStats.java
@@ -89,6 +89,8 @@ public interface RegionStats {
void endGet(long start, boolean miss);
+ void incMisses();
+
void endGetForClient(long start, boolean miss);
long endPut(long start, boolean isUpdate);
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
index 55f1603..df767c6 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/SearchLoadAndWriteProcessor.java
@@ -600,6 +600,9 @@ public class SearchLoadAndWriteProcessor implements
MembershipListener {
return;
}
} finally {
+ if (this.result != null) {
+ stats.incMisses();
+ }
stats.endNetsearch(start);
}
}
@@ -766,6 +769,9 @@ public class SearchLoadAndWriteProcessor implements
MembershipListener {
} while (stayInLoop);
} finally {
+ if (this.result != null) {
+ stats.incMisses();
+ }
stats.endNetload(start);
}
