You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/10/14 17:01:15 UTC
[GitHub] [geode] mhansonp opened a new pull request #5623: GEODE-8607: Adding some test API to get oldest tombstone
mhansonp opened a new pull request #5623:
URL: https://github.com/apache/geode/pull/5623
- Need an API to tell how many how long until the oldest tombstone expires.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mhansonp merged pull request #5623: GEODE-8607: Adding some test API to get oldest tombstone
Posted by GitBox <gi...@apache.org>.
mhansonp merged pull request #5623:
URL: https://github.com/apache/geode/pull/5623
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #5623: GEODE-8607: Adding some test API to get oldest tombstone
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5623:
URL: https://github.com/apache/geode/pull/5623#discussion_r509453172
##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/versions/TombstoneDUnitTest.java
##########
@@ -15,127 +15,226 @@
package org.apache.geode.internal.cache.versions;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import java.io.Serializable;
+import java.util.Arrays;
import java.util.Properties;
import java.util.concurrent.CountDownLatch;
+import org.junit.After;
+import org.junit.Rule;
import org.junit.Test;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
import org.apache.geode.cache.Region;
import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
import org.apache.geode.distributed.internal.ClusterDistributionManager;
import org.apache.geode.distributed.internal.DistributionMessage;
import org.apache.geode.distributed.internal.DistributionMessageObserver;
import org.apache.geode.internal.cache.DestroyOperation;
import org.apache.geode.internal.cache.DistributedTombstoneOperation;
+import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.internal.cache.TombstoneService;
import org.apache.geode.test.dunit.AsyncInvocation;
-import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.NetworkUtils;
import org.apache.geode.test.dunit.VM;
-import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.rules.DistributedRule;
-public class TombstoneDUnitTest extends JUnit4CacheTestCase {
+public class TombstoneDUnitTest implements Serializable {
+ private static final long serialVersionUID = 2992716917694662945L;
+ private static Cache cache;
+ private static Region<String, String> region;
+ final String REGION_NAME = "TestRegion";
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @After
+ public void close() {
+ for (VM vm : Arrays.asList(VM.getVM(0), VM.getVM(1))) {
+ vm.invoke(() -> {
+ region = null;
+ if (cache != null) {
+ cache.close();
+ }
+ });
+ }
+ }
@Test
- public void testTombstoneGcMessagesBetweenPersistnentAndNonPersistentRegion() {
- Host host = Host.getHost(0);
- VM vm0 = host.getVM(0);
- VM vm1 = host.getVM(1);
+ public void testTombstoneGcMessagesBetweenPersistentAndNonPersistentRegion() {
+ VM vm0 = VM.getVM(0);
+ VM vm1 = VM.getVM(1);
vm0.invoke(() -> {
- createRegion("TestRegion", true);
- Region<String, String> region = getCache().getRegion("TestRegion");
+ createCacheAndRegion(RegionShortcut.REPLICATE_PERSISTENT);
region.put("K1", "V1");
region.put("K2", "V2");
});
- vm1.invoke(() -> {
- createRegion("TestRegion", false);
- });
+ vm1.invoke(() -> createCacheAndRegion(RegionShortcut.REPLICATE));
vm0.invoke(() -> {
// Send tombstone gc message to vm1.
- Region<String, String> region = getCache().getRegion("TestRegion");
region.destroy("K1");
- assertEquals(1, getGemfireCache().getCachePerfStats().getTombstoneCount());
- performGC();
+ assertEquals(1, ((InternalCache) cache).getCachePerfStats().getTombstoneCount());
+ performGC(1);
});
vm1.invoke(() -> {
// After processing tombstone message from vm0. The tombstone count should be 0.
waitForTombstoneCount(0);
- assertEquals(0, getGemfireCache().getCachePerfStats().getTombstoneCount());
+ assertEquals(0, ((InternalCache) cache).getCachePerfStats().getTombstoneCount());
// Send tombstone gc message to vm0.
- Region<String, String> region = getCache().getRegion("TestRegion");
region.destroy("K2");
- performGC();
+ performGC(1);
});
vm0.invoke(() -> {
// After processing tombstone message from vm0. The tombstone count should be 0.
waitForTombstoneCount(0);
- assertEquals(0, getGemfireCache().getCachePerfStats().getTombstoneCount());
+ assertEquals(0, ((InternalCache) cache).getCachePerfStats().getTombstoneCount());
});
}
@Test
- public void testTombstonesWithLowerVersionThanTheRecordedVersionGetsGCed() throws Exception {
- Host host = Host.getHost(0);
- VM vm0 = host.getVM(0);
- VM vm1 = host.getVM(1);
+ public void TestGetOldestTombstoneTimeReplicate() {
Review comment:
Change the test name to start with lower-case `test`?
##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/versions/TombstoneDUnitTest.java
##########
@@ -15,127 +15,226 @@
package org.apache.geode.internal.cache.versions;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import java.io.Serializable;
+import java.util.Arrays;
import java.util.Properties;
import java.util.concurrent.CountDownLatch;
+import org.junit.After;
+import org.junit.Rule;
import org.junit.Test;
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
import org.apache.geode.cache.Region;
import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.ClientRegionShortcut;
import org.apache.geode.distributed.internal.ClusterDistributionManager;
import org.apache.geode.distributed.internal.DistributionMessage;
import org.apache.geode.distributed.internal.DistributionMessageObserver;
import org.apache.geode.internal.cache.DestroyOperation;
import org.apache.geode.internal.cache.DistributedTombstoneOperation;
+import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.internal.cache.TombstoneService;
import org.apache.geode.test.dunit.AsyncInvocation;
-import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.NetworkUtils;
import org.apache.geode.test.dunit.VM;
-import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.rules.DistributedRule;
-public class TombstoneDUnitTest extends JUnit4CacheTestCase {
+public class TombstoneDUnitTest implements Serializable {
+ private static final long serialVersionUID = 2992716917694662945L;
+ private static Cache cache;
+ private static Region<String, String> region;
+ final String REGION_NAME = "TestRegion";
+
+ @Rule
+ public DistributedRule distributedRule = new DistributedRule();
+
+ @After
+ public void close() {
+ for (VM vm : Arrays.asList(VM.getVM(0), VM.getVM(1))) {
+ vm.invoke(() -> {
+ region = null;
+ if (cache != null) {
+ cache.close();
+ }
+ });
+ }
+ }
@Test
- public void testTombstoneGcMessagesBetweenPersistnentAndNonPersistentRegion() {
- Host host = Host.getHost(0);
- VM vm0 = host.getVM(0);
- VM vm1 = host.getVM(1);
+ public void testTombstoneGcMessagesBetweenPersistentAndNonPersistentRegion() {
+ VM vm0 = VM.getVM(0);
+ VM vm1 = VM.getVM(1);
vm0.invoke(() -> {
- createRegion("TestRegion", true);
- Region<String, String> region = getCache().getRegion("TestRegion");
+ createCacheAndRegion(RegionShortcut.REPLICATE_PERSISTENT);
region.put("K1", "V1");
region.put("K2", "V2");
});
- vm1.invoke(() -> {
- createRegion("TestRegion", false);
- });
+ vm1.invoke(() -> createCacheAndRegion(RegionShortcut.REPLICATE));
vm0.invoke(() -> {
// Send tombstone gc message to vm1.
- Region<String, String> region = getCache().getRegion("TestRegion");
region.destroy("K1");
- assertEquals(1, getGemfireCache().getCachePerfStats().getTombstoneCount());
- performGC();
+ assertEquals(1, ((InternalCache) cache).getCachePerfStats().getTombstoneCount());
+ performGC(1);
});
vm1.invoke(() -> {
// After processing tombstone message from vm0. The tombstone count should be 0.
waitForTombstoneCount(0);
- assertEquals(0, getGemfireCache().getCachePerfStats().getTombstoneCount());
+ assertEquals(0, ((InternalCache) cache).getCachePerfStats().getTombstoneCount());
// Send tombstone gc message to vm0.
- Region<String, String> region = getCache().getRegion("TestRegion");
region.destroy("K2");
- performGC();
+ performGC(1);
});
vm0.invoke(() -> {
// After processing tombstone message from vm0. The tombstone count should be 0.
waitForTombstoneCount(0);
- assertEquals(0, getGemfireCache().getCachePerfStats().getTombstoneCount());
+ assertEquals(0, ((InternalCache) cache).getCachePerfStats().getTombstoneCount());
});
}
@Test
- public void testTombstonesWithLowerVersionThanTheRecordedVersionGetsGCed() throws Exception {
- Host host = Host.getHost(0);
- VM vm0 = host.getVM(0);
- VM vm1 = host.getVM(1);
+ public void TestGetOldestTombstoneTimeReplicate() {
+ VM server1 = VM.getVM(0);
+ VM server2 = VM.getVM(1);
+
+ server1.invoke(() -> {
+ createCacheAndRegion(RegionShortcut.REPLICATE_PERSISTENT);
+ region.put("K1", "V1");
+ region.put("K2", "V2");
+ });
+
+ server2.invoke(() -> createCacheAndRegion(RegionShortcut.REPLICATE));
+
+ server1.invoke(() -> {
+ // Send tombstone gc message to vm1.
+ region.destroy("K1");
+
+ TombstoneService.TombstoneSweeper tombstoneSweeper =
+ ((InternalCache) cache).getTombstoneService().getSweeper((LocalRegion) region);
+
+ assertThat(tombstoneSweeper.getOldestTombstoneTime()).isGreaterThan(0)
+ .isLessThan(((InternalCache) cache).cacheTimeMillis());
+ performGC(1);
+ assertThat(tombstoneSweeper.getOldestTombstoneTime()).isEqualTo(0);
+ });
+ }
+
+ @Test
+ public void TestGetOldestTombstoneTimeNonReplicate() {
+ VM client = VM.getVM(0);
+ VM server = VM.getVM(1);
+
+ // Fire up the server and put in some data that is deletable
+ server.invoke(() -> {
+ createCacheAndRegion(RegionShortcut.REPLICATE);
+ cache.addCacheServer().start();
+ for (int i = 0; i < 1000; i++) {
+ region.put("K" + i, "V" + i);
+ }
+ });
- createCache(vm0);
- createCache(vm1);
+ String locatorHost = NetworkUtils.getServerHostName();
+ int locatorPort = DistributedRule.getLocatorPort();
+ // Use the client to remove and entry, thus creating a tombstone
+ client.invoke(() -> {
+ createClientCacheAndRegion(locatorHost, locatorPort);
+ region.remove("K3");
+ });
+
+ // Validate that a tombstone was created and that it has a timestamp that is valid,
+ // Then GC and validate there is no oldest tombstone.
+ server.invoke(() -> {
+ TombstoneService.TombstoneSweeper tombstoneSweeper =
+ ((InternalCache) cache).getTombstoneService().getSweeper((LocalRegion) region);
+
+ assertThat(tombstoneSweeper.getOldestTombstoneTime()).isGreaterThan(0)
+ .isLessThan(((InternalCache) cache).cacheTimeMillis());
+ performGC(1);
+ assertThat(tombstoneSweeper.getOldestTombstoneTime()).isEqualTo(0);
+ });
+ }
+
+ @Test
+ public void testTombstonesWithLowerVersionThanTheRecordedVersionGetsGCed() throws Exception {
+ VM vm0 = VM.getVM(0);
+ VM vm1 = VM.getVM(1);
+ Properties props = DistributedRule.getDistributedSystemProperties();
+ props.put("conserve-sockets", "false");
Review comment:
`props.setProperty` is more correct than using the Map interface method `put`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] agingade commented on a change in pull request #5623: GEODE-8607: Adding some test API to get oldest tombstone
Posted by GitBox <gi...@apache.org>.
agingade commented on a change in pull request #5623:
URL: https://github.com/apache/geode/pull/5623#discussion_r504938535
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
##########
@@ -450,6 +450,11 @@ boolean testHook_forceExpiredTombstoneGC(int count, long timeout, TimeUnit unit)
@Override
protected void beforeSleepChecks() {}
+
+ @Override
+ public long getOldestTombstoneTimeDelta() {
+ return tombstones.peek().getVersionTimeStamp() - cacheTime.cacheTimeMillis();
Review comment:
The peek could return null...
Will it be a good enough to return version timestamp instead of delta...The caller can calculate delta with different time mechanism (using cacheTime or currentTime).
Also, will it be helpful if the tombstone.toString() info is returned...It has all the information associated with tombstone.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] mhansonp commented on a change in pull request #5623: GEODE-8607: Adding some test API to get oldest tombstone
Posted by GitBox <gi...@apache.org>.
mhansonp commented on a change in pull request #5623:
URL: https://github.com/apache/geode/pull/5623#discussion_r504969566
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
##########
@@ -450,6 +450,11 @@ boolean testHook_forceExpiredTombstoneGC(int count, long timeout, TimeUnit unit)
@Override
protected void beforeSleepChecks() {}
+
+ @Override
+ public long getOldestTombstoneTimeDelta() {
+ return tombstones.peek().getVersionTimeStamp() - cacheTime.cacheTimeMillis();
Review comment:
I figured that it was one extra step that will always be done, but I will change it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org