Skip to content

[Bug]: Redis Connections Leak, especially with queue api calls. #1065

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

Closed
ikreymer opened this issue Aug 13, 2023 · 0 comments
Closed

[Bug]: Redis Connections Leak, especially with queue api calls. #1065

ikreymer opened this issue Aug 13, 2023 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@ikreymer
Copy link
Member

Browsertrix Cloud Version

v1.6.0

What did you expect to happen? What happened instead?

It appears that number of redis connections keeps growing as the crawl is running!

It appears that we are missing redis.close() calls for most operator and queue API calls.

Also, due to an issue with aioredis.from_url, connections are not being auto-closed even with redis.close().

Looks like this will be fixed in upcoming redis 5.0.0 release, but should be possible to add a workaround. See redis/redis-py#2831.

Step-by-step reproduction instructions

  1. exec pod into redis pod for a running crawl
  2. run redis-cli
  3. run info clients
  4. observe connected_clients number going up

Additional details

No response

@ikreymer ikreymer added the bug Something isn't working label Aug 13, 2023
@ikreymer ikreymer self-assigned this Aug 13, 2023
@ikreymer ikreymer moved this from Triage to Todo in Webrecorder Projects Aug 13, 2023
ikreymer added a commit that referenced this issue Aug 13, 2023
- use contextmanager for accessing redis to ensure redis.close() is always called
- add get_redis_client() to k8sapi to ensure unified place to get redis client
- use connectionpool.from_url() until redis 5.0.0 is released to ensure auto close and single client settings are applied
- also: catch invalid regex passed to re.compile() in queue regex check, return 400 instead of 500 for invalid regex
- redis requirements: bump to 5.0.0rc2
@ikreymer ikreymer moved this from Todo to PR In Review in Webrecorder Projects Aug 14, 2023
@github-project-automation github-project-automation bot moved this from PR In Review to Done! in Webrecorder Projects Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

1 participant