Skip to content

Commit

Permalink
GEODE-7828: Convert backing store for Redis Hashes and Sets to single…
Browse files Browse the repository at this point in the history
… regions (#4745)

* GEODE-7828: Convert backing store for Redis Hashes and Sets to single regions

This PR builds on the work originally submitted by Greg Green in
#404. Also acknowledgements to Galen
O'Sullivan for addressing locking issues in that PR.

This commit changes the storage model of Redis hashes and sets from one
region per each Redis key to a single hash region and single set region.
The Redis key is now also the region key and the data is stored in a Map
and Set respectively in the region. Currently, the backing values do not
implement Delta changes, however this will be a future optimization.

This also fixes the inability of Redis keys to contain other characters
commonly used, such as colons (':').

- Add `RedisLockService` which manages a lock per key within a single
  JVM. Locks are held in a WeakHasMap to allow for automatic cleanup
  (prior PR work, using a pure ConcurrentHashMap, ended up leaking
  memory since there is no straight-forward way to clean up unused
  keys/locks).
- Add new tests including concurrency tests for hashes and sets

Co-authored-by: Greg Green <ggreen@pivotal.io>
Co-authored-by: Ray Ingles <ringles@pivotal.io>
Co-authored-by: Sarah Abbey <sabbey@pivotal.io>
Co-authored-by: John Hutchison <jhutchison@pivotal.io>
Co-authored-by: Murtuza Boxwala <mboxwala@pivotal.io>
Co-authored-by: Prasath Durairaj <prasathd@pivotal.io>
Co-authored-by: Jens Deppe <jdeppe@pivotal.io>
  • Loading branch information
7 people committed Mar 3, 2020
1 parent 4b06a7a commit 2f6bf01
Show file tree
Hide file tree
Showing 138 changed files with 4,334 additions and 1,228 deletions.
3 changes: 0 additions & 3 deletions geode-redis/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,15 @@ dependencies {
implementation('io.netty:netty-all')
implementation('org.apache.logging.log4j:log4j-api')


testImplementation(project(':geode-junit'))
testImplementation('org.mockito:mockito-core')


integrationTestImplementation(project(':geode-dunit'))
integrationTestImplementation(project(':geode-junit'))
integrationTestImplementation('redis.clients:jedis')

integrationTestRuntime(project(':geode-log4j'))


distributedTestImplementation(project(':geode-dunit'))
distributedTestImplementation('redis.clients:jedis')
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import org.apache.geode.redis.internal.GeodeRedisService;

public class AnalyzeRedisSerializablesJUnitTest extends AnalyzeSerializablesJUnitTestBase {
public class AnalyzeRedisSerializablesIntegrationTest extends AnalyzeSerializablesJUnitTestBase {

@Override
protected String getModuleName() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import org.apache.geode.test.junit.categories.RedisTest;

@Category({RedisTest.class})
public class AuthJUnitTest {
public class AuthIntegrationTest {

private static final String PASSWORD = "pwd";
Jedis jedis;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.apache.geode.test.junit.categories.RedisTest;

@Category({RedisTest.class})
public class ConcurrentStartTest {
public class ConcurrentStartIntegrationTest {

private Cache cache;
private int numServers = 10;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.apache.geode.redis.internal.CoderException;
import org.apache.geode.redis.internal.GeoCoder;

public class GeoCoderTest {
public class GeoCoderIntegrationTest {
@Test
public void testGeoHash() throws CoderException {
String hash = GeoCoder.geohash(Double.toString(13.361389).getBytes(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@
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.apache.geode.internal.Assert.assertTrue;
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.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Random;

import org.junit.After;
import org.junit.AfterClass;
Expand All @@ -47,16 +46,14 @@
import org.apache.geode.test.junit.categories.RedisTest;

@Category({RedisTest.class})
public class GeoJUnitTest {
public class GeoIntegrationTest {
private static Jedis jedis;
private static GeodeRedisServer server;
private static GemFireCache cache;
private static Random rand;
private static int port = 6379;

@BeforeClass
public static void setUp() throws IOException {
rand = new Random();
CacheFactory cf = new CacheFactory();
// cf.set("log-file", "redis.log");
cf.set(LOG_LEVEL, "error");
Expand All @@ -70,6 +67,12 @@ public static void setUp() throws IOException {
jedis = new Jedis("localhost", port, 10000000);
}

@After
public void cleanup() {
jedis.zrem("Sicily", "Palermo", "Catania");
}


@Test
public void testGeoAdd() {
Map<String, GeoCoordinate> memberCoordinateMap = new HashMap<>();
Expand Down

0 comments on commit 2f6bf01

Please sign in to comment.