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

is redisClusterAppendCommand, redisClusterGetReply thread Safe? #193

Open
abhinavbansal19961996 opened this issue Oct 5, 2023 · 1 comment

Comments

@abhinavbansal19961996
Copy link

abhinavbansal19961996 commented Oct 5, 2023

We have multiple redis context connecting to different redis clusters, and both contexts will be running in seperate threads and will be passed in redisClusterAppendCommand and redisClusterGetReply function. Do you think there will be any issues? Is there any shared variable which can cause problem in above function call?

Reason i am saying is, we did some load test and we believe that our latency is getting affected. To explain in more detail, earlier we just had single context. It has suppose latency X for redisClusterAppendCommand and latency Y for redisClusterGetReply. Now we have one more redis context, and we called above functions and we saw for old redis context latency for redisClusterAppendCommand increased from X to Some X+T and for redisClusterGetReply from Y to Y+T`.

Technically i am asking two questions:

  1. In serialise call to above two functions for two different redis context, is it supported? Should we expect increase in latency for our original context for above function calls?
  2. In parallel calls to above function with two different redis context, is there any shared variable which can cause issues while calling above two functions?
@bjosv
Copy link
Collaborator

bjosv commented Oct 6, 2023

Interesting. I don't think the latency for individual calls to the blocking API should increase if you have different contexts and run them in the same thread. The redisClusterAppendCommand() command just adds the commands to an internal queue in hiredis and redisClusterGetReply() triggers the socket send and wait for a reply.

To avoid this serial send+wait on multiple contexts you might want to look into the async-API option, i.e. use an eventloop running on a single thread and use hiredis-clusters async-API. You can find examples here and here.
This setup gives you the parallell behaviour on a single thread (you can send commands via different contexts and then "wait" for responses in parallell).

Since hiredis-cluster uses hiredis, which is not stated to be threadsafe, its not supported. I believe it would work if you pin a context to a thread, and I believe there are users that run it like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants