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

Missing pipeline support for async ClusterClient #2186

Closed
karwinliu opened this issue May 17, 2022 · 7 comments · Fixed by #2199
Closed

Missing pipeline support for async ClusterClient #2186

karwinliu opened this issue May 17, 2022 · 7 comments · Fixed by #2199

Comments

@karwinliu
Copy link

Version: redis-py (4.3.1) redis server (6.0)

Platform: Python 3.9 on macOS Apple silicon

Description: I don't see the support of pipeline for the async cluster client. Besides, I don't find any documentation on async version of the cluster client, so maybe it's already implemented and can be invoked in some special ways. If not, I wonder if there's already a plan to support that and if there's any workaround for the time being.

@karwinliu
Copy link
Author

Oh I thought this is not for the clustered version, which would include mapping commands within a pipeline to different nodes/shards, etc.

@grippy
Copy link
Contributor

grippy commented May 20, 2022

@karwinliu You're right, I was looking at the async standalone client. Sorry, it looks like the async RedisCluster client inherits the pipeline method as part of the AbstractRedisCluster base class but its not async: https://github.com/Flipboard/redis-py/blob/29f97e9905637499343367888f1d66f76d9cc25c/redis/asyncio/cluster.py#L62-L63

@karwinliu
Copy link
Author

No worries, would the pipeline be added soon? I think @utkarshgupta137 might already be working on it. I guess I'll just wait until it is ready

@utkarshgupta137
Copy link
Contributor

No worries, would the pipeline be added soon? I think @utkarshgupta137 might already be working on it. I guess I'll just wait until it is ready

I've got it working but I've only done basic testing. I will raise a PR once I write tests for it. If you want, you can check it out here: https://github.com/utkarshgupta137/redis-py/tree/async_cluster_pipeline

@karwinliu
Copy link
Author

Thanks @utkarshgupta137 for all the implementations. We've started to fully evaluate the async cluster, and it's been working well so far. But we do found some issues after using it extensively, and I have just submitted the issues. Would you mind taking a look? We have applied some simple patches to fix the issue locally, but ideally this could be fixed in the package soon.

  1. Support SSL in async Redis cluster client #2303
  2. Race condition bug in async Redis cluster pipeline #2304
  3. Wrong inheritance of the delete API with async Redis cluster pipeline #2305

@utkarshgupta137
Copy link
Contributor

Hey, I've already raised a PR for the first 2 issues, one of which is merged & other should be merged before the next release. For 3rd, I'll have to check if it is possible to use the same command for slots across keys.

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

Successfully merging a pull request may close this issue.

3 participants