[geode] branch develop updated: GEODE-7744: Fix Redis startup failure (#4640)

[je...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jensdeppe 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 2828059  GEODE-7744: Fix Redis startup failure (#4640)
2828059 is described below

commit 28280591ff8094f33e6d72ce77927189863c85fd
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Thu Jan 30 09:07:24 2020 -0800

    GEODE-7744: Fix Redis startup failure (#4640)
    
    - Introduce new ResourceEvent - CLUSTER_CONFIGURATION_APPLIED
    - Also fixes GEODE-7721
    
    Authored-by: Jens Deppe <jd...@pivotal.io>
---
 .../geode/distributed/internal/ResourceEvent.java  |   3 +-
 .../geode/internal/cache/GemFireCacheImpl.java     |   2 +
 .../org/apache/geode/redis/RedisDistDUnitTest.java | 106 ++++++++-------------
 .../redis/RedisUsePersistentRegionDUnitTest.java   |  56 +++++++++++
 .../geode/redis/internal/GeodeRedisService.java    |  18 +++-
 5 files changed, 115 insertions(+), 70 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/ResourceEvent.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/ResourceEvent.java
index 07bb949..51012ca 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/ResourceEvent.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/ResourceEvent.java
@@ -47,6 +47,7 @@ public enum ResourceEvent {
   GATEWAYSENDER_PAUSE,
   GATEWAYSENDER_RESUME,
   CACHE_SERVICE_CREATE,
-  CACHE_SERVICE_REMOVE
+  CACHE_SERVICE_REMOVE,
+  CLUSTER_CONFIGURATION_APPLIED,
 
 }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index fd0fb73..573d0fe 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -1389,6 +1389,8 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
       }
     }
 
+    system.handleResourceEvent(ResourceEvent.CLUSTER_CONFIGURATION_APPLIED, this);
+
     startColocatedJmxManagerLocator();
 
     startRestAgentServer(this);
diff --git a/geode-redis/src/distributedTest/java/org/apache/geode/redis/RedisDistDUnitTest.java b/geode-redis/src/distributedTest/java/org/apache/geode/redis/RedisDistDUnitTest.java
index 65b04d7..6511be2 100644
--- a/geode-redis/src/distributedTest/java/org/apache/geode/redis/RedisDistDUnitTest.java
+++ b/geode-redis/src/distributedTest/java/org/apache/geode/redis/RedisDistDUnitTest.java
@@ -14,54 +14,50 @@
  */
 package org.apache.geode.redis;
 
-import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
-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 java.io.Serializable;
+import java.util.Properties;
 import java.util.Random;
 
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import redis.clients.jedis.Jedis;
 
-import org.apache.geode.cache.CacheFactory;
-import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.internal.AvailablePortHelper;
-import org.apache.geode.internal.inet.LocalHostUtil;
 import org.apache.geode.test.awaitility.GeodeAwaitility;
 import org.apache.geode.test.dunit.AsyncInvocation;
-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.LogWriterUtils;
 import org.apache.geode.test.dunit.SerializableCallable;
 import org.apache.geode.test.dunit.VM;
-import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
 import org.apache.geode.test.junit.categories.RedisTest;
 
 @Category({RedisTest.class})
-public class RedisDistDUnitTest extends JUnit4DistributedTestCase {
+public class RedisDistDUnitTest implements Serializable {
 
-  public static final String TEST_KEY = "key";
-  public static int pushes = 200;
-  int redisPort = 6379;
-  private Host host;
-  private VM server1;
-  private VM server2;
-  private VM client1;
-  private VM client2;
+  @ClassRule
+  public static ClusterStartupRule cluster = new ClusterStartupRule(5);
+
+  private static String LOCALHOST = "localhost";
 
-  private int server1Port;
-  private int server2Port;
+  public static final String TEST_KEY = "key";
+  private static MemberVM locator;
+  private static MemberVM server1;
+  private static MemberVM server2;
+  private static VM client1;
+  private static VM client2;
 
-  private String localHost;
+  private static int server1Port;
+  private static int server2Port;
 
   private static final int JEDIS_TIMEOUT =
       Math.toIntExact(GeodeAwaitility.getTimeout().getValueInMS());
 
-  private abstract class ClientTestBase extends SerializableCallable {
-
+  private abstract static class ClientTestBase extends SerializableCallable<Object> {
     int port;
 
     protected ClientTestBase(int port) {
@@ -69,50 +65,32 @@ public class RedisDistDUnitTest extends JUnit4DistributedTestCase {
     }
   }
 
-  @Override
-  public final void postSetUp() throws Exception {
-    JUnit4DistributedTestCase.disconnectAllFromDS();
+  @BeforeClass
+  public static void setup() {
+    final int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    server1Port = ports[0];
+    server2Port = ports[1];
 
-    localHost = LocalHostUtil.getLocalHost().getHostName();
+    locator = cluster.startLocatorVM(0);
 
-    host = Host.getHost(0);
-    server1 = host.getVM(0);
-    server2 = host.getVM(1);
-    client1 = host.getVM(2);
-    client2 = host.getVM(3);
-    final int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
-    final int locatorPort = DistributedTestUtils.getDUnitLocatorPort();
-    final SerializableCallable<Object> startRedisAdapter = new SerializableCallable<Object>() {
+    Properties redisProps = new Properties();
+    redisProps.setProperty("redis-bind-address", LOCALHOST);
+    redisProps.setProperty("redis-port", Integer.toString(ports[0]));
+    server1 = cluster.startServerVM(1, redisProps, locator.getPort());
 
-      @Override
-      public Object call() throws Exception {
-        int port = ports[VM.getCurrentVMNum()];
-        CacheFactory cF = new CacheFactory();
-        String locator = LocalHostUtil.getLocalHost().getHostName() + "[" + locatorPort + "]";
-        cF.set(LOG_LEVEL, LogWriterUtils.getDUnitLogLevel());
-        cF.set(ConfigurationProperties.REDIS_BIND_ADDRESS, localHost);
-        cF.set(ConfigurationProperties.REDIS_PORT, "" + port);
-        cF.set(MCAST_PORT, "0");
-        cF.set(LOCATORS, locator);
-        cF.create();
-        return Integer.valueOf(port);
-      }
-    };
-    AsyncInvocation i = server1.invokeAsync(startRedisAdapter);
-    server2Port = (Integer) server2.invoke(startRedisAdapter);
-    server1Port = (Integer) i.getResult();
-  }
+    redisProps.setProperty("redis-port", Integer.toString(ports[1]));
+    server2 = cluster.startServerVM(2, redisProps, locator.getPort());
 
-  @Override
-  public final void preTearDown() throws Exception {
-    JUnit4DistributedTestCase.disconnectAllFromDS();
+    client1 = cluster.getVM(3);
+    client2 = cluster.getVM(4);
   }
 
   @Test
   public void testConcListOps() throws Exception {
-    final Jedis jedis1 = new Jedis(localHost, server1Port, JEDIS_TIMEOUT);
-    final Jedis jedis2 = new Jedis(localHost, server2Port, JEDIS_TIMEOUT);
+    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) {
         super(port);
@@ -120,7 +98,7 @@ public class RedisDistDUnitTest extends JUnit4DistributedTestCase {
 
       @Override
       public Object call() throws Exception {
-        Jedis jedis = new Jedis(localHost, port, JEDIS_TIMEOUT);
+        Jedis jedis = new Jedis(LOCALHOST, port, JEDIS_TIMEOUT);
         Random r = new Random();
         for (int i = 0; i < pushes; i++) {
           if (r.nextBoolean()) {
@@ -135,7 +113,7 @@ public class RedisDistDUnitTest extends JUnit4DistributedTestCase {
 
     AsyncInvocation i = client1.invokeAsync(new ConcListOps(server1Port));
     client2.invoke(new ConcListOps(server2Port));
-    i.getResult();
+    i.get();
     long expected = 2 * pushes;
     long result1 = jedis1.llen(TEST_KEY);
     long result2 = jedis2.llen(TEST_KEY);
@@ -160,7 +138,7 @@ public class RedisDistDUnitTest extends JUnit4DistributedTestCase {
 
       @Override
       public Object call() throws Exception {
-        Jedis jedis = new Jedis(localHost, port, JEDIS_TIMEOUT);
+        Jedis jedis = new Jedis(LOCALHOST, port, JEDIS_TIMEOUT);
         Random r = new Random();
         for (int i = 0; i < ops; i++) {
           int n = r.nextInt(4);
@@ -197,7 +175,7 @@ public class RedisDistDUnitTest extends JUnit4DistributedTestCase {
     // Expect to run with no exception
     AsyncInvocation i = client1.invokeAsync(new ConcCreateDestroy(server1Port));
     client2.invoke(new ConcCreateDestroy(server2Port));
-    i.getResult();
+    i.get();
   }
 
   /**
@@ -219,7 +197,7 @@ public class RedisDistDUnitTest extends JUnit4DistributedTestCase {
 
       @Override
       public Object call() throws Exception {
-        Jedis jedis = new Jedis(localHost, port, JEDIS_TIMEOUT);
+        Jedis jedis = new Jedis(LOCALHOST, port, JEDIS_TIMEOUT);
         Random r = new Random();
         for (int i = 0; i < ops; i++) {
           int n = r.nextInt(4);
diff --git a/geode-redis/src/distributedTest/java/org/apache/geode/redis/RedisUsePersistentRegionDUnitTest.java b/geode-redis/src/distributedTest/java/org/apache/geode/redis/RedisUsePersistentRegionDUnitTest.java
new file mode 100644
index 0000000..5767b06
--- /dev/null
+++ b/geode-redis/src/distributedTest/java/org/apache/geode/redis/RedisUsePersistentRegionDUnitTest.java
@@ -0,0 +1,56 @@
+/*
+ * 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.redis;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Rule;
+import org.junit.Test;
+import redis.clients.jedis.Jedis;
+
+import org.apache.geode.cache.Region;
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+public class RedisUsePersistentRegionDUnitTest {
+
+  @Rule
+  public final ServerStarterRule server = new ServerStarterRule();
+
+  @Test
+  public void startRedisWithPersistentRegion() throws Exception {
+    int port = AvailablePortHelper.getRandomAvailableTCPPort();
+
+    ServerStarterRule server = new ServerStarterRule()
+        .withJMXManager()
+        .withProperty("redis-port", Integer.toString(port))
+        .withProperty("redis-bind-address", "localhost")
+        .withSystemProperty(GeodeRedisServer.DEFAULT_REGION_SYS_PROP_NAME, "PARTITION_PERSISTENT")
+        .withAutoStart();;
+
+    // Using before() ensures that system properties are applied correctly
+    server.before();
+
+    Jedis client = new Jedis("localhost", port);
+
+    long result = client.hset("user", "name", "Joe");
+    assertThat(result).isEqualTo(1);
+
+    Region<?, ?> stringRegion = server.getCache().getRegion(GeodeRedisServer.STRING_REGION);
+    assertThat(stringRegion.getAttributes().getDataPolicy().withPersistence()).isTrue();
+  }
+}
diff --git a/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java b/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java
index ee56f90..12b1d7a 100644
--- a/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java
+++ b/geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisService.java
@@ -19,20 +19,23 @@ import org.apache.logging.log4j.Logger;
 import org.apache.geode.cache.Cache;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ResourceEvent;
+import org.apache.geode.distributed.internal.ResourceEventsListener;
 import org.apache.geode.internal.cache.CacheService;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.logging.internal.log4j.api.LogService;
 import org.apache.geode.management.internal.beans.CacheServiceMBeanBase;
 import org.apache.geode.redis.GeodeRedisServer;
 
-public class GeodeRedisService implements CacheService {
+public class GeodeRedisService implements CacheService, ResourceEventsListener {
   private static final Logger logger = LogService.getLogger();
   private GeodeRedisServer redisServer;
+  private InternalCache cache;
 
   @Override
   public boolean init(Cache cache) {
-    InternalCache internalCache = (InternalCache) cache;
-    startRedisServer(internalCache);
+    this.cache = (InternalCache) cache;
+    this.cache.getInternalDistributedSystem().addResourceListener(this);
 
     return true;
   }
@@ -42,6 +45,13 @@ public class GeodeRedisService implements CacheService {
     stopRedisServer();
   }
 
+  @Override
+  public void handleEvent(ResourceEvent event, Object resource) {
+    if (event.equals(ResourceEvent.CLUSTER_CONFIGURATION_APPLIED) && resource == cache) {
+      startRedisServer(cache);
+    }
+  }
+
   private void startRedisServer(InternalCache cache) {
     InternalDistributedSystem system = cache.getInternalDistributedSystem();
     int port = system.getConfig().getRedisPort();
@@ -67,8 +77,6 @@ public class GeodeRedisService implements CacheService {
       this.redisServer.shutdown();
   }
 
-
-
   @Override
   public Class<? extends CacheService> getInterface() {
     return GeodeRedisService.class;