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

replace asyncio-redis with redis-py #867

Merged
merged 27 commits into from
Feb 4, 2023
Merged

replace asyncio-redis with redis-py #867

merged 27 commits into from
Feb 4, 2023

Conversation

chdsbd
Copy link
Owner

@chdsbd chdsbd commented Feb 4, 2023

Asyncio-redis is pretty terrible in adverse network conditions. Digital Ocean network connectivity can flake out and the lack of timeouts within asyncio-redis make it easy for the library to cause hangs, which will break Kodiak for users. redis-py's async version (which originally was aioredis) has excellent timeouts for every operation, making it a safer choice. It also has a reasonable connection pool setup where we don't need to have a fixed pool size.

The default timeouts for redis-py async are infinite, which isn't great, but we explicitly set short timeouts.

Hopefully, now we shouldn't see any more hangs, which has been a problem for us for many months.

@chdsbd chdsbd requested a review from sbdchd February 4, 2023 19:45
@chdsbd chdsbd added the automerge Mark PR for auto merge by Kodiak label Feb 4, 2023
@chdsbd chdsbd marked this pull request as ready for review February 4, 2023 19:45
bot/kodiak/app_config.py Outdated Show resolved Hide resolved
bot/pyproject.toml Outdated Show resolved Hide resolved
bot/kodiak/redis_client.py Outdated Show resolved Hide resolved
bot/kodiak/app_config.py Outdated Show resolved Hide resolved
Co-authored-by: Steve Dignam <steve@dignam.xyz>
@kodiakhq kodiakhq bot merged commit e00e716 into master Feb 4, 2023
@kodiakhq kodiakhq bot deleted the chris/redis-py branch February 4, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Mark PR for auto merge by Kodiak
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants