sewenew / redis-plus-plus

Redis client written in C++
Apache License 2.0
1.6k stars 347 forks source link

[BUG] Sharding is using the wrong key on module commands #513

Closed victor-timofei closed 1 year ago

victor-timofei commented 1 year ago

Describe the bug We are trying to send a redis module command using the AsyncRedisCluster interface. We are using the generic command interface AsyncRedisCluster::command(Input first, Input last, Callback &&cb). Our redis module command format looks like module-name subcommand <key> ...args.

We discovered that the redis++ library uses the second argument as the key for the sharding. In our case as it can be seen in the example command format, the key is the 3rd argument.

To Reproduce

This minimal code is without async and callbacks due to its simplicity, but the behavior is the same.

       auto redis = RedisCluster("tcp://127.0.0.1:6379");

        std::vector<std::string> raw_cmd;
        raw_cmd.push_back("module-name");
        raw_cmd.push_back("create");
        raw_cmd.push_back("key2");
        raw_cmd.push_back("arg1");
        raw_cmd.push_back("arg2");
        redis.command<void>(raw_cmd.begin(), raw_cmd.end());

Expected behavior We would like to be able to use the command interface so as to be able to send to db commands like:

module-name create key1 arg1 arg2..
module-name delete key2 arg3 arg4 arg5..

And do the hashing based on the key.

Environment:

Additional context Add any other context about the problem here.

sewenew commented 1 year ago

Yes, redis-plus-plus' generic interface uses the second command argument as key. In order to work around it, you can create an AsyncRedis object with AsyncRedisCluster, and send the command to it with this new AsyncRedis object:

auto r = async_redis_cluster.redis("key", false); // create it with a connection from the underlying connection pool
r.command<void>(raw_cmd.begin(), raw_cmd.end());

Regards

victor-timofei commented 1 year ago

Hi @sewenew. Thanks a lot for the workaround. We validated this works.

I think it would be useful to have this workaround as a note on the docs under the Redis Modules section. I could help with adding that.

sewenew commented 1 year ago

@victor-timofei You're always welcome to create a pull request to fix the doc. Thanks a lot!

Regards