[
https://issues.apache.org/jira/browse/GEODE-8515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17201688#comment-17201688
]
ASF GitHub Bot commented on GEODE-8515:
---------------------------------------
dschneider-pivotal commented on a change in pull request #5544:
URL: https://github.com/apache/geode/pull/5544#discussion_r494487359
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/pubsub/SubscriptionsIntegrationTest.java
##########
@@ -39,9 +38,8 @@
public static ExecutorServiceRule executor = new ExecutorServiceRule();
@Test
- @Ignore("GEODE-8515")
public void pingWhileSubscribed() {
- Jedis client = new Jedis("localhost", server.getPort());
+ Jedis client = new Jedis("localhost", server.getPort(), 1000000000);
Review comment:
why a long timeout on this one but not on
pingWithArgumentWhileSubscribed?
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/PingExecutor.java
##########
@@ -31,9 +32,17 @@ public RedisResponse executeCommand(Command command,
ExecutionHandlerContext context) {
List<byte[]> commandElems = command.getProcessedCommand();
byte[] result = PING_RESPONSE.getBytes();
+ byte[] subscribeResult = "".getBytes();
if (commandElems.size() > 1) {
result = commandElems.get(1);
+ subscribeResult = result;
}
+
+ if
(!context.getPubSub().findSubscribedChannels(context.getClient()).isEmpty()) {
Review comment:
consider refactoring this so that we only compute the RedisResponse for
subscribe if we find a client and then otherwise with (an else) compute the
RedisResponse when not subscribed. I don't like all the logic we have
initializing two different results and then we only end up using one or the
other. I'd prefer that we only have state at the top that would be shared by
either branch (for example commandElems). Once the RedisResponse is done we can
then fall through to a command return.
----------------------------------------------------------------
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
> Redis PING should respond appropriately when called from within a
> SUBSCRIBE/PSUBSCRIBE
> --------------------------------------------------------------------------------------
>
> Key: GEODE-8515
> URL: https://issues.apache.org/jira/browse/GEODE-8515
> Project: Geode
> Issue Type: Improvement
> Components: redis
> Reporter: Sarah Abbey
> Assignee: Sarah Abbey
> Priority: Minor
> Labels: pull-request-available
>
> From PING documentation (https://redis.io/commands/ping):
> If the client is subscribed to a channel or a pattern, it will instead return
> a multi-bulk with a "pong" in the first position and an empty bulk in the
> second position, unless an argument is provided in which case it returns a
> copy of the argument.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
