Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support INFO command in UnifiedJedis #4078

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/main/java/redis/clients/jedis/UnifiedJedis.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import redis.clients.jedis.search.schemafields.SchemaField;
import redis.clients.jedis.timeseries.*;
import redis.clients.jedis.util.IOUtils;
import redis.clients.jedis.util.JedisBroadcastReplies;
import redis.clients.jedis.util.JedisURIHelper;
import redis.clients.jedis.util.KeyValue;

Expand Down Expand Up @@ -333,6 +334,10 @@ public void setBroadcastAndRoundRobinConfig(JedisBroadcastAndRoundRobinConfig co
this.commandObjects.setBroadcastAndRoundRobinConfig(this.broadcastAndRoundRobinConfig);
}

public final JedisBroadcastReplies broadcastCommandDifferingReplies(CommandObject commandObject) {
return executor.broadcastCommandDifferingReplies(commandObject);
}

public Cache getCache() {
return cache;
}
Expand All @@ -353,6 +358,16 @@ public String configSet(String parameter, String value) {
return checkAndBroadcastCommand(commandObjects.configSet(parameter, value));
}

public final JedisBroadcastReplies info() {
Copy link
Contributor

@ggivo ggivo Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be a bit strange API when connecting to standalone Redis using let's say UnifiedJedis.
Then you will get a JedisBroadcastReplies which makes sense for the cluster.
But in case of standalone, you will get JedisBroadcastReplies, and to get to actual value need to invoke JedisBroadcastReplies::getReplies(with host/port)

or end up with something like
infoReplies.getReplies().values().stream().findFirst().get()
just to get the value

return executor.broadcastCommandDifferingReplies(new CommandObject(
new CommandArguments(Protocol.Command.INFO), BuilderFactory.STRING));
}

public final JedisBroadcastReplies info(String section) {
return executor.broadcastCommandDifferingReplies(new CommandObject(
new CommandArguments(Protocol.Command.INFO).add(section), BuilderFactory.STRING));
}

// Key commands
@Override
public boolean exists(String key) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import redis.clients.jedis.exceptions.*;
import redis.clients.jedis.providers.ClusterConnectionProvider;
import redis.clients.jedis.util.IOUtils;
import redis.clients.jedis.util.JedisBroadcastReplies;

