geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lgalli...@apache.org
Subject geode git commit: GEODE-2661: afterDestroy events fired on non-existent keys during destroy or removeAll.
Date Thu, 01 Jun 2017 20:40:19 GMT
Repository: geode
Updated Branches:
  refs/heads/develop b30545108 -> 08451526e


GEODE-2661: afterDestroy events fired on non-existent keys during destroy or removeAll.

In a client/server topology, afterDestroy events were fired on keys that did not exist
for removeAll, remove and destroy. This suppresses those events.


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/08451526
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/08451526
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/08451526

Branch: refs/heads/develop
Commit: 08451526ea5a87e15c4ce609bdf5a0f14fed7d06
Parents: b305451
Author: Lynn Gallinat <lgallinat@pivotal.io>
Authored: Fri May 5 17:00:02 2017 -0700
Committer: Lynn Gallinat <lgallinat@pivotal.io>
Committed: Thu Jun 1 13:34:53 2017 -0700

----------------------------------------------------------------------
 .../cache/DistributedRemoveAllOperation.java    |  15 +-
 .../geode/internal/cache/LocalRegion.java       |   2 +-
 ...emoveAllCacheListenerPeerRegressionTest.java | 159 ++++++++
 ...CacheListenerClientServerRegressionTest.java | 390 +++++++++++++++++++
 .../cache/ha/EventIdOptimizationDUnitTest.java  |  28 +-
 5 files changed, 574 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/08451526/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java
index e236f80..a4661b6 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DistributedRemoveAllOperation.java
@@ -22,8 +22,6 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Set;
 
-import org.apache.logging.log4j.Logger;
-
 import org.apache.geode.DataSerializer;
 import org.apache.geode.cache.CacheEvent;
 import org.apache.geode.cache.DataPolicy;
@@ -53,6 +51,7 @@ import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.offheap.annotations.Released;
 import org.apache.geode.internal.offheap.annotations.Retained;
 import org.apache.geode.internal.offheap.annotations.Unretained;
+import org.apache.logging.log4j.Logger;
 
 /**
  * Handles distribution of a Region.removeAll operation.
@@ -214,6 +213,7 @@ public class DistributedRemoveAllOperation extends AbstractUpdateOperation
{
     boolean returnedEv = false;
     try {
       ev.setPossibleDuplicate(entry.isPossibleDuplicate());
+      ev.setIsRedestroyedEntry(entry.getRedestroyedEntry());
       if (entry.versionTag != null && region.concurrencyChecksEnabled) {
         VersionSource id = entry.versionTag.getMemberID();
         if (id != null) {
@@ -279,6 +279,8 @@ public class DistributedRemoveAllOperation extends AbstractUpdateOperation
{
 
     transient boolean inhibitDistribution;
 
+    transient boolean redestroyedEntry;
+
     /**
      * Constructor to use when preparing to send putall data out
      */
