Skip to content

Commit

Permalink
Revert "GEODE-9756: add geode-for-redis-replica-count gemfire property (
Browse files Browse the repository at this point in the history
apache#7058)" (apache#7069)

This reverts commit caa561f.
  • Loading branch information
dschneider-pivotal authored Nov 2, 2021
1 parent caa561f commit a18ad46
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1929,9 +1929,8 @@ public interface ConfigurationProperties {
* all local addresses.
* </p>
* <U>Default</U>: ""
* </p>
* <U>Since</U>: Geode 1.15
*/

String GEODE_FOR_REDIS_BIND_ADDRESS = "geode-for-redis-bind-address";
/**
* The static String definition of the <i>"geode-for-redis-enabled"</i> property <a
Expand All @@ -1945,7 +1944,6 @@ public interface ConfigurationProperties {
* When the default value of false, Geode for Redis is not available.
* Set to true to enable Geode for Redis.
* </p>
* <U>Since</U>: Geode 1.15
*/
String GEODE_FOR_REDIS_ENABLED = "geode-for-redis-enabled";
/**
Expand All @@ -1956,47 +1954,20 @@ public interface ConfigurationProperties {
* to authenticate using only a password. This requires a SecurityManager to be configured.
* </p>
* <U>Default</U>: default
* </p>
* <U>Since</U>: Geode 1.15
*/
String GEODE_FOR_REDIS_USERNAME = "geode-for-redis-username";
/**
* The static String definition of the <i>"geode-for-redis-port"</i> property <a
* name="geode-for-redis-port"/a>
* </p>
* <U>Description</U>: Specifies the port on which the Geode for Redis server listens for clients.
* A value of 0 selects a random port.</td>
* <U>Description</U>: Specifies the port on which the server listens for connections from Geode
* Geode for Redis. A value of 0 selects a random port.</td>
* </p>
* <U>Default</U>: 6379
* </p>
* <U>Allowed values</U>: 0..65535
* </p>
* <U>Since</U>: Geode 1.15
*/
String GEODE_FOR_REDIS_PORT = "geode-for-redis-port";
/**
* The static String definition of the <i>"geode-for-redis-replica-count"</i> property <a
* name="geode-for-redis-replica-count"/a>
* </p>
* <U>Description</U>: Specifies the number of replicas Geode for Redis will attempt to keep
* in the cluster. A value of 0 means no replicas; no extra copies of data will be stored in
* the cluster. Note that extra servers need to be running for replicas to be made. For
* example if the cluster only has one server then no replicas will exist no matter what the
* value of this property is. Also note that Geode for Redis uses a Geode partitioned region
* to implement replicas and this property corresponds to the partitioned region's
* "redundantCopies"
* attribute. Because of this, geode-for-redis-replica-count can also be customized by setting
* {@link #ENFORCE_UNIQUE_HOST} or {@link #REDUNDANCY_ZONE}.
* This property must be set the same on every server in the cluster that is running a
* Geode for Redis server.</td>
* </p>
* <U>Default</U>: 1
* </p>
* <U>Allowed values</U>: 0..3
* </p>
* <U>Since</U>: Geode 1.15
*/
String GEODE_FOR_REDIS_REPLICA_COUNT = "geode-for-redis-replica-count";
/**
* The static String definition of the <i>"lock-memory"</i> property <a name="lock-memory"/a>
* </p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_ENABLED;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REPLICA_COUNT;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_USERNAME;
import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
Expand Down Expand Up @@ -1315,15 +1314,13 @@ static Class _getAttributeType(String attName) {
m.put(MEMCACHED_BIND_ADDRESS,
"The address the GemFireMemcachedServer will listen on for remote connections. Default is \"\" which causes the GemFireMemcachedServer to listen on the host's default address. This property is ignored if memcached-port is \"0\".");
m.put(GEODE_FOR_REDIS_BIND_ADDRESS,
"Specifies the address on which the Geode for Redis server is listening. If set to the empty string or this property is not specified, then all local addresses are listened to. Default is an empty string.");
"Specifies the address on which the Redis API for Geode is listening. If set to the empty string or this property is not specified, localhost is requested from the operating system.");
m.put(GEODE_FOR_REDIS_ENABLED,
"When false Geode for Redis is not available. Set to true to enable Geode for Redis. Default is false.");
"When the default value of false, the Redis API for Geode is not available. Set to true to enable the Redis API for Geode.");
m.put(GEODE_FOR_REDIS_USERNAME,
"Specifies the username that the Geode for Redis server uses when a client attempts to authenticate using only a password. The default is 'default'.");
"Specifies the username that the server uses when a client attempts to authenticate using only a password. The default is 'default'.");
m.put(GEODE_FOR_REDIS_PORT,
"Specifies the port on which the Geode for Redis server listens for client connections. A value of 0 selects a random port. Default is 6379.");
m.put(GEODE_FOR_REDIS_REPLICA_COUNT,
"Specifies how many extra copies of data will be stored in the cluster by Geode for Redis. Default is 1.");
"Specifies the port on which the server listens for Redis API for Geode connections. A value of 0 selects a random port. Default is 6379.");
m.put(ENABLE_CLUSTER_CONFIGURATION,
"Enables cluster configuration support in dedicated locators. This allows the locator to share configuration information amongst members and save configuration changes made using GFSH.");
m.put(ENABLE_MANAGEMENT_REST_SERVICE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_ENABLED;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REPLICA_COUNT;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_USERNAME;
import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
import static org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_BIND_ADDRESS;
Expand Down Expand Up @@ -3501,6 +3500,8 @@ static InetAddress _getDefaultMcastAddress() {
* {@link ConfigurationProperties#GEODE_FOR_REDIS_ENABLED} property
*
* @return boolean value indicating whether or not a Redis API for Geode Server should be started
*
* @since GemFire 14.0
*/
@ConfigAttributeGetter(name = GEODE_FOR_REDIS_ENABLED)
boolean getRedisEnabled();
Expand All @@ -3520,6 +3521,8 @@ static InetAddress _getDefaultMcastAddress() {
* {@link ConfigurationProperties#GEODE_FOR_REDIS_USERNAME} property
*
* @return the authentication username for GeodeRedisServer
*
* @since GemFire 8.0
*/
@ConfigAttributeGetter(name = GEODE_FOR_REDIS_USERNAME)
String getRedisUsername();
Expand All @@ -3535,6 +3538,8 @@ static InetAddress _getDefaultMcastAddress() {
* Returns the value of the {@link ConfigurationProperties#GEODE_FOR_REDIS_PORT} property
*
* @return the port on which GeodeRedisServer should be started
*
* @since GemFire 8.0
*/
@ConfigAttributeGetter(name = GEODE_FOR_REDIS_PORT)
int getRedisPort();
Expand All @@ -3546,22 +3551,6 @@ static InetAddress _getDefaultMcastAddress() {
String REDIS_PORT_NAME = GEODE_FOR_REDIS_PORT;
int DEFAULT_REDIS_PORT = 6379;

/**
* Returns the value of the {@link ConfigurationProperties#GEODE_FOR_REDIS_REPLICA_COUNT} property
*
* @return the Geode for Redis replica count
*
*/
@ConfigAttributeGetter(name = GEODE_FOR_REDIS_REPLICA_COUNT)
int getRedisReplicaCount();

@ConfigAttributeSetter(name = GEODE_FOR_REDIS_REPLICA_COUNT)
void setRedisReplicaCount(int value);

@ConfigAttribute(type = Integer.class, min = 0, max = 3)
String REDIS_REPLICA_COUNT_NAME = GEODE_FOR_REDIS_REPLICA_COUNT;
int DEFAULT_REDIS_REPLICA_COUNT = 1;

// Added for the HTTP service

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,6 @@ public class DistributionConfigImpl extends AbstractDistributionConfig implement
* port on which GeodeRedisServer is started
*/
private int redisPort = DEFAULT_REDIS_PORT;
private int redisReplicaCount = DEFAULT_REDIS_REPLICA_COUNT;


private boolean jmxManager =
Expand Down Expand Up @@ -797,7 +796,6 @@ public DistributionConfigImpl(DistributionConfig other) {
redisBindAddress = other.getRedisBindAddress();
redisUsername = other.getRedisUsername();
redisEnabled = other.getRedisEnabled();
redisReplicaCount = other.getRedisReplicaCount();
userCommandPackages = other.getUserCommandPackages();

// following added for 8.0
Expand Down Expand Up @@ -3309,7 +3307,6 @@ public boolean equals(final Object obj) {
.append(redisBindAddress, that.redisBindAddress)
.append(redisUsername, that.redisUsername)
.append(redisPort, that.redisPort)
.append(redisReplicaCount, that.redisReplicaCount)
.append(redisEnabled, that.redisEnabled)
.append(jmxManagerBindAddress, that.jmxManagerBindAddress)
.append(jmxManagerHostnameForClients, that.jmxManagerHostnameForClients)
Expand Down Expand Up @@ -3405,7 +3402,7 @@ public int hashCode() {
.append(loadSharedConfigurationFromDir).append(clusterConfigDir).append(httpServicePort)
.append(httpServiceBindAddress).append(startDevRestApi).append(memcachedPort)
.append(memcachedProtocol).append(memcachedBindAddress).append(distributedTransactions)
.append(redisPort).append(redisBindAddress).append(redisUsername).append(redisReplicaCount)
.append(redisPort).append(redisBindAddress).append(redisUsername)
.append(redisEnabled).append(jmxManager)
.append(jmxManagerStart).append(jmxManagerPort).append(jmxManagerBindAddress)
.append(jmxManagerHostnameForClients).append(jmxManagerPasswordFile)
Expand Down Expand Up @@ -3516,16 +3513,6 @@ public void setRedisPort(int value) {
redisPort = value;
}

@Override
public int getRedisReplicaCount() {
return redisReplicaCount;
}

@Override
public void setRedisReplicaCount(int value) {
redisReplicaCount = value;
}

@Override
public String getRedisBindAddress() {
return redisBindAddress;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1194,8 +1194,7 @@ public int getDistributedSystemId() {
@Override
public boolean enforceUniqueZone() {
return system.getConfig().getEnforceUniqueHost()
|| (system.getConfig().getRedundancyZone() != null
&& !system.getConfig().getRedundancyZone().isEmpty());
|| system.getConfig().getRedundancyZone() != null;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public void before() {
@Test
public void testGetAttributeNames() {
String[] attNames = AbstractDistributionConfig._getAttNames();
assertThat(attNames).hasSize(170);
assertThat(attNames).hasSize(169);

List boolList = new ArrayList();
List intList = new ArrayList();
Expand Down Expand Up @@ -136,7 +136,7 @@ public void testGetAttributeNames() {
// TODO - This makes no sense. One has no idea what the correct expected number of attributes
// are.
assertThat(boolList).hasSize(36);
assertThat(intList).hasSize(36);
assertThat(intList).hasSize(35);
assertThat(stringList).hasSize(88);
assertThat(fileList).hasSize(5);
assertThat(otherList).hasSize(5);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_BIND_ADDRESS;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_ENABLED;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_PORT;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REPLICA_COUNT;
import static org.apache.geode.test.dunit.IgnoredException.addIgnoredException;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand All @@ -34,11 +33,9 @@
import org.junit.experimental.categories.Category;

import org.apache.geode.internal.AvailablePortHelper;
import org.apache.geode.internal.cache.PartitionedRegion;
import org.apache.geode.internal.inet.LocalHostUtil;
import org.apache.geode.redis.internal.GeodeRedisServer;
import org.apache.geode.redis.internal.GeodeRedisService;
import org.apache.geode.redis.internal.RegionProvider;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.dunit.rules.RedisClusterStartupRule;
Expand Down Expand Up @@ -134,29 +131,6 @@ public void startupFailsGivenInvalidBindAddress() {
"The geode-for-redis-bind-address 1.1.1.1 is not a valid address for this machine");
}

@Test
public void startupFailsGivenInvalidReplicaCount() {
int port = AvailablePortHelper.getRandomAvailableTCPPort();

addIgnoredException("Could not start server compatible with Redis");
assertThatThrownBy(() -> cluster.startServerVM(0, s -> s
.withProperty(GEODE_FOR_REDIS_PORT, "" + port)
.withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost")
.withProperty(GEODE_FOR_REDIS_ENABLED, "true")
.withProperty(GEODE_FOR_REDIS_REPLICA_COUNT, "4")))
.hasStackTraceContaining(
"Could not set \"" + GEODE_FOR_REDIS_REPLICA_COUNT
+ "\" to \"4\" because its value can not be greater than \"3\".");
assertThatThrownBy(() -> cluster.startServerVM(0, s -> s
.withProperty(GEODE_FOR_REDIS_PORT, "" + port)
.withProperty(GEODE_FOR_REDIS_BIND_ADDRESS, "localhost")
.withProperty(GEODE_FOR_REDIS_ENABLED, "true")
.withProperty(GEODE_FOR_REDIS_REPLICA_COUNT, "-1")))
.hasStackTraceContaining(
"Could not set \"" + GEODE_FOR_REDIS_REPLICA_COUNT
+ "\" to \"-1\" because its value can not be less than \"0\".");
}

@Test
public void startupOnSpecifiedPort() {
MemberVM server = cluster.startServerVM(0, s -> s
Expand Down Expand Up @@ -188,34 +162,4 @@ public void startupWorksGivenNoBindAddress() {
assertThat(cluster.getRedisPort(server))
.isNotEqualTo(GeodeRedisServer.DEFAULT_REDIS_SERVER_PORT);
}

@Test
public void startupWorksGivenReplicaCountOfZero() {
MemberVM server = cluster.startServerVM(0, s -> s
.withProperty(GEODE_FOR_REDIS_REPLICA_COUNT, "0")
.withProperty(GEODE_FOR_REDIS_PORT, "0")
.withProperty(GEODE_FOR_REDIS_ENABLED, "true"));

int replicaCount = cluster.getMember(0).invoke("getReplicaCount", () -> {
PartitionedRegion pr = (PartitionedRegion) RedisClusterStartupRule.getCache()
.getRegion(RegionProvider.DEFAULT_REDIS_REGION_NAME);
return pr.getPartitionAttributes().getRedundantCopies();
});
assertThat(replicaCount).isEqualTo(0);
}

@Test
public void startupWorksGivenReplicaCountOfThree() {
MemberVM server = cluster.startServerVM(0, s -> s
.withProperty(GEODE_FOR_REDIS_REPLICA_COUNT, "3")
.withProperty(GEODE_FOR_REDIS_PORT, "0")
.withProperty(GEODE_FOR_REDIS_ENABLED, "true"));

int replicaCount = cluster.getMember(0).invoke("getReplicaCount", () -> {
PartitionedRegion pr = (PartitionedRegion) RedisClusterStartupRule.getCache()
.getRegion(RegionProvider.DEFAULT_REDIS_REGION_NAME);
return pr.getPartitionAttributes().getRedundantCopies();
});
assertThat(replicaCount).isEqualTo(3);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.apache.geode.redis;

import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_REPLICA_COUNT;
import static org.apache.geode.distributed.ConfigurationProperties.GEODE_FOR_REDIS_USERNAME;
import static org.apache.geode.distributed.ConfigurationProperties.LOCATORS;
import static org.apache.geode.distributed.ConfigurationProperties.LOG_LEVEL;
Expand All @@ -39,7 +38,6 @@ public GeodeRedisServerRule() {
cacheFactory.set(LOG_LEVEL, "warn");
cacheFactory.set(MCAST_PORT, "0");
cacheFactory.set(LOCATORS, "");
cacheFactory.set(GEODE_FOR_REDIS_REPLICA_COUNT, "0");
}

public void setEnableUnsupportedCommands(boolean allow) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.apache.geode.cache.RegionShortcut;
import org.apache.geode.cache.partition.PartitionRegionHelper;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.InternalRegionFactory;
import org.apache.geode.internal.cache.PartitionedRegion;
Expand Down Expand Up @@ -89,8 +88,6 @@ public RegionProvider(InternalCache cache, StripedCoordinator stripedCoordinator

PartitionAttributesFactory<RedisKey, RedisData> attributesFactory =
new PartitionAttributesFactory<>();
DistributionConfig config = cache.getInternalDistributedSystem().getConfig();
attributesFactory.setRedundantCopies(config.getRedisReplicaCount());
attributesFactory.setPartitionResolver(new RedisPartitionResolver());
attributesFactory.setTotalNumBuckets(REDIS_REGION_BUCKETS);
redisDataRegionFactory.setPartitionAttributes(attributesFactory.create());
Expand Down

0 comments on commit a18ad46

Please sign in to comment.