[ 
https://issues.apache.org/jira/browse/GEODE-8607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17218398#comment-17218398
 ] 

ASF GitHub Bot commented on GEODE-8607:
---------------------------------------

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:
us...@infra.apache.org


> Add new API for getting oldest tombstone age.
> ---------------------------------------------
>
>                 Key: GEODE-8607
>                 URL: https://issues.apache.org/jira/browse/GEODE-8607
>             Project: Geode
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.14.0
>            Reporter: Mark Hanson
>            Assignee: Mark Hanson
>            Priority: Major
>              Labels: GeodeOperationAPI, pull-request-available
>
> It would be handy in diagnosing certain issues.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to