jhutchison commented on a change in pull request #5067:
URL: https://github.com/apache/geode/pull/5067#discussion_r425394439
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java
##########
@@ -240,30 +237,18 @@
*/
public static final String STRING_REGION = "ReDiS_StRiNgS";
- /**
- * TThe field that defines the name of the {@link Region} which holds
non-named hash. The current
- * value of this field is {@value #HASH_REGION}.
- */
- public static final String HASH_REGION = "ReDiS_HASH";
-
- /**
- * TThe field that defines the name of the {@link Region} which holds sets.
The current value of
- * this field is {@value #SET_REGION}.
- */
- public static final String SET_REGION = "ReDiS_SET";
-
-
/**
* The field that defines the name of the {@link Region} which holds all of
the HyperLogLogs. The
* current value of this field is {@code HLL_REGION}.
*/
public static final String HLL_REGION = "ReDiS_HlL";
/**
- * The field that defines the name of the {@link Region} which holds all of
the Redis meta data.
- * The current value of this field is {@code REDIS_META_DATA_REGION}.
+ * The name of the region that holds data stored in redis.
+ * Currently this is the meta data but at some point the value for a
particular
+ * type will be changed to an instance of {@link RedisData}
*/
- public static final String REDIS_META_DATA_REGION = "ReDiS_MeTa_DaTa";
+ public static final String REDIS_DATA_REGION = "ReDiS_MeTa_DaTa";
Review comment:
maybe the value of this string is just redis_data now? (or ReDiS_daTa?)
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java
##########
@@ -462,37 +445,25 @@ private void initializeRedis() {
hLLRegion = regionFactory.create(HLL_REGION);
}
- if ((redisHash = cache.getRegion(HASH_REGION)) == null) {
- RegionFactory<ByteArrayWrapper, RedisHash> regionFactory =
- gemFireCache.createRegionFactory(DEFAULT_REGION_TYPE);
- redisHash = regionFactory.create(HASH_REGION);
- }
-
- if ((redisSet = cache.getRegion(SET_REGION)) == null) {
- RegionFactory<ByteArrayWrapper, RedisSet> regionFactory =
- gemFireCache.createRegionFactory(DEFAULT_REGION_TYPE);
- redisSet = regionFactory.create(SET_REGION);
- }
-
- if ((redisMetaData = cache.getRegion(REDIS_META_DATA_REGION)) == null) {
- InternalRegionFactory<String, RedisDataType> redisMetaDataFactory =
- gemFireCache.createInternalRegionFactory();
+ if ((redisData = cache.getRegion(REDIS_DATA_REGION)) == null) {
+ InternalRegionFactory<ByteArrayWrapper, RedisData>
redisMetaDataFactory =
+ gemFireCache.createInternalRegionFactory(DEFAULT_REGION_TYPE);
redisMetaDataFactory.addCacheListener(metaListener);
- redisMetaDataFactory.setDataPolicy(DataPolicy.REPLICATE);
redisMetaDataFactory.setInternalRegion(true).setIsUsedForMetaRegion(true);
- redisMetaData = redisMetaDataFactory.create(REDIS_META_DATA_REGION);
+ redisData = redisMetaDataFactory.create(REDIS_DATA_REGION);
}
- keyRegistrar = new KeyRegistrar(redisMetaData);
+ keyRegistrar = new KeyRegistrar(redisData);
Review comment:
This may be useful for reasons that I'm not immediately grasping, but do
we still need the KeyRegistrar abstraction?
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java
##########
@@ -437,18 +422,16 @@ private void startGemFire() {
}
@VisibleForTesting
- public RegionProvider getRegionCache() {
- return regionCache;
+ public RegionProvider getRegionProvider() {
Review comment:
Don't feel the need to respond to this, but also curious if this change
will allow us to eventually get rid of the region provider concept.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
