Is there a way to change the redis connection timeout?

AI-generated summary

The discussion revolves around changing the Redis connection timeout in Discourse. Shane_Liebling wants to increase the timeout to resolve issues with a remote Redis instance and disable CLIENT operations.

Falco initially suggests modifying the Redis configuration, but Shane_Liebling clarifies that the change is needed on the client side. Falco then suggests creating a pull request (PR) to add a setting for the Redis timeout.

Additionally, Shane_Liebling mentions the need to disable CLIENT operations by setting “id = nil” due to restrictions in some Redis setups, specifically with Google Cloud’s Memorystore.

Falco agrees that a PR to allow setting custom Redis client ID and timeout is a good idea. However, Shane_Liebling is unable to submit the PR due to release form restrictions.

Justin_Domingus offers to submit the PR, and Shane_Liebling declines co-authorship. The PR is submitted by Justin_Domingus and is later merged into core, as mentioned by sam.

I keep getting timeouts from redis/hiredis and would like to increase the timeout to see if it fixes the issue. (I am connecting to a remote redis instance, not on the same instance as Discourse.)

Additionally I need to turn off CLIENT operations as well. Afaict both of these would be done in a redis.config{} stanza but I’d prefer not to have to go in and patch the code.

You can config redis using the app.yml section Beginning of custom commands with something like

  - replace:
      filename: "/etc/redis/redis.conf"
      from: "daemonize yes"
      to: ""

I’m sure I am misunderstanding something here, but when I see /etc/… I assume that is happening on the vm running the redis server. Am I wrong in that reading?

I am trying to change how the Discourse rails app communicates with the remote redis instance so that it (the client side) will make those calls with a longer timeout.

Sorry if I am misunderstanding your response.

Oh I see now. You want to pass a new parameters here:

Sounds like an acceptable PR for me, that adds a setting to set c[:timeout] = redis_timeout if redis_timeout.

However, we have seen some people suffering for bad page timings when redis has a long latency to the webserver, keep that in mind.

Would setting no CLIENT ops here too be viable? (eg “id = nil”)

I’m not familiar with restricting the CLIENT ops, can you share more about your use case and why is this necessary?

Some Redis setups (specifically cloud providers) do not allow the CLIENT operation. You can prevent this from occurring by setting “id = nil” in the redis configuration. This is an issue I have run into with sidekiq.

Other apps that use sidekiq have noted this as an issue:
https://github.com/mperham/sidekiq/issues/3518
https://github.com/tootsuite/mastodon/issues/7785

I don’t think we ever hit this in AWS Elasticache offering, and also didn’t hear anything from colleagues working with Azure. What is the cloud provider that is failing for you?

Running against Google Cloud’s Memorystore.

Oh that is interesting to know.

A PR to allow setting custom REDIS_CLIENT_ID and REDIS_TIMEOUT, with current values as defaults sounds a good idea.

So if you look at https://github.com/discourse/discourse/compare/master...shanel:master you’ll see the necessary edit for the client id. (Turns out that is the only one I needed.)

Unfortunately I am currently not allowed by my job to sign your release form. I could try to get my SVP to okay it, but I would bet it would take a month or two. :frowning:

Would you be willing/able to apply the patch?

We have this same issue when trying to use Google Memorystore for redis. If you like, I can make the PR from my account. Would you like me to add you as a co-author or would that complicate the release form for you @Shane_Liebling? Here is the diff: https://github.com/discourse/discourse/compare/master...domingusj:master

If that looks correct I can submit the PR.

No co authorship please. Thanks!

Cool, all done: Adding a setting for redis client id by domingusj · Pull Request #6837 · discourse/discourse · GitHub

This is tricky, I was going to force this into a plugin cause it is so rare, but we can not hook early enough.

Feature is added to core per:

https://review.discourse.org/t/feature-add-setting-to-bypass-sending-redis-client-commands/966