dschneider-pivotal commented on a change in pull request #5067:
URL: https://github.com/apache/geode/pull/5067#discussion_r425447693
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java
##########
@@ -526,20 +499,28 @@ private void registerLockServiceMBean() {
}
private void checkForRegions() {
- Collection<Entry<String, RedisDataType>> entrySet =
keyRegistrar.keyInfos();
- for (Entry<String, RedisDataType> entry : entrySet) {
- String regionName = entry.getKey();
- RedisDataType type = entry.getValue();
- Region<?, ?> newRegion = cache.getRegion(regionName);
- if (newRegion == null && type != RedisDataType.REDIS_STRING && type !=
RedisDataType.REDIS_HLL
- && type != RedisDataType.REDIS_PROTECTED) {
- try {
- regionCache
-
.createRemoteRegionReferenceLocally(Coder.stringToByteArrayWrapper(regionName),
type);
- } catch (Exception e) {
- if (logger.errorEnabled()) {
- logger.error(e);
- }
+ Collection<Entry<ByteArrayWrapper, RedisData>> entrySet =
keyRegistrar.keyInfos();
+ for (Entry<ByteArrayWrapper, RedisData> entry : entrySet) {
+ ByteArrayWrapper key = entry.getKey();
+ RedisDataType type = entry.getValue().getType();
+ if (!regionProvider.typeUsesDynamicRegions(type)) {
+ continue;
+ }
+ if (cache.getRegion(key.toString()) != null) {
+ // TODO: this seems to be correct (i.e. no need to call
createRemoteRegionReferenceLocally
+ // if region already exists).
+ // HOWEVER: createRemoteRegionReferenceLocally ends up doing nothing
if the region does not
+ // exist. So this caller of createRemoteRegionReferenceLocally
basically does nothing.
+ // createRemoteRegionReferenceLocally might be needed even if the
region exists because
+ // local state needs to be initialized (like indexes and queries).
+ continue;
+ }
+ try {
+ regionProvider.createRemoteRegionReferenceLocally(key, type);
+ } catch (Exception e) {
+ // TODO: this eats the exception so if something really is wrong we
don't fail but just log.
+ if (logger.errorEnabled()) {
+ logger.error(e);
}
Review comment:
Once we change them not to use a region per list/zset then this code
goes away. But I think this TODO just marks HA bug we have with these dynamic
regions. It would cause a new server to never create an already existing
dynamic region for a list/zset.
----------------------------------------------------------------
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
