dschneider-pivotal commented on a change in pull request #6686:
URL: https://github.com/apache/geode/pull/6686#discussion_r672556499
##########
File path:
geode-apis-compatible-with-redis/src/commonTest/java/org/apache/geode/redis/GeodeRedisServerRule.java
##########
@@ -49,6 +49,12 @@ protected void before() {
cache = cacheFactory.create();
server = new GeodeRedisServer("localhost", 0, (InternalCache) cache);
server.setAllowUnsupportedCommands(enableUnsupportedCommands);
+
+ // Ensure that buckets are created up front
+ try {
Review comment:
I think this block should be refactored into a method on
GeodeRedisServer.
Also it looks to me like it will be happy even if a bucket can not be
created. It looks like we basically need to call SlotAdvisor.getMemberInfo for
each bucketId but that method has some retry logic in it and it can fail to get
the member info. In that case it logs an error and returns null but this code
ends up ignoring those nulls and moving on. I don't know if you want something
different here that would fail the startup if one of the memberInfo calls times
out.
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/set/SScanExecutor.java
##########
@@ -66,12 +67,13 @@ public RedisResponse executeCommand(Command command,
ExecutionHandlerContext con
RedisKey key = command.getKey();
- if (!getDataRegion(context).containsKey(key)) {
+ RedisData value = context.getRegionProvider().getRedisData(key, null);
Review comment:
this is another place that could be changed to
"context.getRegionProvider().getRedisData(key);
if (value.isNull()"
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
