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

Too many redis client connections by safe-transaction #1501

Closed
ausias-armesto opened this issue Jun 9, 2023 · 10 comments
Closed

Too many redis client connections by safe-transaction #1501

ausias-armesto opened this issue Jun 9, 2023 · 10 comments
Labels
bug Something isn't working

Comments

@ausias-armesto
Copy link

Description
A running instance of safe-transaction configured with a Gnosis chain and without users still using the service is consuming more than 400 redis connections. Even after the initial sync period with the Gnosis Chain

To Reproduce
Steps to reproduce the behavior:

  1. setup safe-transaction server with Gnosis
  2. Connect to the Redis server and count the opened connections
REDISCLI_AUTH=$REDIS_PASSWORD redis-cli CLIENT LIST | sed -n 's|.*addr=\(.*\)\:.*|\1|p' | sort | uniq -c

    434 10.2.89.158
      1 127.0.0.1

Expected behavior
I would expect to have a lower number of connections and be sure

Environment (please complete the following information):

  • Staging
  • Gnosis
  • Using a local installation of Safe Global (v4.18.6)

Additional context
There are other metrics which are quite strange regarding the usage of Redis. Here I show some metrics from Grafana

Screenshot 2023-06-09 at 08 52 08 Screenshot 2023-06-09 at 08 52 49
@ausias-armesto ausias-armesto added the bug Something isn't working label Jun 9, 2023
@Uxio0
Copy link
Member

Uxio0 commented Jun 20, 2023

Hi @ausias-armesto ,

We use a connection pool and don't limit the number of connections. Do you see any Redis issue, massive usage of CPU or memory?

@ausias-armesto
Copy link
Author

Hi @ausias-armesto ,

We use a connection pool and don't limit the number of connections. Do you see any Redis issue, massive usage of CPU or memory?

Could you give me some examples on where do you use the connection pool and how. I expected that the application itself would manage those issues and not at the infrastructure level. I don't see any exceptional CPU or memory consumption, but we have a prometheus alert firing constantly indicating a posible bad usage of redis connections

@Uxio0
Copy link
Member

Uxio0 commented Jun 29, 2023

@cache
def get_redis() -> Redis:
return Redis.from_url(settings.REDIS_URL)

It's cached, so we always reuse the same redis client

@ausias-armesto
Copy link
Author

@cache
def get_redis() -> Redis:
return Redis.from_url(settings.REDIS_URL)

It's cached, so we always reuse the same redis client

I already looked at the cached annotation before opening the bug, and wondered why there were still so many redis client connections if they are cached, and thought that perhaps it's not working (caching) as expected.

@Uxio0
Copy link
Member

Uxio0 commented Jun 29, 2023

I rechecked and I don't see any issue here. ConnectionPool must be used when calling from_url. But remember web service can run with some threads (1 redis pool for thread) and also every worker (indexer, contracts and notification) holds a redis connection

@ausias-armesto
Copy link
Author

ausias-armesto commented Jul 11, 2023

I @Uxio0 , I don't know how to handle this, as the metrics gathered from the monitoring are also quite significant that the cache might not be working.

Perhaps, it would be nice to know which is the expected number of redis connections to be opened by the service in a standard scenario.

Would you be able to run this command in your development, staging or production environment ?

REDISCLI_AUTH=$REDIS_PASSWORD redis-cli CLIENT LIST | sed -n 's|.*addr=\(.*\)\:.*|\1|p' | sort | uniq -c

@Uxio0
Copy link
Member

Uxio0 commented Jul 11, 2023

I see 381 connections opened in production mainnet

@ausias-armesto
Copy link
Author

Thanks @Uxio0 , It's nice to know that we have similar numbers on the metric. Then, I would say that it's time to consider if that value is considered correct. I applied this prometheus rule where it recommends not to have more than 90.
Should this be changed at the application level?

@Uxio0
Copy link
Member

Uxio0 commented Jul 12, 2023

Hi @ausias-armesto , I don't think is relevant that someone decided that 90 connections to redis are too much. Taking a look in the offical docs I don't see any kind of problem or recommendation: https://redis.io/docs/reference/clients/ . Also in our production environments we don't see any issues

@Uxio0 Uxio0 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 12, 2024
@Uxio0
Copy link
Member

Uxio0 commented Apr 12, 2024

@ausias-armesto thanks for the detailed reports on this issue, but we still didn't find any issue in our production deployments so we decided not to do anything about this

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
None yet
Development

No branches or pull requests

2 participants