Merge branch 'sh-ignore-search-path-pgbouncer' into 'master'

Add PgBouncer search_path as default ignored startup parameter

See merge request https://gitlab.com/gitlab-org/omnibus-gitlab/-/merge_requests/7566

Merged-by: Stan Hu <stanhu@gmail.com>

doc/settings/memory_constrained_envs.md

@@ -145,14 +145,14 @@ We observed 100-400MB of memory usage reduction configuring Puma this way.
 ## Optimize Sidekiq
 
 Sidekiq is a background processing daemon. When configured with GitLab by default
-it runs with a high concurrency mode of `50`. This does impact how much memory it can
+it runs with a concurrency mode of `20`. This does impact how much memory it can
 allocate at a given time. It is advised to configure it to use a significantly
 smaller value of `5` or `10` (preferred).
 
 In `/etc/gitlab/gitlab.rb`:
 
 ```ruby
-sidekiq['max_concurrency'] = 10
+sidekiq['concurrency'] = 10
 ```
 
 ## Optimize Gitaly
@@ -249,7 +249,7 @@ and disable the Prometheus Metrics feature:
    ```ruby
    puma['worker_processes'] = 0
 
-   sidekiq['max_concurrency'] = 10
+   sidekiq['concurrency'] = 10
 
    prometheus_monitoring['enable'] = false
 

doc/settings/rpi.md

@@ -55,7 +55,7 @@ some running processes:
    ```ruby
    # Reduce the number of running workers to the minimum in order to reduce memory usage
    puma['worker_processes'] = 2
