Make puma default webserver (#485)

* Make Puma the default webserver

Thin can still be used using the new config flag `cc.temporary_enable_deprecated_thin_webserver`

* Change Puma defaults to 2 workers with each 10 threads and 10 DB connections

* Enable jemalloc by default and remove feature flag

* Re-add `cc.experimental.use_puma_webserver`

jobs/cc_deployment_updater/templates/bin/cc_deployment_updater.erb

@@ -7,8 +7,6 @@ cd /var/vcap/packages/cloud_controller_ng/cloud_controller_ng
 export RUBYOPT='--yjit'
 <% end %>
 
-<% if link("cloud_controller_internal").p('cc.experimental.use_jemalloc_memory_allocator') %>
-  export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
-<% end %>
+export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
 
 exec bundle exec rake deployment_updater:start

jobs/cloud_controller_clock/templates/bin/cloud_controller_clock.erb

@@ -7,8 +7,6 @@ cd /var/vcap/packages/cloud_controller_ng/cloud_controller_ng
 export RUBYOPT='--yjit'
 <% end %>
 
-<% if link("cloud_controller_internal").p('cc.experimental.use_jemalloc_memory_allocator') %>
 export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
-<% end %>
 
 exec bundle exec rake clock:start

jobs/cloud_controller_ng/spec

@@ -163,7 +163,6 @@ provides:
   - cc.external_port
   - cc.external_protocol
   - cc.experimental.use_yjit_compiler
-  - cc.experimental.use_jemalloc_memory_allocator
   - cc.internal_service_hostname
   - cc.jobs.enable_dynamic_job_priorities
   - cc.log_db_queries
@@ -229,6 +228,7 @@ provides:
   - cc.app_log_revision
   - cc.app_instance_stopping_state
   - cc.deprecated_stacks
+  - cc.temporary_enable_deprecated_thin_webserver
 
 consumes:
 - name: database
@@ -1253,28 +1253,21 @@ properties:
     description: "Use Ruby's YJIT compiler when running Cloud Controller API servers and workers. This feature is experimental and not recommended. Please review the drawbacks and benefits of YJIT before enabling."
     default: false
 
-  cc.experimental.use_jemalloc_memory_allocator:
-    description: "Enables jemalloc rather than malloc for Cloud Controller API servers and workers; however, it's experimental and not typically recommended, so review its pros and cons before use."
-    default: false
-
-  cc.experimental.use_puma_webserver:
-    description: "Use Puma in place of Thin as the webserver. This may increase performance as Puma forks Cloud Controller processes to avoid relying on threads"
-    default: false
-
   cc.experimental.use_redis:
     description: "Use co-deployed Valkey (Redis fork) for rate limiting and metrics. If the Puma webserver is enabled, Valkey will automatically be used."
     default: false
 
   cc.puma.workers:
     description: "Number of workers for Puma webserver."
-    default: 3
+    default: 2
 
   cc.puma.max_threads:
     description: "Maximum number of threads per Puma webserver worker."
-    default: 2
+    default: 10
 
   cc.puma.max_db_connections_per_process:
-    description: "Maximum database connections for Puma per process (main + Puma workers), if not set the ccng value is used (default)"
+    description: "Maximum database connections for Puma per process (main + Puma workers)."
+    default: 10
 
   cc.update_metric_tags_on_rename:
     description: "Enable sending a Desired LRP update when an app is renamed"
@@ -1283,3 +1276,13 @@ properties:
   cc.legacy_md5_buildpack_paths_enabled:
     description: "Enable legacy MD5 buildpack paths. If disabled, xxhash64 is used for calculating paths in buildpack image layers."
     default: false
+
+  cc.temporary_enable_deprecated_thin_webserver:
+    description: "Use deprecated Thin webserver. Please note that when using Thin instead of Puma you miss out on the following benefits: Better resource utilization, well maintained and more performant. Thin will be removed in a future release. `cc.experimental.use_puma_webserver` takes precedence over this."
+    default: false
+
+
+# deprecated configuration
+
+  cc.experimental.use_puma_webserver:
+    description: "Deprecated as Puma is now the default. This config flag will be removed in the future. Currently it takes precedence over `cc.temporary_enable_deprecated_thin_webserver` i.e. when set to false Thin will be used."

jobs/cloud_controller_ng/templates/bin/cloud_controller_ng.erb

@@ -18,9 +18,7 @@ echo 'Finished migrations and seeds'
 export RUBYOPT='--yjit'
 <% end %>
 
-<% if p('cc.experimental.use_jemalloc_memory_allocator') %>
 export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
-<% end %>
 
 exec /var/vcap/packages/cloud_controller_ng/cloud_controller_ng/bin/cloud_controller \
   -c /var/vcap/jobs/cloud_controller_ng/config/cloud_controller_ng.yml

jobs/cloud_controller_ng/templates/bin/local_worker.erb

@@ -9,9 +9,7 @@ wait_for_blobstore
 export RUBYOPT='--yjit'
 <% end %>
 
-<% if p('cc.experimental.use_jemalloc_memory_allocator') %>
 export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
-<% end %>
 
 cd /var/vcap/packages/cloud_controller_ng/cloud_controller_ng
 exec bundle exec rake "jobs:local[cc_api_worker.<%= spec.job.name %>.<%= spec.index %>.${INDEX}]"

jobs/cloud_controller_ng/templates/bpm.yml.erb

@@ -12,8 +12,16 @@ def mount_nfs_volume!(config)
   end
 end
 
+def thin_webserver_enabled?
+  if_p('cc.experimental.use_puma_webserver') do |prop|
+   return !prop
+  end
+
+  p('cc.temporary_enable_deprecated_thin_webserver')
+end
+
 def mount_valkey_volume!(config)
-  if p("cc.experimental.use_puma_webserver") || p("cc.experimental.use_redis")
+  unless thin_webserver_enabled? && !p("cc.experimental.use_redis")
     config['additional_volumes'] = [] unless config.key?('additional_volumes')
     config['additional_volumes'] << {
       "path" => "/var/vcap/data/valkey",

jobs/cloud_controller_ng/templates/cloud_controller_ng.yml.erb

@@ -42,6 +42,14 @@
     end
     raise "Error for database_encryption: #{active_keys.length} were marked active. Only one key may be active"
   end
+
+  def thin_webserver_enabled?
+  if_p('cc.experimental.use_puma_webserver') do |prop|
+   return !prop
+  end
+
+  p('cc.temporary_enable_deprecated_thin_webserver')
+end
 %>
 ---
 #Actually NGX host and port
@@ -53,7 +61,7 @@ local_route: <%= p('cc.nginx.ip') %>
 external_port: <%= p("cc.external_port") %>
 tls_port: <%= p("cc.tls_port") %>
 internal_service_hostname: <%= p("cc.internal_service_hostname") %>
-webserver: <%= p("cc.experimental.use_puma_webserver") ? 'puma' : 'thin' %>
+webserver: <%= thin_webserver_enabled? ? 'thin' : 'puma' %>
 puma:
   workers: <%= p("cc.puma.workers") %>
   max_threads: <%= p("cc.puma.max_threads") %>
@@ -490,7 +498,7 @@ rate_limiter_v2_api:
 
 temporary_enable_v2: <%= p("cc.temporary_enable_v2") %>
 
-<% if p("cc.experimental.use_puma_webserver") || p("cc.experimental.use_redis") %>
+<% unless thin_webserver_enabled? && !p("cc.experimental.use_redis") %>
 redis:
   socket: "/var/vcap/data/valkey/valkey.sock"
 <% end %>

jobs/cloud_controller_worker/templates/bin/cloud_controller_worker.erb

@@ -9,9 +9,7 @@ wait_for_blobstore
 export RUBYOPT='--yjit'
 <% end %>
 
-<% if link("cloud_controller_internal").p('cc.experimental.use_jemalloc_memory_allocator') %>
 export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
-<% end %>
 
 cd /var/vcap/packages/cloud_controller_ng/cloud_controller_ng
 

jobs/rotate_cc_database_key/templates/bin/run.erb

@@ -2,9 +2,7 @@
 
 set -eu
 
-<% if link("cloud_controller_internal").p('cc.experimental.use_jemalloc_memory_allocator') %>
-  export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
-<% end %>
+export LD_PRELOAD=/var/vcap/packages/jemalloc/lib/libjemalloc.so
 
 rotate() {
   export CLOUD_CONTROLLER_NG_CONFIG=/var/vcap/jobs/rotate_cc_database_key/config/cloud_controller_ng.yml

jobs/valkey/monit

@@ -1,6 +1,14 @@
 <%
+  def thin_webserver_enabled?(link)
+    link.if_p('cc.experimental.use_puma_webserver') do |prop|
+     return !prop
+    end
+
+    link.p('cc.temporary_enable_deprecated_thin_webserver')
+  end
+
   cloud_controller_internal = link("cloud_controller_internal")
-  if cloud_controller_internal.p("cc.experimental.use_puma_webserver") || cloud_controller_internal.p("cc.experimental.use_redis")
+  unless thin_webserver_enabled?(cloud_controller_internal) && !cloud_controller_internal.p("cc.experimental.use_redis")
 %>
 
 check process valkey

spec/cloud_controller_ng/bpm_spec.rb

@@ -112,33 +112,87 @@ def valkey_volume_mounted?(process)
             end
           end
 
-          context 'when the puma webserver is used' do
-            it 'mounts the valkey volume into the ccng job container' do
-              template_hash = YAML.safe_load(template.render({ 'cc' => { 'experimental' => { 'use_puma_webserver' => true } } }, consumes: {}))
-
-              results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
-              expect(results.length).to eq(1)
-              expect(valkey_volume_mounted?(results[0])).to be_truthy
+          describe 'valkey config' do
+            context 'when the puma webserver is used by default' do
+              it 'mounts the valkey volume into the ccng job container' do
+                template_hash = YAML.safe_load(template.render({}, consumes: {}))
+
+                results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
+                expect(results.length).to eq(1)
+                expect(valkey_volume_mounted?(results[0])).to be_truthy
+              end
             end
-          end
-
-          context "when 'cc.experimental.use_redis' is set to 'true'" do
-            it 'mounts the valkey volume into the ccng job container' do
-              template_hash = YAML.safe_load(template.render({ 'cc' => { 'experimental' => { 'use_redis' => true } } }, consumes: {}))
 
-              results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
-              expect(results.length).to eq(1)
-              expect(valkey_volume_mounted?(results[0])).to be_truthy
+            context 'when the puma webserver is enabled by deprecated config' do
+              it 'mounts the valkey volume into the ccng job container' do
+                template_hash = YAML.safe_load(template.render({ 'cc' => { 'experimental' => { 'use_puma_webserver' => true } } }, consumes: {}))
+
+                results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
+                expect(results.length).to eq(1)
+                expect(valkey_volume_mounted?(results[0])).to be_truthy
+              end
+
+              context 'when `cc.temporary_enable_deprecated_thin_webserver` is also enabled' do
+                it 'still uses Puma and mounts the valkey volume into the ccng job container' do
+                  template_hash = YAML.safe_load(template.render(
+                                                   { 'cc' => { 'experimental' => { 'use_puma_webserver' => true },
+                                                               'temporary_enable_deprecated_thin_webserver' => true } }, consumes: {}
+                                                 ))
+
+                  results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
+                  expect(results.length).to eq(1)
+                  expect(valkey_volume_mounted?(results[0])).to be_truthy
+                end
+              end
             end
-          end
 
-          context "when neither the puma webserver is used nor 'cc.experimental.use_redis' is set to 'true'" do
-            it 'does not mount the valkey volume into the ccng job container' do
-              template_hash = YAML.safe_load(template.render({}, consumes: {}))
+            context 'when thin webserver is explicitly enabled' do
+              context "when 'cc.experimental.use_redis' is set to 'true'" do
+                it 'mounts the valkey volume into the ccng job container' do
+                  template_hash = YAML.safe_load(
+                    template.render({ 'cc' => { 'temporary_enable_deprecated_thin_webserver' => true,
+                                                'experimental' => { 'use_redis' => true } } }, consumes: {})
+                  )
+
+                  results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
+                  expect(results.length).to eq(1)
+                  expect(valkey_volume_mounted?(results[0])).to be_truthy
+                end
+              end
+
+              context "when 'cc.experimental.use_redis' is not set'" do
+                it 'mounts the valkey volume into the ccng job container' do
+                  template_hash = YAML.safe_load(template.render({ 'cc' => { 'temporary_enable_deprecated_thin_webserver' => true } }, consumes: {}))
+
+                  results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
+                  expect(results.length).to eq(1)
+                  expect(valkey_volume_mounted?(results[0])).to be_falsey
+                end
+              end
+            end
 
-              results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
-              expect(results.length).to eq(1)
-              expect(valkey_volume_mounted?(results[0])).to be_falsey
+            context 'when thin webserver is implicitly enabled through `cc.experimental.use_puma_webserver` => false' do
+              context "when 'cc.experimental.use_redis' is set to 'true'" do
+                it 'mounts the valkey volume into the ccng job container' do
+                  template_hash = YAML.safe_load(
+                    template.render({ 'cc' => { 'experimental' => { 'use_puma_webserver' => false, 'use_redis' => true } } }, consumes: {})
+                  )
+
+                  results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
+                  expect(results.length).to eq(1)
+                  expect(valkey_volume_mounted?(results[0])).to be_truthy
+                end
+              end
+
+              context "when 'cc.experimental.use_redis' is not set'" do
+                it 'mounts the valkey volume into the ccng job container' do
+                  template_hash = YAML.safe_load(template.render({ 'cc' => { 'temporary_enable_deprecated_thin_webserver' => true } }, consumes: {}))
+
+                  results = template_hash['processes'].select { |p| p['name'].include?('cloud_controller_ng') }
+                  expect(results.length).to eq(1)
+                  expect(valkey_volume_mounted?(results[0])).to be_falsey
+                end
+              end
             end
           end
         end