You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2019/05/31 23:57:55 UTC
[geode] branch develop updated: GEODE-6472 Fix double increment
gets stat for partitioned region (#3640)
This is an automated email from the ASF dual-hosted git repository.
dschneider 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 cd2eae3 GEODE-6472 Fix double increment gets stat for partitioned region (#3640)
cd2eae3 is described below
commit cd2eae334dce0cd78c6f690c2dc489c8344258f6
Author: mkevo <48...@users.noreply.github.com>
AuthorDate: Sat Jun 1 01:57:41 2019 +0200
GEODE-6472 Fix double increment gets stat for partitioned region (#3640)
This also fixes the "misses" stat on partitioned region.
---
.../management/PartitionedRegionStatsTest.java | 76 ++++++++++++++++++++++
.../apache/geode/internal/cache/LocalRegion.java | 13 +++-
.../geode/internal/cache/PartitionedRegion.java | 12 +++-
3 files changed, 97 insertions(+), 4 deletions(-)
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
new file mode 100644
index 0000000..578cd78
--- /dev/null
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/PartitionedRegionStatsTest.java
@@ -0,0 +1,76 @@
+/*
+ * 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/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 5149d50..9afcece 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
@@ -1349,8 +1349,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
checkReadiness();
checkForNoAccess();
discoverJTA();
- CachePerfStats stats = getCachePerfStats();
- long start = stats.startGet();
+ long start = startGet();
boolean isMiss = true;
try {
KeyInfo keyInfo = getKeyInfo(key, aCallbackArgument);
@@ -1381,10 +1380,18 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
}
return value;
} finally {
- stats.endGet(start, isMiss);
+ endGet(start, isMiss);
}
}
+ protected long startGet() {
+ return getCachePerfStats().startGet();
+ }
+
+ protected void endGet(long start, boolean isMiss) {
+ getCachePerfStats().endGet(start, isMiss);
+ }
+
/**
* Update region and potentially entry stats for the miss case
*
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 67ac4e7..d2941d8 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
@@ -3220,7 +3220,6 @@ public class PartitionedRegion extends LocalRegion
checkForNoAccess();
discoverJTA();
boolean miss = true;
-
Object value = getDataView().findObject(getKeyInfo(key, aCallbackArgument), this,
true/* isCreate */, generateCallbacks, null /* no local value */, disableCopyOnRead,
preferCD, requestingClient, clientEvent, returnTombstones);
@@ -3230,6 +3229,17 @@ public class PartitionedRegion extends LocalRegion
return value;
}
+ @Override
+ protected long startGet() {
+ return 0;
+ }
+
+ @Override
+ protected void endGet(long start, boolean isMiss) {
+ // get stats are recorded by the BucketRegion
+ // so nothing is needed on the PartitionedRegion.
+ }
+
public InternalDistributedMember getOrCreateNodeForBucketRead(int bucketId) {
InternalDistributedMember targetNode = getNodeForBucketRead(bucketId);
if (targetNode != null) {