ringles commented on a change in pull request #6511:
URL: https://github.com/apache/geode/pull/6511#discussion_r639840448
##########
File path:
geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/set/SremDUnitTest.java
##########
@@ -48,26 +48,21 @@
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
private static JedisCluster jedis;
- private static Properties locatorProperties;
-
- private static MemberVM locator;
private static MemberVM server1;
private static MemberVM server2;
private static MemberVM server3;
- private static int redisServerPort;
-
@BeforeClass
public static void classSetup() {
- locatorProperties = new Properties();
+ Properties locatorProperties = new Properties();
locatorProperties.setProperty(MAX_WAIT_TIME_RECONNECT, "15000");
- locator = clusterStartUp.startLocatorVM(0, locatorProperties);
+ MemberVM locator = clusterStartUp.startLocatorVM(0, locatorProperties);
server1 = clusterStartUp.startRedisVM(1, locator.getPort());
server2 = clusterStartUp.startRedisVM(2, locator.getPort());
server3 = clusterStartUp.startRedisVM(3, locator.getPort());
- redisServerPort = clusterStartUp.getRedisPort(1);
+ int redisServerPort = clusterStartUp.getRedisPort(1);
jedis = new JedisCluster(new HostAndPort(LOCAL_HOST, redisServerPort),
JEDIS_TIMEOUT);
Review comment:
Does it make sense to inline this?
##########
File path:
geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/pubsub/PubSubImpl.java
##########
@@ -66,16 +69,18 @@ public int getSubscriptionCount() {
public long publish(Region<RedisKey, RedisData> dataRegion, byte[] channel,
byte[] message) {
PartitionRegionInfo info =
PartitionRegionHelper.getPartitionRegionInfo(dataRegion);
Set<DistributedMember> membersWithDataRegion = new HashSet<>();
- for (PartitionMemberInfo memberInfo : info.getPartitionMemberInfo()) {
- membersWithDataRegion.add(memberInfo.getDistributedMember());
+ if (info != null) {
Review comment:
Is there a test that checks the necessity of this code?
##########
File path:
geode-apis-compatible-with-redis/src/distributedTest/java/org/apache/geode/redis/internal/executor/set/SaddDUnitTest.java
##########
@@ -48,27 +48,22 @@
Math.toIntExact(GeodeAwaitility.getTimeout().toMillis());
private static JedisCluster jedis;
- private static Properties locatorProperties;
-
- private static MemberVM locator;
private static MemberVM server1;
private static MemberVM server2;
private static MemberVM server3;
- private static int redisServerPort;
-
@BeforeClass
public static void classSetup() {
- locatorProperties = new Properties();
+ Properties locatorProperties = new Properties();
locatorProperties.setProperty(MAX_WAIT_TIME_RECONNECT, "15000");
- locator = clusterStartUp.startLocatorVM(0, locatorProperties);
+ MemberVM locator = clusterStartUp.startLocatorVM(0, locatorProperties);
server1 = clusterStartUp.startRedisVM(1, locator.getPort());
server2 = clusterStartUp.startRedisVM(2, locator.getPort());
server3 = clusterStartUp.startRedisVM(3, locator.getPort());
- redisServerPort = clusterStartUp.getRedisPort(1);
+ int redisServerPort = clusterStartUp.getRedisPort(1);
Review comment:
See SremDunitTest - this can be inlined
--
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
