# HG changeset patch
# User Stan Hu <stanhu@gmail.com>
# Date 1719714849 25200
#      Sat Jun 29 19:34:09 2024 -0700
# Node ID ab5ec89a6381cd873d9826c9e8cce9d7ba88a5fd
# Parent  7244146d42bb12026ffc00fa9acaf8d6d57b4fbc
Add support for configuring Redis client timeouts

For some installations, the default 1-second Redis connect timeout may
not be sufficient. Just as PostgreSQL has the ability to configure the
connection timeout, add the same for Redis. Include read and write
timeout options for completness.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/466271

Changelog: added

diff --git a/doc/settings/redis.md b/doc/settings/redis.md
--- a/doc/settings/redis.md
+++ b/doc/settings/redis.md
@@ -232,6 +232,18 @@
 redis['io_threads_do_reads'] = true
 ```
 
+### Client Timeouts
+
+By default, the [Ruby client for Redis](https://github.com/redis-rb/redis-client?tab=readme-ov-file#configuration)
+uses a 1-second default for the connect, read, and write timeouts. You may need to tune these values to account for local network latency.
+For example, if you see `Connection timed out - user specified timeout` errors, you may need to raise `connect_timeout`:
+
+```ruby
+gitlab_rails['redis_connect_timeout'] = 3
+gitlab_rails['redis_read_timeout'] = 1
+gitlab_rails['redis_write_timeout'] = 1
+```
+
 ## Provide sensitive configuration to Redis clients without plain text storage
 
 For more information, see the example in [configuration documentation](../settings/configuration.md#provide-redis-password-to-redis-server-and-client-components).
diff --git a/files/gitlab-config-template/gitlab.rb.template b/files/gitlab-config-template/gitlab.rb.template
--- a/files/gitlab-config-template/gitlab.rb.template
+++ b/files/gitlab-config-template/gitlab.rb.template
@@ -810,6 +810,9 @@
 # gitlab_rails['redis_tls_ca_cert_file'] = '/opt/gitlab/embedded/ssl/certs/cacert.pem'
 # gitlab_rails['redis_tls_client_cert_file'] = nil
 # gitlab_rails['redis_tls_client_key_file'] = nil
+# gitlab_rails['redis_connect_timeout'] = nil
+# gitlab_rails['redis_read_timeout'] = nil
+# gitlab_rails['redis_write_timeout'] = nil
 
 #### Redis local UNIX socket (will be disabled if TCP method is used)
 # gitlab_rails['redis_socket'] = "/var/opt/gitlab/redis/redis.socket"
diff --git a/files/gitlab-cookbooks/gitlab/attributes/default.rb b/files/gitlab-cookbooks/gitlab/attributes/default.rb
--- a/files/gitlab-cookbooks/gitlab/attributes/default.rb
+++ b/files/gitlab-cookbooks/gitlab/attributes/default.rb
@@ -446,6 +446,9 @@
 default['gitlab']['gitlab_rails']['redis_host'] = "127.0.0.1"
 default['gitlab']['gitlab_rails']['redis_port'] = nil
 default['gitlab']['gitlab_rails']['redis_ssl'] = false
+default['gitlab']['gitlab_rails']['redis_connect_timeout'] = nil
+default['gitlab']['gitlab_rails']['redis_read_timeout'] = nil
+default['gitlab']['gitlab_rails']['redis_write_timeout'] = nil
 default['gitlab']['gitlab_rails']['redis_tls_ca_cert_dir'] = "#{node['package']['install-dir']}/embedded/ssl/certs/"
 default['gitlab']['gitlab_rails']['redis_tls_ca_cert_file'] = "#{node['package']['install-dir']}/embedded/ssl/certs/cacert.pem"
 default['gitlab']['gitlab_rails']['redis_tls_client_cert_file'] = nil
diff --git a/files/gitlab-cookbooks/gitlab/recipes/gitlab-rails.rb b/files/gitlab-cookbooks/gitlab/recipes/gitlab-rails.rb
--- a/files/gitlab-cookbooks/gitlab/recipes/gitlab-rails.rb
+++ b/files/gitlab-cookbooks/gitlab/recipes/gitlab-rails.rb
@@ -200,6 +200,9 @@
 redis_tls_client_key_file = node['gitlab']['gitlab_rails']['redis_tls_client_key_file']
 redis_encrypted_settings_file = node['gitlab']['gitlab_rails']['redis_encrypted_settings_file']
 redis_extra_config_command = node['gitlab']['gitlab_rails']['redis_extra_config_command']
+redis_connect_timeout = node['gitlab']['gitlab_rails']['redis_connect_timeout']
+redis_read_timeout = node['gitlab']['gitlab_rails']['redis_read_timeout']
+redis_write_timeout = node['gitlab']['gitlab_rails']['redis_write_timeout']
 
 templatesymlink "Create a secrets.yml and create a symlink to Rails root" do
   link_from File.join(gitlab_rails_source_dir, "config/secrets.yml")
@@ -238,7 +241,10 @@
     redis_tls_client_cert_file: redis_tls_client_cert_file,
     redis_tls_client_key_file: redis_tls_client_key_file,
     redis_encrypted_settings_file: redis_encrypted_settings_file,
-    redis_extra_config_command: redis_extra_config_command
+    redis_extra_config_command: redis_extra_config_command,
+    redis_connect_timeout: redis_connect_timeout,
+    redis_read_timeout: redis_read_timeout,
+    redis_write_timeout: redis_write_timeout
   )
   dependent_services.each { |svc| notifies :restart, svc }
   sensitive true
diff --git a/files/gitlab-cookbooks/gitlab/templates/default/resque.yml.erb b/files/gitlab-cookbooks/gitlab/templates/default/resque.yml.erb
--- a/files/gitlab-cookbooks/gitlab/templates/default/resque.yml.erb
+++ b/files/gitlab-cookbooks/gitlab/templates/default/resque.yml.erb
@@ -5,6 +5,15 @@
   <% if !@redis_enable_client %>
   id:
   <% end %>
+  <% if @redis_connect_timeout %>
+  connect_timeout: <%= @redis_connect_timeout %>
+  <% end %>
+  <% if @redis_read_timeout %>
+  read_timeout: <%= @redis_read_timeout %>
+  <% end %>
+  <% if @redis_write_timeout %>
+  write_timeout: <%= @redis_write_timeout %>
+  <% end %>
   <% if defined?(@cluster_nodes) && !@cluster_nodes.empty? %>
   <%- if @cluster_username %>username: <%= @cluster_username %><% end %>
   <%- if @cluster_password %>password: <%= @cluster_password %><% end %>
diff --git a/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb b/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb
--- a/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb
+++ b/spec/chef/cookbooks/gitlab/recipes/gitlab-rails_spec.rb
@@ -229,6 +229,9 @@
         expect(chef_run).to render_file(config_file).with_content { |content|
           expect(content).to match(%r(url: unix:///var/opt/gitlab/redis/redis.socket$))
           expect(content).not_to match(/id:/)
+          expect(content).not_to match(/connect_timeout:/)
+          expect(content).not_to match(/read_timeout:/)
+          expect(content).not_to match(/write_timeout:/)
         }
       end
 
@@ -243,6 +246,9 @@
 
         expect(chef_run).to render_file('/var/opt/gitlab/gitlab-rails/etc/cable.yml').with_content { |content|
           expect(content).to match(%r(url: unix:///var/opt/gitlab/redis/redis.socket$))
+          expect(content).not_to match(/connect_timeout:/)
+          expect(content).not_to match(/read_timeout:/)
+          expect(content).not_to match(/write_timeout:/)
         }
       end
 
@@ -268,23 +274,32 @@
             redis_port: 8888,
             redis_database: 2,
             redis_password: 'my pass',
-            redis_enable_client: false
+            redis_enable_client: false,
+            redis_connect_timeout: 3,
+            redis_read_timeout: 4,
+            redis_write_timeout: 5
           }
         )
       end
 
-      it 'creates the config file with custom host, port, password and database' do
+      it 'creates the config file with custom host, port, password, database, and timeouts' do
         expect(chef_run).to create_templatesymlink('Create a resque.yml and create a symlink to Rails root').with_variables(
           hash_including(
             redis_url: URI('redis://:my%20pass@redis.example.com:8888/2'),
             redis_sentinels: [],
-            redis_enable_client: false
+            redis_enable_client: false,
+            redis_connect_timeout: 3,
+            redis_read_timeout: 4,
+            redis_write_timeout: 5
           )
         )
 
         expect(chef_run).to render_file(config_file).with_content { |content|
           expect(content).to match(%r(url: redis://:my%20pass@redis.example.com:8888/2))
           expect(content).to match(/id:$/)
+          expect(content).to match(/connect_timeout: 3/)
+          expect(content).to match(/read_timeout: 4/)
+          expect(content).to match(/write_timeout: 5/)
         }
       end