-   sidekiq['max_concurrency'] = 9
+   sidekiq['concurrency'] = 9
    # Turn off monitoring to reduce idle cpu and disk usage
    prometheus_monitoring['enable'] = false
    ```

files/gitlab-config-template/gitlab.rb.template

@@ -1280,9 +1280,7 @@ external_url 'GENERATED_EXTERNAL_URL'
 # sidekiq['log_format'] = "json"
 # sidekiq['shutdown_timeout'] = 4
 # sidekiq['interval'] = nil
-# sidekiq['concurrency'] = nil
-# sidekiq['max_concurrency'] = 20
-# sidekiq['min_concurrency'] = nil
+# sidekiq['concurrency'] = 20
 
 ##! GitLab allows route a job to a particular queue determined by an array of ##! routing rules.
 ##! Each routing rule is a tuple of queue selector query and corresponding queue. By default,
@@ -3131,7 +3129,7 @@ external_url 'GENERATED_EXTERNAL_URL'
 # pgbouncer['dns_nxdomain_ttl'] = '15.0'
 # pgbouncer['admin_users'] = %w(gitlab-psql postgres pgbouncer)
 # pgbouncer['stats_users'] = %w(gitlab-psql postgres pgbouncer)
-# pgbouncer['ignore_startup_parameters'] = 'extra_float_digits'
+# pgbouncer['ignore_startup_parameters'] = %w(extra_float_digits search_path)
 # pgbouncer['track_extra_parameters'] = %w(IntervalStyle)
 # pgbouncer['databases'] = {
 #   DATABASE_NAME: {

files/gitlab-cookbooks/gitlab/attributes/default.rb

@@ -728,9 +728,7 @@ default['gitlab']['sidekiq']['health_checks_listen_port'] = 8092
 
 # Cluster specific settings
 default['gitlab']['sidekiq']['interval'] = nil
-default['gitlab']['sidekiq']['concurrency'] = nil
-default['gitlab']['sidekiq']['max_concurrency'] = 20
-default['gitlab']['sidekiq']['min_concurrency'] = nil
+default['gitlab']['sidekiq']['concurrency'] = 20
 default['gitlab']['sidekiq']['queue_groups'] = ['*']
 default['gitlab']['sidekiq']['consul_service_name'] = 'sidekiq'
 default['gitlab']['sidekiq']['consul_service_meta'] = nil

files/gitlab-cookbooks/gitlab/templates/default/sv-sidekiq-run.erb

@@ -22,12 +22,6 @@ exec chpst -e /opt/gitlab/etc/gitlab-rails/env -P \
       <% if node['gitlab']['sidekiq']['concurrency'] %>
       -c <%= node['gitlab']['sidekiq']['concurrency'] %> \
       <% end %>
-      <% if node['gitlab']['sidekiq']['max_concurrency'] %>
-      -m <%= node['gitlab']['sidekiq']['max_concurrency'] %> \
-      <% end %>
-      <% if node['gitlab']['sidekiq']['min_concurrency'] %>
-      --min-concurrency <%= node['gitlab']['sidekiq']['min_concurrency'] %> \
-      <% end %>
       <% if node['gitlab']['sidekiq']['shutdown_timeout'] %>
       --timeout <%= node['gitlab']['sidekiq']['shutdown_timeout'] %> \
       <% end %>

files/gitlab-cookbooks/package/libraries/deprecations.rb

@@ -346,14 +346,14 @@ module Gitlab
           },
           {
             config_keys: %w(gitlab sidekiq min_concurrency),
-            deprecation: '16.9',
-            removal: '17.0',
+            deprecation: '16.9', # Remove message issue: https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/8491
+            removal: '17.0', # Removal issue: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/3422
             note: "Starting with GitLab 17.0, `sidekiq['min_concurrency']` will be removed. Please follow https://docs.gitlab.com/ee/administration/sidekiq/extra_sidekiq_processes.html#manage-thread-counts-explicitly to use `sidekiq['concurrency']` instead."
           },
           {
             config_keys: %w(gitlab sidekiq max_concurrency),
-            deprecation: '16.9',
-            removal: '17.0',
+            deprecation: '16.9', # Remove message issue: https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/8491
+            removal: '17.0', # Removal issue: https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/3422
             note: "Starting with GitLab 17.0, `sidekiq['max_concurrency']` will be removed. Please follow https://docs.gitlab.com/ee/administration/sidekiq/extra_sidekiq_processes.html#manage-thread-counts-explicitly to use `sidekiq['concurrency']` instead."
           },
           {

files/gitlab-cookbooks/pgbouncer/attributes/default.rb

@@ -24,7 +24,7 @@ default['pgbouncer']['dns_zone_check_period'] = 0
 default['pgbouncer']['dns_nxdomain_ttl'] = '15.0'
 default['pgbouncer']['admin_users'] = %w(gitlab-psql postgres pgbouncer)
 default['pgbouncer']['stats_users'] = %w(gitlab-psql postgres pgbouncer)
-default['pgbouncer']['ignore_startup_parameters'] = 'extra_float_digits'
+default['pgbouncer']['ignore_startup_parameters'] = %w(extra_float_digits search_path)
 default['pgbouncer']['track_extra_parameters'] = %w(IntervalStyle)
 default['pgbouncer']['databases_ini'] = '/var/opt/gitlab/pgbouncer/databases.ini'
 default['pgbouncer']['databases_ini_user'] = 'root'

files/gitlab-cookbooks/pgbouncer/libraries/pgbouncer_helper.rb

@@ -11,6 +11,24 @@ class PgbouncerHelper < BaseHelper
     end.join(' ').chomp
   end
 
+  def pgbouncer_ini_config
+    config = node['pgbouncer'].to_hash
+    ignore_startup_params = config['ignore_startup_parameters']
+
+    ignore_startup_params =
+      case ignore_startup_params
+      when String
+        ignore_startup_params.split(',').map(&:strip)
+      when Array
+        ignore_startup_params
+      else
+        ""
+      end
+
+    config['ignore_startup_parameters'] = ignore_startup_params.join(", ")
+    config
+  end
+
   def pgbouncer_admin_config
     user = node['postgresql']['pgbouncer_user']
     port = node['pgbouncer']['listen_port']

files/gitlab-cookbooks/pgbouncer/recipes/enable.rb

@@ -70,7 +70,7 @@ end
 
 template "#{node['pgbouncer']['data_directory']}/pgbouncer.ini" do
   source "#{File.basename(name)}.erb"
-  variables lazy { node['pgbouncer'].to_hash }
+  variables lazy { pgb_helper.pgbouncer_ini_config }
   owner account_helper.postgresql_user
   group account_helper.postgresql_group
   mode '0600'

spec/chef/cookbooks/gitlab/recipes/sidekiq_spec.rb

@@ -26,7 +26,7 @@ RSpec.describe 'gitlab::sidekiq' do
           expect(content).to match(/rubyopt=\"-W:no-experimental\"/)
           expect(content).to include(%(RUBYOPT="${rubyopt}"))
           expect(content).to match(%r{bin/sidekiq-cluster})
-          expect(content).to match(/-m 20/) # max_concurrency
+          expect(content).to match(/-c 20/) # concurrency
           expect(content).to match(/--timeout 25/) # shutdown timeout
           expect(content).to match(/\*/) # all queues
         }
@@ -64,11 +64,19 @@ RSpec.describe 'gitlab::sidekiq' do
       before do
         stub_gitlab_rb(
           sidekiq: {
-            log_group: 'fugee'
+            log_group: 'fugee',
+            concurrency: 42
           }
         )
       end
       it_behaves_like 'enabled logged service', 'sidekiq', true, { log_directory_owner: 'git', log_group: 'fugee' }
+
+      it 'correctly renders out the sidekiq service file' do
+        expect(chef_run).to render_file("/opt/gitlab/sv/sidekiq/run")
+          .with_content { |content|
+            expect(content).to match(/-c 42/) # concurrency
+          }
+      end
     end
   end
 

spec/chef/cookbooks/pgbouncer/libraries/pgbouncer_helper_spec.rb

@@ -42,6 +42,60 @@ RSpec.describe PgbouncerHelper do
     end
   end
 
+  describe '#pgbouncer_ini_config' do
+    let(:ini_config) { subject.pgbouncer_ini_config }
+
+    context 'by default' do
+      it 'returns default values' do
+        expect(ini_config).to be_a(Hash)
+        expect(ini_config['listen_addr']).to eq('0.0.0.0')
+        expect(ini_config['listen_port']).to eq(6432)
+        expect(ini_config['data_directory']).to eq('/var/opt/gitlab/pgbouncer')
+        expect(ini_config['ignore_startup_parameters']).to eq('extra_float_digits, search_path')
+      end
+    end
+
+    context 'with custom sttings' do
+      before do
+        stub_gitlab_rb(
+          pgbouncer: {
+            listen_port: 1234,
+            data_directory: '/tmp',
+            ignore_startup_parameters: %w[extra_float_digits]
+          }
+        )
+      end
+
+      it 'returns custom values' do
+        expect(ini_config).to be_a(Hash)
+        expect(ini_config['listen_addr']).to eq('0.0.0.0')
+        expect(ini_config['listen_port']).to eq(1234)
+        expect(ini_config['data_directory']).to eq('/tmp')
+        expect(ini_config['ignore_startup_parameters']).to eq('extra_float_digits')
+      end
+    end
+
+    context 'with ignore_startup_parameters separated by commas' do
+      before do
+        stub_gitlab_rb(
+          pgbouncer: {
+            listen_port: 1234,
+            data_directory: '/tmp',
+            ignore_startup_parameters: "search_path, extra_float_digits"
+          }
+        )
+      end
+
+      it 'returns custom values' do
+        expect(ini_config).to be_a(Hash)
+        expect(ini_config['listen_addr']).to eq('0.0.0.0')
+        expect(ini_config['listen_port']).to eq(1234)
+        expect(ini_config['data_directory']).to eq('/tmp')
+        expect(ini_config['ignore_startup_parameters']).to eq('search_path, extra_float_digits')
+      end
+    end
+  end
+
   describe '#pgbouncer_admin_config' do
     context 'by default' do
       it 'by default' do

spec/chef/cookbooks/pgbouncer/recipes/pgbouncer_spec.rb

@@ -90,7 +90,7 @@ RSpec.describe 'pgbouncer' do
         expect(content).to match(%r{^auth_file = /var/opt/gitlab/pgbouncer/pg_auth$})
         expect(content).to match(/^admin_users = gitlab-psql, postgres, pgbouncer$/)
         expect(content).to match(/^stats_users = gitlab-psql, postgres, pgbouncer$/)
-        expect(content).to match(/^ignore_startup_parameters = extra_float_digits$/)
+        expect(content).to match(/^ignore_startup_parameters = extra_float_digits, search_path$/)
         expect(content).to match(/^track_extra_parameters = IntervalStyle$/)
         expect(content).to match(%r{^unix_socket_dir = /var/opt/gitlab/pgbouncer$})
         expect(content).to match(%r{^%include /var/opt/gitlab/pgbouncer/databases.ini})