Repository: incubator-geode Updated Branches: refs/heads/develop 8eb6cef8a -> ff9b2f05f
GEODE-223: Handle region create/destroy remote event in Redis adpater Ignore events where region creation initiated remotely attempts to create a local region reference when the region has already been destroyed. Also, the destruction of a region may be caught the query engine, so I have accounted for that by handling com.gemstone.gemfire.cache.query.RegionNotFoundException. Finally, a the Jedis client timeout has been increased for RedisDistDunitTest to account for concurrent region creation/destruction and an expected exception has been added to not fail over the log scanning. Sometimes when a region is destroyed the PooledMessage Processor will log a regiondestroyed exception, which is ok, but makes the test fail. closes #16 Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/ff9b2f05 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/ff9b2f05 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/ff9b2f05 Branch: refs/heads/develop Commit: ff9b2f05ffa6312e3fcd3abc2b7bc96077870829 Parents: 8eb6cef Author: Vito Gavrilov <vgavri...@pivotal.io> Authored: Mon Aug 17 18:30:12 2015 -0700 Committer: Swapnil Bawaskar <sbawas...@pivotal.io> Committed: Tue Aug 18 17:48:11 2015 -0700 ---------------------------------------------------------------------- .../internal/redis/ExecutionHandlerContext.java | 3 ++- .../gemfire/internal/redis/RegionProvider.java | 11 ++++++++--- .../gemstone/gemfire/redis/GemFireRedisServer.java | 11 ++++++++--- .../gemstone/gemfire/redis/RedisDistDUnitTest.java | 15 +++++++++------ 4 files changed, 27 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/ff9b2f05/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/ExecutionHandlerContext.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/ExecutionHandlerContext.java b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/ExecutionHandlerContext.java index cf20ea8..d2d2f51 100644 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/ExecutionHandlerContext.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/ExecutionHandlerContext.java @@ -22,6 +22,7 @@ import com.gemstone.gemfire.cache.TransactionException; import com.gemstone.gemfire.cache.TransactionId; import com.gemstone.gemfire.cache.UnsupportedOperationInTransactionException; import com.gemstone.gemfire.cache.query.QueryInvocationTargetException; +import com.gemstone.gemfire.cache.query.RegionNotFoundException; import com.gemstone.gemfire.internal.redis.executor.transactions.TransactionExecutor; import com.gemstone.gemfire.redis.GemFireRedisServer; @@ -213,7 +214,7 @@ public class ExecutionHandlerContext extends ChannelInboundHandlerAdapter { return; } catch (Exception e) { cause = e; - if (e instanceof RegionDestroyedException || e.getCause() instanceof QueryInvocationTargetException) + if (e instanceof RegionDestroyedException || e instanceof RegionNotFoundException || e.getCause() instanceof QueryInvocationTargetException) Thread.sleep(WAIT_REGION_DSTRYD_MILLIS); } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/ff9b2f05/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/RegionProvider.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/RegionProvider.java b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/RegionProvider.java index a95f853..cadaf5f 100644 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/RegionProvider.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/internal/redis/RegionProvider.java @@ -194,7 +194,7 @@ public class RegionProvider implements Closeable { return getOrCreateRegion0(key, type, context, true); } - public void createRemoteRegionLocally(ByteArrayWrapper key, RedisDataType type) { + public void createRemoteRegionReferenceLocally(ByteArrayWrapper key, RedisDataType type) { if (type == null || type == RedisDataType.REDIS_STRING || type == RedisDataType.REDIS_HLL) return; Region<?, ?> r = this.regions.get(key); @@ -210,11 +210,16 @@ public class RegionProvider implements Closeable { boolean locked = false; try { locked = lock.tryLock(); - // If we cannot get the lock then this remote even may have been initialized + // If we cannot get the lock then this remote event may have been initialized // independently on this machine, so if we wait on the lock it is more than - // likely we will deadlock just to do the same task, this even can be ignored + // likely we will deadlock just to do the same task. This event can be ignored if (locked) { r = cache.getRegion(key.toString()); + // If r is null, this implies that we are after a create/destroy + // simply ignore. Calls to getRegion or getOrCreate will work correctly + if (r == null) + return; + if (type == RedisDataType.REDIS_LIST) doInitializeList(key, r); else if (type == RedisDataType.REDIS_SORTEDSET) http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/ff9b2f05/gemfire-core/src/main/java/com/gemstone/gemfire/redis/GemFireRedisServer.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/redis/GemFireRedisServer.java b/gemfire-core/src/main/java/com/gemstone/gemfire/redis/GemFireRedisServer.java index 821ea08..05f6024 100644 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/redis/GemFireRedisServer.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/redis/GemFireRedisServer.java @@ -35,6 +35,7 @@ import com.gemstone.gemfire.cache.Cache; import com.gemstone.gemfire.cache.CacheFactory; import com.gemstone.gemfire.cache.EntryEvent; import com.gemstone.gemfire.cache.Region; +import com.gemstone.gemfire.cache.RegionDestroyedException; import com.gemstone.gemfire.cache.RegionFactory; import com.gemstone.gemfire.cache.RegionShortcut; import com.gemstone.gemfire.cache.util.CacheListenerAdapter; @@ -429,7 +430,7 @@ public class GemFireRedisServer { Region<?, ?> newRegion = cache.getRegion(regionName); if (newRegion == null && type != RedisDataType.REDIS_STRING && type != RedisDataType.REDIS_HLL && type != RedisDataType.REDIS_PROTECTED) { try { - this.regionCache.createRemoteRegionLocally(Coder.stringToByteArrayWrapper(regionName), type); + this.regionCache.createRemoteRegionReferenceLocally(Coder.stringToByteArrayWrapper(regionName), type); } catch (Exception e) { if (logger.errorEnabled()) logger.error(e); @@ -528,8 +529,12 @@ public class GemFireRedisServer { if (event.isOriginRemote()) { final String key = (String) event.getKey(); final RedisDataType value = event.getNewValue(); - if (value != RedisDataType.REDIS_STRING && value != RedisDataType.REDIS_HLL && value != RedisDataType.REDIS_PROTECTED) - this.regionCache.createRemoteRegionLocally(Coder.stringToByteArrayWrapper(key), value); + if (value != RedisDataType.REDIS_STRING && value != RedisDataType.REDIS_HLL && value != RedisDataType.REDIS_PROTECTED) { + try { + this.regionCache.createRemoteRegionReferenceLocally(Coder.stringToByteArrayWrapper(key), value); + } catch (RegionDestroyedException ignore) { // Region already destroyed, ignore + } + } } } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/ff9b2f05/gemfire-core/src/test/java/com/gemstone/gemfire/redis/RedisDistDUnitTest.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/test/java/com/gemstone/gemfire/redis/RedisDistDUnitTest.java b/gemfire-core/src/test/java/com/gemstone/gemfire/redis/RedisDistDUnitTest.java index 858321d..4f4ab03 100644 --- a/gemfire-core/src/test/java/com/gemstone/gemfire/redis/RedisDistDUnitTest.java +++ b/gemfire-core/src/test/java/com/gemstone/gemfire/redis/RedisDistDUnitTest.java @@ -26,6 +26,8 @@ public class RedisDistDUnitTest extends CacheTestCase { private int server1Port; private int server2Port; + + private static final int JEDIS_TIMEOUT = 20 * 1000; private abstract class ClientTestBase extends SerializableCallable { @@ -81,8 +83,8 @@ public class RedisDistDUnitTest extends CacheTestCase { } public void testConcListOps() throws Throwable { - final Jedis jedis1 = new Jedis("localhost", server1Port, 10000); - final Jedis jedis2 = new Jedis("localhost", server2Port, 10000); + final Jedis jedis1 = new Jedis("localhost", server1Port, JEDIS_TIMEOUT); + final Jedis jedis2 = new Jedis("localhost", server2Port, JEDIS_TIMEOUT); final int pushes = 20; class ConcListOps extends ClientTestBase { protected ConcListOps(int port) { @@ -91,7 +93,7 @@ public class RedisDistDUnitTest extends CacheTestCase { @Override public Object call() throws Exception { - Jedis jedis = new Jedis("localhost", port, 10000); + Jedis jedis = new Jedis("localhost", port, JEDIS_TIMEOUT); Random r = new Random(); for (int i = 0; i < pushes; i++) { if (r.nextBoolean()) { @@ -114,8 +116,9 @@ public class RedisDistDUnitTest extends CacheTestCase { assertEquals(result1, result2); } - public void testConcCreateDestroy() throws Throwable { + addExpectedException("RegionDestroyedException"); + addExpectedException("IndexInvalidException"); final int ops = 40; final String hKey = TEST_KEY+"hash"; final String lKey = TEST_KEY+"list"; @@ -129,7 +132,7 @@ public class RedisDistDUnitTest extends CacheTestCase { @Override public Object call() throws Exception { - Jedis jedis = new Jedis("localhost", port, 10000); + Jedis jedis = new Jedis("localhost", port, JEDIS_TIMEOUT); Random r = new Random(); for (int i = 0; i < ops; i++) { int n = r.nextInt(4); @@ -189,7 +192,7 @@ public class RedisDistDUnitTest extends CacheTestCase { @Override public Object call() throws Exception { - Jedis jedis = new Jedis("localhost", port, 10000); + Jedis jedis = new Jedis("localhost", port, JEDIS_TIMEOUT); Random r = new Random(); for (int i = 0; i < ops; i++) { int n = r.nextInt(4);