JedisClusterConnection. Execute. executeCommandOnArbitraryNode. [DATAREDIS-1010]

[spring-projects-issues]

Pavel Khokhlov opened DATAREDIS-1010 and commented

Current implementation of method:

org.springframework.data.redis.connection.jedis.JedisClusterConnection#execute(java.lang.String, byte[]...)

public Object execute(String command, byte[]... args) {

   Assert.notNull(command, "Command must not be null!");
   Assert.notNull(args, "Args must not be null!");

   return clusterCommandExecutor
         .executeCommandOnArbitraryNode((JedisClusterCommandCallback<Object>) client -> JedisClientUtils.execute(command,
               EMPTY_2D_BYTE_ARRAY, args, () -> client))
         .getValue();
}

Do execution of command on executeCommandOnArbitraryNode

which is queerly because potentially in Сluster  we could run command on each cluster node

For example: "FLUSHDB ASYNC"

In general we should have a choice in which way we should run a command otherwise current solution do not cover all cases.

I would replace current implementation with executeCommandOnAllNodes it would be more universal.

Open discussion:

https://github.com/spring-projects/spring-data-redis/pull/283/files#r294802916

 

---

Affects: 2.2 RC1 (Moore)

1 votes, 3 watchers

[spring-projects-issues]

Mark Paluch commented

Running a command on all nodes seems specific to your use-case. With Jedis, we have no correlation to keys that would allow us to route commands to a specific node. For Lettuce, the Lettuce driver itself routes commands to a single node.

Changing the method behavior would be a breaking change.

With Redis Cluster, we do not have a single strategy that fits all commands. It's more that certain commands need to be executed on a specific node (key-bound), some commands can be executed anywhere (PUBLISH), some commands (write commands, FLUSHDB) only on masters and some on the entire cluster (PING, SCRIPT LOAD)

[spring-projects-issues]

Pavel Khokhlov commented

Hello Mark Paluch !

Thank you for reply.

I agree that it's kind of specific case but I think user should have a choice  (API should allow to do it).

In our case we potentially can use this function

org.springframework.data.redis.connection.jedis.JedisClusterServerCommands#flushAll()

@Override
public void flushAll() {
   connection.getClusterCommandExecutor()
         .executeCommandOnAllNodes((JedisClusterCommandCallback<String>) BinaryJedis::flushAll);
}

but unfortunately Jedis implementation of flushAll doesn't support ASYNC option.

https://redis.io/commands/flushall

So we don't have a choice. Only one way to run this command is run it over execute method.

Otherwise we have to override current implementation of

org.springframework.data.redis.connection.jedis.JedisClusterConnection and overrides execute method

which looks very odd...

 

 

[spring-projects-issues]

Mark Paluch commented

Our general recommendation is using the client directly if you have a case which isn't covered. You can obtain JedisCluster by calling JedisClusterConnection.getNativeConnection

[spring-projects-issues]

Pavel Khokhlov commented

Hello Mark Paluch!

I think for me would be better choice to have ability to create my own version of JedisClusterConnection and add specific method with keeping current implementation of  execute method

But for this exercise would be nice if I had access to private fields (cluster, clusterCommandExecutor, topologyProvider) in 

org.springframework.data.redis.connection.jedis.JedisConnectionFactory

and overwrite one method in my own connection factory

@Override
public RedisClusterConnection getClusterConnection() {

   if (!isRedisClusterAware()) {
      throw new InvalidDataAccessApiUsageException("Cluster is not configured!");
   }
   return new CustomJedisClusterConnection(this.cluster, this.clusterCommandExecutor, this.topologyProvider);
}

So in this case we can still use original JedisClusterConnection + required stuff.

What do you think is it possible just create public getters for class org.springframework.data.redis.connection.jedis.JedisConnectionFactory ?

public JedisCluster getCluster() {
    return cluster;
}

public ClusterTopologyProvider getTopologyProvider() {
    return topologyProvider;
}

public ClusterCommandExecutor getClusterCommandExecutor() {
    return clusterCommandExecutor;
}

Thank you.

 

[spring-projects-issues]

hradilf commented

Hi Christoph Strobl,
please, is there any timeline available for this improvement?
Thank you