@@ -302,6 +304,7 @@ public class DistributedRemoveAllOperation extends AbstractUpdateOperation
{
       setCallbacksInvoked(event.callbacksInvoked());
       setPossibleDuplicate(event.isPossibleDuplicate());
       setInhibitDistribution(event.getInhibitDistribution());
+      setRedestroyedEntry(event.getIsRedestroyedEntry());
     }
 
     /**
@@ -528,6 +531,14 @@ public class DistributedRemoveAllOperation extends AbstractUpdateOperation
{
       this.inhibitDistribution = inhibitDistribution;
     }
 
+    public boolean getRedestroyedEntry() {
+      return redestroyedEntry;
+    }
+
+    public void setRedestroyedEntry(boolean redestroyedEntry) {
+      this.redestroyedEntry = redestroyedEntry;
+    }
+
     public boolean isCallbacksInvoked() {
       return this.callbacksInvoked;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/08451526/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
----------------------------------------------------------------------
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 a8dd611..2b45a54 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
@@ -6831,7 +6831,7 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
     if (notifyGateways) {
       notifyGatewaySender(eventType, event);
     }
-    if (callDispatchListenerEvent) {
+    if (callDispatchListenerEvent && !event.getIsRedestroyedEntry()) {
       dispatchListenerEvent(eventType, event);
     }
   }

http://git-wip-us.apache.org/repos/asf/geode/blob/08451526/geode-core/src/test/java/org/apache/geode/cache/RemoveAllCacheListenerPeerRegressionTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/RemoveAllCacheListenerPeerRegressionTest.java
b/geode-core/src/test/java/org/apache/geode/cache/RemoveAllCacheListenerPeerRegressionTest.java
new file mode 100644
index 0000000..90b11f9
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/cache/RemoveAllCacheListenerPeerRegressionTest.java
@@ -0,0 +1,159 @@
+/*
+ * 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 o * or implied. See the License for the specific language governing permissions
+ * and limitations under n an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
either
+ * express the License.
+ */
+package org.apache.geode.cache;
+
+import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import org.apache.geode.cache.util.CacheListenerAdapter;
+import org.apache.geode.distributed.DistributedSystem;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(IntegrationTest.class)
+public class RemoveAllCacheListenerPeerRegressionTest {
+  private Cache cache;
+  private DistributedSystem ds;
+  String errStr = null;
+  private final String NON_EXISTENT_KEY = "nonExistentKey";
+
+
+  @Before
+  public void setUp() throws Exception {
+    Properties p = new Properties();
+    p.setProperty(MCAST_PORT, "0");
+    p.setProperty(LOCATORS, "");
+    ds = DistributedSystem.connect(p);
+    cache = CacheFactory.create(ds);
+    errStr = null;
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    if (cache != null) {
+      cache.close();
+      cache = null;
+    }
+    if (this.ds != null) {
+      this.ds.disconnect();
+      this.ds = null;
+    }
+  }
+
+  @Test
+  public void removeAllListenerPeerReplicateTest() throws Exception {
+    doRemoveAllTest(RegionShortcut.REPLICATE);
+  }
+
+  @Test
+  public void removeAllListenerPeerPartitionTest() throws Exception {
+    doRemoveAllTest(RegionShortcut.PARTITION);
+  }
+
+  @Test
+  public void destroyListenerPeerReplicateTest() throws Exception {
+    doDestroyTest(RegionShortcut.REPLICATE);
+  }
+
+  @Test
+  public void destroyListenerPeerPartitionTest() throws Exception {
+    doDestroyTest(RegionShortcut.PARTITION);
+  }
+
+  @Test
+  public void removeListenerPeerReplicateTest() throws Exception {
+    doRemoveTest(RegionShortcut.REPLICATE);
+  }
+
+  @Test
+  public void removeListenerPeerPartitionTest() throws Exception {
+    doRemoveTest(RegionShortcut.PARTITION);
+  }
+
+  private class TestListener extends CacheListenerAdapter {
+
+    @Override
+    public void afterDestroy(EntryEvent event) {
+      if (event.getKey().equals(NON_EXISTENT_KEY)) {
+        errStr = "event fired for non-existent key " + event.getKey() + "; " + event + "\n"
+            + getStackTrace();
+      }
+    }
+
+  }
+
+  private void doRemoveAllTest(RegionShortcut shortcut) {
+    RegionFactory<Object, Object> factory = cache.createRegionFactory(shortcut);
+    factory.addCacheListener(new TestListener());
+    Region aRegion = factory.create("TestRegion");
+    aRegion.put("key1", "value1");
+    aRegion.put("key2", "value2");
+    aRegion.put("key3", "value3");
+
+    List<String> removeAllColl = new ArrayList<String>();
+    removeAllColl.add("key1");
+    removeAllColl.add("key2");
+    removeAllColl.add(NON_EXISTENT_KEY);
+    aRegion.removeAll(removeAllColl);
+    assertNull(errStr); // errStr is set if we invoke afterDestroy in the listener
+  }
+
+  private void doDestroyTest(RegionShortcut shortcut) {
+    RegionFactory<Object, Object> factory = cache.createRegionFactory(shortcut);
+    factory.addCacheListener(new TestListener());
+    Region aRegion = factory.create("TestRegion");
+    try {
+      aRegion.destroy(NON_EXISTENT_KEY);
+      fail(EntryNotFoundException.class.getName()
+          + " was not thrown when destroying a non-existent key");
+    } catch (EntryNotFoundException e) {
+      // expected
+    } finally {
+      assertNull(errStr); // errStr is set if we invoke afterDestroy in the listener
+    }
+  }
+
+  private void doRemoveTest(RegionShortcut shortcut) {
+    RegionFactory<Object, Object> factory = cache.createRegionFactory(shortcut);
+    factory.addCacheListener(new TestListener());
+    Region aRegion = factory.create("TestRegion");
+    Object returnedValue = aRegion.remove(NON_EXISTENT_KEY);
+    assertNull(returnedValue);
+    assertNull(errStr); // errStr is set if we invoke afterDestroy in the listener
+  }
+
+  /** Get a stack trace of the current stack and return it as a String */
+  public static String getStackTrace() {
+    try {
+      throw new Exception("Exception to get stack trace");
+    } catch (Exception e) {
+      StringWriter sw = new StringWriter();
+      e.printStackTrace(new PrintWriter(sw, true));
+      return sw.toString();
+    }
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/08451526/geode-core/src/test/java/org/apache/geode/cache/client/RemoveAllCacheListenerClientServerRegressionTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/client/RemoveAllCacheListenerClientServerRegressionTest.java
b/geode-core/src/test/java/org/apache/geode/cache/client/RemoveAllCacheListenerClientServerRegressionTest.java
new file mode 100644
index 0000000..0e2404f
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/cache/client/RemoveAllCacheListenerClientServerRegressionTest.java
@@ -0,0 +1,390 @@
+/*
+ * 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.cache.client;
+
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_FILE;
+import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.net.UnknownHostException;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.EntryEvent;
+import org.apache.geode.cache.EntryNotFoundException;
+import org.apache.geode.cache.InterestResultPolicy;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.server.CacheServer;
+import org.apache.geode.cache.util.CacheListenerAdapter;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.dunit.DistributedTestCase;
+import org.apache.geode.test.dunit.DistributedTestUtils;
+import org.apache.geode.test.dunit.Host;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.SerializableRunnable;
+import org.apache.geode.test.dunit.VM;
+import org.apache.geode.test.junit.categories.ClientServerTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({DistributedTest.class, ClientServerTest.class})
+public class RemoveAllCacheListenerClientServerRegressionTest extends DistributedTestCase
{
+
+  private final AtomicInteger serverPort = new AtomicInteger(0);
+  private final String REGION_NAME = "TestRegion";
+  private VM clientVM = null;
+  private VM serverVM = null;
+  private final String BB_MAILBOX_KEY = "error";
+  private final String NON_EXISTENT_KEY = "nonExistentKey";
+
+  @Override
+  public final void postSetUp() throws Exception {
+    disconnectAllFromDS();
+    IgnoredException.addIgnoredException("java.net.ConnectException");
+    reset();
+  }
+
+  @Override
+  public final void preTearDown() throws Exception {
+    disconnectAllFromDS();
+    reset();
+  }
+
+  private void reset() {
+    getBlackboard().initBlackboard();
+    getBlackboard().setMailbox(BB_MAILBOX_KEY, "");
+    clientVM = null;
+    serverVM = null;
+    serverPort.set(0);
+  }
+
+  // ================================================================================
+  // replicate on server
+  // caching proxy client
+  // server concurrency checks enabled
+  // client concurrency checks enabled
+  @Test
+  public void removeAllListenerReplicateCachingProxyServerChecksClientChecksTest() {
+    doRemoveAllTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, true, true);
+  }
+
+  @Test
+  public void destroyListenerReplicateCachingProxyServerChecksClientChecksTest() {
+    doDestroyTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, true, true);
+  }
+
+  @Test
+  public void removeListenerReplicateCachingProxyServerChecksClientChecksTest() {
+    doRemoveTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, true, true);
+  }
+
+  // ================================================================================
+  // partitioned on server
+  // caching proxy client
+  // server concurrency checks enabled
+  // client concurrency checks enabled
+  @Test
+  public void removeAllListenerPartitionCachingProxyServerChecksClientChecksTest() {
+    doRemoveAllTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, true, true);
+  }
+
+  @Test
+  public void destroyListenerPartitionCachingProxyServerChecksClientChecksTest() {
+    doDestroyTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, true, true);
+  }
+
+  @Test
+  public void removeListenerPartitionCachingProxyServerChecksClientChecksTest() {
+    doRemoveTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, true, true);
+  }
+
+  // ================================================================================
+  // replicate on server
+  // caching proxy client
+  // server concurrency checks disabled
+  // client concurrency checks enabled
+  @Test
+  public void removeAllListenerReplicateCachingProxyClientChecksOnlyTest() {
+    doRemoveAllTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, false,
true);
+  }
+
+  @Test
+  public void destroyListenerReplicateCachingProxyClientChecksOnlyTest() {
+    doDestroyTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, false, true);
+  }
+
+  @Test
+  public void removeListenerReplicateCachingProxyClientChecksOnlyTest() {
+    doRemoveTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, false, true);
+  }
+
+  // ================================================================================
+  // partitioned on server
+  // caching proxy client
+  // server concurrency checks disabled
+  // client concurrency checks enabled
+  @Test
+  public void removeAllListenerPartitionCachingProxyClientChecksOnlyTest() {
+    doRemoveAllTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, false,
true);
+  }
+
+  @Test
+  public void destroyListenerPartitionCachingProxyClientChecksOnlyTest() {
+    doDestroyTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, false, true);
+  }
+
+  @Test
+  public void removeListenerPartitionCachingProxyClientChecksOnlyTest() {
+    doRemoveTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, false, true);
+  }
+
+  // ================================================================================
+  // replicate on server
+  // caching proxy client
+  // server concurrency checks enabled
+  // client concurrency checks disabled
+  @Test
+  public void removeAllListenerReplicateCachingProxyServerChecksOnlyTest() {
+    doRemoveAllTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, true, false);
+  }
+
+  @Test
+  public void destroyListenerReplicateCachingProxyServerChecksOnlyTest() {
+    doDestroyTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, true, false);
+  }
+
+  @Test
+  public void removeListenerReplicateCachingProxyServerChecksOnlyTest() {
+    doRemoveTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, true, false);
+  }
+
+  // ================================================================================
+  // partitioned on server
+  // caching proxy client
+  // server concurrency checks enabled
+  // client concurrency checks disabled
+  @Test
+  public void removeAllListenerPartitionCachingProxyServerChecksOnlyTest() {
+    doRemoveAllTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, true, false);
+  }
+
+  @Test
+  public void destroyListenerPartitionCachingProxyServerChecksOnlyTest() {
+    doDestroyTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, true, false);
+  }
+
+  @Test
+  public void removeListenerPartitionCachingProxyServerChecksOnlyTest() {
+    doRemoveTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, true, false);
+  }
+
+  // ================================================================================
+  // replicate on server
+  // caching proxy client
+  // server concurrency checks disabled
+  // client concurrency checks disabled
+  @Test
+  public void removeAllListenerReplicateCachingProxyChecksDisabledTest() {
+    doRemoveAllTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, false,
false);
+  }
+
+  @Test
+  public void destroyListenerReplicateCachingProxyChecksDisabledTest() {
+    doDestroyTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, false, false);
+  }
+
+  @Test
+  public void removeListenerReplicateCachingProxyChecksDisabledTest() {
+    doRemoveTest(RegionShortcut.REPLICATE, ClientRegionShortcut.CACHING_PROXY, false, false);
+  }
+
+  // ================================================================================
+  // partitioned on server
+  // caching proxy client
+  // server concurrency checks disabled
+  // client concurrency checks disabled
+  @Test
+  public void removeAllListenerPartitionCachingProxyChecksDisabledTest() {
+    doRemoveAllTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, false,
false);
+  }
+
+  @Test
+  public void destroyListenerPartitionCachingProxyChecksDisabledTest() {
+    doDestroyTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, false, false);
+  }
+
+  @Test
+  public void removeListenerPartitionCachingProxyChecksDisabledTest() {
+    doRemoveTest(RegionShortcut.PARTITION, ClientRegionShortcut.CACHING_PROXY, false, false);
+  }
+
+  // ================================================================================
+  private void setupGemFireCacheServer(RegionShortcut shortcut, boolean concChecks) {
+    serverVM = Host.getHost(0).getVM(0);
+    serverPort.set(AvailablePortHelper.getRandomAvailableTCPPort());
+    String serverName = this.getClass().getSimpleName() + "_server";
+    serverVM.invoke(new SerializableRunnable() {
+      @Override
+      public void run() {
+        try {
+          Cache cache = new CacheFactory().set("name", serverName).set(MCAST_PORT, "0")
+              .set(LOG_FILE, serverName + ".log").set(LOG_LEVEL, "config")
+              .set("locators", "localhost[" + DistributedTestUtils.getDUnitLocatorPort()
+ "]")
+              .create();
+
+          RegionFactory<String, String> regionFactory = cache.createRegionFactory(shortcut);
+          regionFactory.addCacheListener(new TestListener());
+          regionFactory.setConcurrencyChecksEnabled(concChecks);
+          regionFactory.create(REGION_NAME);
+
+          CacheServer cacheServer = cache.addCacheServer();
+          cacheServer.setPort(serverPort.get());
+          cacheServer.setMaxConnections(10);
+          cacheServer.start();
+          assertTrue("Cache Server is not running!", cacheServer.isRunning());
+        } catch (UnknownHostException ignore) {
+          throw new RuntimeException(ignore);
+        } catch (IOException e) {
+          throw new RuntimeException("Failed to start cache server " + serverName + " on
port "
+              + serverPort.get() + ": " + e.getStackTrace());
+        }
+      }
+    });
+  }
+
+  private void setupGemFireClientCache(ClientRegionShortcut shortcut, boolean concChecks)
{
+    clientVM = Host.getHost(0).getVM(1);
+    clientVM.invoke(() -> {
+      ClientCache clientCache = new ClientCacheFactory().create();
+
+      PoolFactory poolFactory = PoolManager.createFactory();
+      poolFactory.setMaxConnections(10);
+      poolFactory.setMinConnections(1);
+      poolFactory.setReadTimeout(5000);
+      poolFactory.setSubscriptionEnabled(true);
+      poolFactory.addServer("localhost", serverPort.get());
+      Pool pool = poolFactory.create("serverConnectionPool");
+      assertNotNull("The 'serverConnectionPool' was not properly configured and initialized!",
+          pool);
+
+      ClientRegionFactory<String, String> regionFactory =
+          clientCache.createClientRegionFactory(shortcut);
+      regionFactory.setPoolName(pool.getName());
+      regionFactory.addCacheListener(new TestListener());
+      regionFactory.setConcurrencyChecksEnabled(concChecks);
+      Region<String, String> aRegion = regionFactory.create(REGION_NAME);
+      assertNotNull("The region " + REGION_NAME + " was not properly configured and initialized",
+          aRegion);
+
+      aRegion.registerInterest("ALL_KEYS", InterestResultPolicy.KEYS_VALUES);
+    });
+  }
+
+  private class TestListener extends CacheListenerAdapter<String, String> {
+
+    @Override
+    public void afterDestroy(EntryEvent event) {
+      if (event.getKey().equals(NON_EXISTENT_KEY)) {
+        String errStr = "event fired for non-existent key " + event.getKey() + "; " + event
+ "\n"
+            + getStackTrace();
+        getBlackboard().setMailbox(BB_MAILBOX_KEY, errStr);
+      }
+    }
+
+  }
+
+  /**
+   * Add values to a region, then do a removeAll using both existent and non-existent keys.
If a
+   * listener event is invoked on a non-existent key, the listener writes it to the blackboard.
Read
+   * from the blackboard and fail the test if a listener event was invoked for a non-existent
key.
+   */
+  private void doRemoveAllTest(RegionShortcut serverShortcut, ClientRegionShortcut clientShortcut,
+      boolean serverConcChecks, boolean clientConcChecks) {
+    setupGemFireCacheServer(serverShortcut, serverConcChecks);
+    setupGemFireClientCache(clientShortcut, clientConcChecks);
+    clientVM.invoke(() -> {
+      Region<String, String> aRegion = ClientCacheFactory.getAnyInstance().getRegion(REGION_NAME);
+      aRegion.put("key1", "value1");
+      aRegion.put("key2", "value2");
+      aRegion.put("key3", "value3");
+
+      Set<String> removeAllSet = new HashSet<String>();
+      removeAllSet.add("key1");
+      removeAllSet.add("key2");
+      removeAllSet.add(NON_EXISTENT_KEY);
+      aRegion.removeAll(removeAllSet);
+      Object error = getBlackboard().getMailbox(BB_MAILBOX_KEY);
+      assertNotNull(error);
+      assertEquals("", error);
+    });
+  }
+
+  private void doDestroyTest(RegionShortcut serverShortcut, ClientRegionShortcut clientShortcut,
+      boolean serverConcChecks, boolean clientConcChecks) {
+    setupGemFireCacheServer(serverShortcut, serverConcChecks);
+    setupGemFireClientCache(clientShortcut, clientConcChecks);
+    clientVM.invoke(() -> {
+      Region<String, String> aRegion = ClientCacheFactory.getAnyInstance().getRegion(REGION_NAME);
+      try {
+        aRegion.destroy(NON_EXISTENT_KEY);
+      } catch (EntryNotFoundException e) {
+        // expected
+      }
+      Object error = getBlackboard().getMailbox(BB_MAILBOX_KEY);
+      assertNotNull(error);
+      assertEquals("", error);
+    });
+  }
+
+  private void doRemoveTest(RegionShortcut serverShortcut, ClientRegionShortcut clientShortcut,
+      boolean serverConcChecks, boolean clientConcChecks) {
+    setupGemFireCacheServer(serverShortcut, serverConcChecks);
+    setupGemFireClientCache(clientShortcut, clientConcChecks);
+    clientVM.invoke(() -> {
+      getBlackboard().setMailbox(BB_MAILBOX_KEY, "");
+      Region<String, String> aRegion = ClientCacheFactory.getAnyInstance().getRegion(REGION_NAME);
+      Object returnedValue = aRegion.remove(NON_EXISTENT_KEY);
+      assertNull(returnedValue);
+      Object error = getBlackboard().getMailbox(BB_MAILBOX_KEY);
+      assertNotNull(error);
+      assertEquals("", error);
+    });
+  }
+
+  /** Get a stack trace of the current stack and return it as a String */
+  public static String getStackTrace() {
+    try {
+      throw new Exception("Exception to get stack trace");
+    } catch (Exception e) {
+      StringWriter sw = new StringWriter();
+      e.printStackTrace(new PrintWriter(sw, true));
+      return sw.toString();
+    }
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/geode/blob/08451526/geode-core/src/test/java/org/apache/geode/internal/cache/ha/EventIdOptimizationDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/ha/EventIdOptimizationDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/ha/EventIdOptimizationDUnitTest.java
index e4db6b1..09e39d8 100755
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/ha/EventIdOptimizationDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/ha/EventIdOptimizationDUnitTest.java
@@ -14,6 +14,8 @@
  */
 package org.apache.geode.internal.cache.ha;
 
+import static java.util.concurrent.TimeUnit.*;
+import static org.awaitility.Awaitility.*;
 import static org.junit.Assert.*;
 
 import java.util.Iterator;
@@ -305,9 +307,12 @@ public class EventIdOptimizationDUnitTest extends JUnit4DistributedTestCase
{
     ServerRegionProxy srp = new ServerRegionProxy(regionName, pool);
 
     for (int i = 0; i < eventIds.length; i++) {
-      srp.destroyOnForTestsOnly(connection, "KEY-" + i, null, Operation.DESTROY,
+      String key = "KEY-" + i;
+      srp.putOnForTestsOnly(connection, key, "value-" + i, eventIds[i], null);
+      srp.destroyOnForTestsOnly(connection, key, null, Operation.DESTROY,
           new EventIDHolder(eventIds[i]), null);
     }
+    srp.putOnForTestsOnly(connection, LAST_KEY, "lastValue", eventIdForLastKey, null);
     srp.destroyOnForTestsOnly(connection, LAST_KEY, null, Operation.DESTROY,
         new EventIDHolder(eventIdForLastKey), null);
   }
@@ -404,17 +409,9 @@ public class EventIdOptimizationDUnitTest extends JUnit4DistributedTestCase
{
    * Waits for the listener to receive all events and validates that no exception occurred
in client
    */
   public static void verifyEventIdsOnClient2() {
-    if (!proceedForValidation) {
-      synchronized (EventIdOptimizationDUnitTest.class) {
-        if (!proceedForValidation)
-          try {
-            LogWriterUtils.getLogWriter().info("Client2 going in wait before starting validation");
-            EventIdOptimizationDUnitTest.class.wait();
-          } catch (InterruptedException e) {
-            fail("interrupted");
-          }
-      }
-    }
+    await("Waiting for proceedForValidation to be true").atMost(2, MINUTES)
+        .until(() -> proceedForValidation);
+
     LogWriterUtils.getLogWriter().info("Starting validation on client2");
     if (validationFailed) {
       fail(
@@ -515,11 +512,8 @@ public class EventIdOptimizationDUnitTest extends JUnit4DistributedTestCase
{
     EventID eventIdAtClient2 = (EventID) assertThreadIdToSequenceIdMapHasEntryId();
     if ((eventIdAtClient2.getThreadID() == eventIdForLastKey.getThreadID())
         && (eventIdAtClient2.getSequenceID() == eventIdForLastKey.getSequenceID()))
{
-      synchronized (EventIdOptimizationDUnitTest.class) {
-        LogWriterUtils.getLogWriter().info("Notifying client2 to proceed for validation");
-        proceedForValidation = true;
-        EventIdOptimizationDUnitTest.class.notify();
-      }
+      LogWriterUtils.getLogWriter().info("Notifying client2 to proceed for validation");
+      proceedForValidation = true;
     } else {
       boolean containsEventId = false;
       for (int i = 0; i < eventIds.length; i++) {


Mime
View raw message