public class ClusterCommandExecutor implements CommandExecutor {

Expand Down Expand Up @@ -71,6 +72,24 @@ public final <T> T broadcastCommand(CommandObject<T> commandObject) {
return reply;
}

@Override
public final JedisBroadcastReplies broadcastCommandDifferingReplies(CommandObject commandObject) {
Map<String, ConnectionPool> connectionMap = provider.getConnectionMap();

JedisBroadcastReplies bcastReplies = new JedisBroadcastReplies(connectionMap.size());
for (Map.Entry<String, ConnectionPool> entry : connectionMap.entrySet()) {
HostAndPort node = HostAndPort.from(entry.getKey());
ConnectionPool pool = entry.getValue();
try (Connection connection = pool.getResource()) {
Object aReply = execute(connection, commandObject);
bcastReplies.addReply(node, aReply);
} catch (Exception anError) {
bcastReplies.addReply(node, anError);
}
}
return bcastReplies;
}

@Override
public final <T> T executeCommand(CommandObject<T> commandObject) {
return doExecuteCommand(commandObject, false);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package redis.clients.jedis.executors;

import redis.clients.jedis.CommandObject;
import redis.clients.jedis.util.JedisBroadcastReplies;

public interface CommandExecutor extends AutoCloseable {

Expand All @@ -9,4 +10,8 @@ public interface CommandExecutor extends AutoCloseable {
default <T> T broadcastCommand(CommandObject<T> commandObject) {
return executeCommand(commandObject);
}

default JedisBroadcastReplies broadcastCommandDifferingReplies(CommandObject commandObject) {
return JedisBroadcastReplies.singleton(executeCommand(commandObject));
}
}
40 changes: 40 additions & 0 deletions src/main/java/redis/clients/jedis/util/JedisBroadcastReplies.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package redis.clients.jedis.util;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

/**
* The collection of replies where a command is broadcasted to multiple nodes, but the replies are expected to differ
* even in ideal situation.
*/
public class JedisBroadcastReplies {

private static final Object DUMMY_OBJECT = new Object();

private final Map<Object, Object> replies;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not Map<HostAndPort, Object> replies; or NodeID?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we don't a specified solution for this, e.g. in ConnectionProvider and CommandExecutor interfaces, yet. For example, in ConnectionProvider interface the respective method definition is Map<?, ?> getConnectionMap().


public JedisBroadcastReplies() {
this(new HashMap<>());
}

public JedisBroadcastReplies(int size) {
this(new HashMap<>(size));
}

private JedisBroadcastReplies(Map<Object, Object> replies) {
this.replies = replies;
}

public void addReply(Object node, Object reply) {
replies.put(node, reply);
}

public Map<Object, Object> getReplies() {
return Collections.unmodifiableMap(replies);
}

public static JedisBroadcastReplies singleton(Object reply) {
return new JedisBroadcastReplies(Collections.singletonMap(DUMMY_OBJECT, reply));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package redis.clients.jedis.commands.jedis;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
Expand All @@ -10,6 +11,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.hamcrest.Matchers;
import org.junit.Test;

import redis.clients.jedis.BuilderFactory;
Expand All @@ -23,6 +25,7 @@
import redis.clients.jedis.params.GeoRadiusParam;
import redis.clients.jedis.params.GeoRadiusStoreParam;
import redis.clients.jedis.resps.ScanResult;
import redis.clients.jedis.util.JedisBroadcastReplies;

public class ClusterValuesCommandsTest extends ClusterJedisCommandsTestBase {

Expand Down Expand Up @@ -128,6 +131,21 @@ public void pingBroadcast() {
assertEquals("PONG", cluster.ping());
}

@Test
public void broadcastDifferentReplies() {
JedisBroadcastReplies infoReplies = cluster.info();
assertThat(infoReplies.getReplies(), Matchers.aMapWithSize(3));
infoReplies.getReplies().values().forEach(infoValue -> {
assertThat((String) infoValue, Matchers.notNullValue());
});

infoReplies = cluster.info("server");
assertThat(infoReplies.getReplies(), Matchers.aMapWithSize(3));
infoReplies.getReplies().values().forEach(infoValue -> {
assertThat((String) infoValue, Matchers.notNullValue());
});
}

@Test
public void flushAllBroadcast() {
assertNull(cluster.get("foo"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package redis.clients.jedis.commands.unified.pooled;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
Expand All @@ -9,6 +10,7 @@
import java.util.List;

import io.redis.test.annotations.SinceRedisVersion;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -23,6 +25,7 @@
import redis.clients.jedis.commands.unified.UnifiedJedisCommandsTestBase;
import redis.clients.jedis.exceptions.JedisDataException;
import redis.clients.jedis.util.EnabledOnCommandRule;
import redis.clients.jedis.util.JedisBroadcastReplies;
import redis.clients.jedis.util.RedisVersionRule;

@RunWith(Parameterized.class)
Expand Down Expand Up @@ -164,4 +167,17 @@ public void broadcastWithError() {
() -> jedis.functionDelete("xyz"));
assertEquals("ERR Library not found", error.getMessage());
}

@Test
public void broadcastDifferentReplies() {
JedisBroadcastReplies infoReplies = jedis.info();
assertThat(infoReplies.getReplies(), Matchers.aMapWithSize(1));
Object infoValue = infoReplies.getReplies().values().stream().findFirst().get();
assertThat((String) infoValue, Matchers.notNullValue());

infoReplies = jedis.info("server");
assertThat(infoReplies.getReplies(), Matchers.aMapWithSize(1));
infoValue = infoReplies.getReplies().values().stream().findFirst().get();
assertThat((String) infoValue, Matchers.notNullValue());
}
}
Loading