From 82829f4a03babf3d6b639d484cbe40ed0b4df1ad Mon Sep 17 00:00:00 2001 From: andy-paine Date: Thu, 6 May 2021 17:51:33 +0100 Subject: [PATCH] WIP: Swap Thin for Puma Still needs a lot of work: * Not feature flagged (probably should be) * Does not allow configuration of puma workers/threads * Periodic metrics are added on every worker but most are gauges about the individual thread (see vitals) so will clash and cause confusing numbers as each worker reports * Starting EM in a different thread means it _may_ not get shutdown properly but given it just does metrics now I'm not sure this really matters anymore? * Are we really benefitting from preload_app! - we don't really need fast startup time for workers as we probably won't cycle them and it means we have to disconnect the DB after loading the models. It might be simpler to just start the CC after forking which should hopefully mean the DB only connects after forking * Some config options like DB connection pool will need expressing/updating to be per-worker to avoid over-allocating --- Gemfile | 2 +- Gemfile.lock | 10 ++-- lib/cloud_controller.rb | 3 +- lib/cloud_controller/db.rb | 1 + lib/cloud_controller/runner.rb | 89 ++++++++++++---------------------- 5 files changed, 39 insertions(+), 66 deletions(-) diff --git a/Gemfile b/Gemfile index b5e001c8498..87b8e0ded96 100644 --- a/Gemfile +++ b/Gemfile @@ -39,7 +39,7 @@ gem 'sinatra-contrib' gem 'statsd-ruby', '~> 1.4.0' gem 'steno' gem 'talentbox-delayed_job_sequel', '~> 4.3.0' -gem 'thin' +gem 'puma' gem 'unf' gem 'vmstat', '~> 2.3' gem 'yajl-ruby' diff --git a/Gemfile.lock b/Gemfile.lock index b5ed1b50a1a..f9dc1d029ee 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -123,7 +123,6 @@ GEM crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.6) - daemons (1.2.6) debase (0.2.5.beta2) debase-ruby_core_source (>= 0.10.12) debase-ruby_core_source (0.10.12) @@ -306,6 +305,7 @@ GEM netaddr (2.0.4) netrc (0.11.0) newrelic_rpm (6.15.0) + nio4r (2.5.7) nokogiri (1.11.3) mini_portile2 (~> 2.5.0) racc (~> 1.4) @@ -334,6 +334,8 @@ GEM byebug (~> 11.0) pry (~> 0.13.0) public_suffix (4.0.6) + puma (5.2.2) + nio4r (~> 2.0) racc (1.5.2) rack (2.2.3) rack-protection (2.0.8.1) @@ -480,10 +482,6 @@ GEM delayed_job (~> 4.1.0) sequel (>= 3.38, < 6.0) tzinfo - thin (1.7.2) - daemons (~> 1.0, >= 1.0.9) - eventmachine (~> 1.0, >= 1.0.4) - rack (>= 1, < 3) thor (1.1.0) thread_safe (0.3.6) tilt (2.0.10) @@ -569,6 +567,7 @@ DEPENDENCIES protobuf (= 3.6.12) pry-byebug public_suffix + puma rack-test railties (~> 5.2.4, >= 5.2.4.3) rake @@ -596,7 +595,6 @@ DEPENDENCIES statsd-ruby (~> 1.4.0) steno talentbox-delayed_job_sequel (~> 4.3.0) - thin timecop unf vcap-concurrency! diff --git a/lib/cloud_controller.rb b/lib/cloud_controller.rb index 38eae27b5aa..e6a826b1692 100644 --- a/lib/cloud_controller.rb +++ b/lib/cloud_controller.rb @@ -1,8 +1,9 @@ require 'sinatra' require 'sequel' -require 'thin' require 'multi_json' require 'delayed_job' +require 'puma' +require 'puma/configuration' require 'allowy' diff --git a/lib/cloud_controller/db.rb b/lib/cloud_controller/db.rb index a58040902ce..28f50922230 100644 --- a/lib/cloud_controller/db.rb +++ b/lib/cloud_controller/db.rb @@ -54,6 +54,7 @@ def self.load_models(db_config, logger) require 'models' require 'delayed_job_sequel' + db end def self.load_models_without_migrations_check(db_config, logger) diff --git a/lib/cloud_controller/runner.rb b/lib/cloud_controller/runner.rb index 5443da6e029..87afcef3786 100644 --- a/lib/cloud_controller/runner.rb +++ b/lib/cloud_controller/runner.rb @@ -73,21 +73,16 @@ def parse_config(secrets_hash) def run! create_pidfile + start_cloud_controller - EM.run do - start_cloud_controller + request_metrics = VCAP::CloudController::Metrics::RequestMetrics.new(statsd_client) + builder = RackAppBuilder.new + app = builder.build(@config, request_metrics) - request_metrics = VCAP::CloudController::Metrics::RequestMetrics.new(statsd_client) - gather_periodic_metrics - - builder = RackAppBuilder.new - app = builder.build(@config, request_metrics) - - start_thin_server(app) - rescue => e - logger.error "Encountered error: #{e}\n#{e.backtrace.join("\n")}" - raise e - end + start_puma_server app + rescue => e + logger.error "Encountered error: #{e}\n#{e.backtrace.join("\n")}" + raise e end def gather_periodic_metrics @@ -95,37 +90,13 @@ def gather_periodic_metrics periodic_updater.setup_updates end - def trap_signals - %w(TERM INT QUIT).each do |signal| - trap(signal) do - EM.add_timer(0) do - logger.warn("Caught signal #{signal}") - stop! - end - end - end - - trap('USR1') do - EM.add_timer(0) do - logger.warn('Collecting diagnostics') - collect_diagnostics - end - end - end - - def stop! - stop_thin_server - logger.info('Stopping EventMachine') - EM.stop - end - private def start_cloud_controller setup_logging setup_telemetry_logging - setup_db setup_blobstore + @db = setup_db @config.configure_components setup_app_log_emitter @@ -186,28 +157,30 @@ def fluent_emitter )) end - def start_thin_server(app) - @thin_server = if @config.get(:nginx, :use_nginx) - Thin::Server.new(@config.get(:nginx, :instance_socket), signals: false) + def start_puma_server(app) + puma_config = Puma::Configuration.new do |user_config| + user_config.workers 3 + bind_address = if @config.get(:nginx, :use_nginx) + "tcp://127.0.0.1:#{@config.get(:nginx, :instance_socket)}" else - Thin::Server.new(@config.get(:external_host), @config.get(:external_port), signals: false) + "tcp://#{@config.get(:external_host)}:#{@config.get(:external_port)}" end - - @thin_server.app = app - trap_signals - - # The routers proxying to us handle killing inactive connections. - # Set an upper limit just to be safe. - @thin_server.timeout = @config.get(:request_timeout_in_seconds) - @thin_server.threaded = true - @thin_server.threadpool_size = @config.get(:threadpool_size) - logger.info("Starting thin server with #{EventMachine.threadpool_size} threads") - @thin_server.start! - end - - def stop_thin_server - logger.info('Stopping Thin Server.') - @thin_server.stop if @thin_server + user_config.bind bind_address + user_config.app app + user_config.preload_app! + user_config.before_fork { + @db.disconnect + } + user_config.after_worker_boot { + Thread.new do + EM.run do + gather_periodic_metrics + end + end + } + end + @puma_launcher = Puma::Launcher.new(puma_config) + @puma_launcher.run end def periodic_updater