Call client setname on a connection to cluster - currently blocked

[dev-konrad]

Hi,

I would like to client setname on connections to redis cluster using your library, but unfortunately they are in a 'blocked' list of commands, what prevents me from doing it.

Are you able to suggest if there's any way to achieve it using your library?

Thanks in advance,
Konrad

[Grokzen]

It is blocked because of this line

redis-py-cluster/rediscluster/client.py

Line 52 in d811250

"CLIENT SETNAME", "SENTINEL GET-MASTER-ADDR-BY-NAME", 'SENTINEL MASTER', 'SENTINEL MASTERS',

and according to this

redis-py-cluster/docs/commands.rst

Line 87 in b12c353

- client_setname - Not yet implemented

it says that the command has not yet been implemented.

Basically what you need to find out to implement this command is to know how exactly this command should work. Should it be sent to a specific node, all nodes, based on a slot etc?

If you can find that out then i can implement the command properly much faster.

[dev-konrad]

Hi @Grokzen,

OK, I can have a look into this. I'll check how it's done in java library lettuce to see how it's implemented there.

Thanks,
Konrad

[Grokzen]

That sounds like a very good idea

[dev-konrad]

OK, so this is what I managed to find in RedisAdvancedClusterAsyncCommandsImpl.java - to me it looks like the command is called on all nodes.

` 
 @Override
    public RedisFuture<String> clientSetname(K name) {

        Map<String, CompletionStage<String>> executions = new HashMap<>();

        CompletableFuture<String> ok = CompletableFuture.completedFuture("OK");

        executions.put("Default", super.clientSetname(name).toCompletableFuture());

        for (RedisClusterNode redisClusterNode : getStatefulConnection().getPartitions()) {

            RedisURI uri = redisClusterNode.getUri();

            CompletableFuture<RedisClusterAsyncCommands<K, V>> byNodeId = getConnectionAsync(redisClusterNode.getNodeId());

            executions.put("NodeId: " + redisClusterNode.getNodeId(), byNodeId.thenCompose(c -> {

                if (c.isOpen()) {
                    return c.clientSetname(name);
                }
                return ok;
            }));

            CompletableFuture<RedisClusterAsyncCommands<K, V>> byHost = getConnectionAsync(uri.getHost(), uri.getPort());

            executions.put("HostAndPort: " + redisClusterNode.getNodeId(), byHost.thenCompose(c -> {

                if (c.isOpen()) {
                    return c.clientSetname(name);
                }
                return ok;
            }));
        }

        return MultiNodeExecution.firstOfAsync(executions);
`

As it comes to what a partition is:

Cluster topology view. An instance of {@link Partitions} provides access to the partitions of a Redis Cluster. A partition is
 * represented by a Redis Cluster node that has a {@link RedisClusterNode#getNodeId() nodeId} and
 * {@link RedisClusterNode#getUri() connection point details}.
 * <p>
 * Partitions can be looked up by {@code nodeId} or {@code slot} (masters only). A nodeId can be migrated to a different host.
 * Partitions are cached to ensure a cheap lookup by {@code slot}. Users of {@link Partitions} are required to call
 * {@link #updateCache()} after topology changes occur.

Would that be enough for you?

Thanks,
Konrad

[Grokzen]

Yes i think that should be easily replicated so the next release could include that fix.

[dev-konrad]

Awesome, thanks.
And when is the next release planned for?

Best regards,
Konrad

[Grokzen]

I don't have a planned date for it. Last release was very slow and drawn out to get done for me. I hope to pick up the pace and get more fixes and new versions supported much faster then what 2.0.0 was.

[dev-konrad]

OK, I understand. And is there a chance this fix is going to get into the repository soon so I can apply it and test?

Thanks,
Konrad

[dev-konrad]

FYI I managed to get what I want just by extending ClusterConnection, without even touching your code.

from rediscluster.connection import ClusterConnection

class NamedClusterConnection(ClusterConnection):

    def __init__(self, **kwargs):
        self.client_name = kwargs.pop('redis_client_name', None)
        super(NamedClusterConnection, self).__init__(**kwargs)

    def on_connect(self):
        '''
        Initialize the connection, authenticate and select a database and send READONLY if it is
        set during object initialization.
        '''
        super(ClusterConnection, self).on_connect()

        if self.client_name is not None:
            printf(f'sending CLIENT SETNAME {self.client_name} command')
            self.send_command(f'CLIENT SETNAME {self.client_name}')

            if nativestr(self.read_response()) != 'OK':
                printf(f'CLIENT SETNAME {self.client_name} command failed')

...

additional_kwargs = {'redis_client_name': 'some_client_name'}
client = StrictRedisCluster.from_url(url=url, decode_responses=True, connection_class=NamedClusterConnection, **additional_kwargs)

Thing is, you've got to set this client name on all connections whenever they open.
I learnt that during playing with Redis cluster, also with libraries for other languages.
In other words it makes no sense to just set client name on all nodes somewhere, as this name only lives on a connection.
Therefore you gotta set it like a password for instance (AUTH).

BR,
Konrad

[Grokzen]

Yeah, that looks like a hack that works for now, but yeah, you dont really want to make your own connection class really, but if it solves it for you now, then go for it.

What is the use-case for using CLIENT SETNAME? what feature and use does it provide for you?

[dev-konrad]

So, I'm introducing Redis into an existing application now and am trying to analyze a big number of open connections to Redis.
Setting client name to something meaningful (to human) will help me interpreting CLIENT LIST, which normally have IP-s that don't say much.
Other than that, it's useful to have a quick look into this list and see what's connected to Redis.

I'm happy to upgrade to a newer version once the functionality is delivered, but for now I would like to start my analysing without waiting.

BTW Do you plan to set the client name for each new connection?

Thanks,
Konrad

[Grokzen]

I dont know really. I would have to dig deeper into what the java lib is doing, but my guess is that i will just replicate whatever redis-py is doing and then just fanout it to all nodes. More then that i dont know now.

[dev-konrad]

OK, sure thing.
Just FYI I spent some time analysing lettuce and found this nice feature that I actually needed:
when you set client client name in RedisURI, it gets propagated later on to all new connections.
So in other words, you set it once, kind of like an item of configuration next to URL, password etc., and then library makes sure it gets used, whenever a new connection to node is made.
That's the best way to do it in my opinion.

[Grokzen]

@MrUKI If you can show that this feature is something that is implemented inside redis-py but not working in this lib then i will fix it. Otherwise this sounds like a feature that should be included in the upstream repo first so it will then propegate down to this lib in a updated version of redis-py-cluster. If this would be a cluster onlly feature that you found inside lettuce then i will consider it, otherwise you have to propse it to the upstream package.