diff --git a/config/database.yml b/config/database.yml index 2f8472a..845eb70 100644 --- a/config/database.yml +++ b/config/database.yml @@ -9,8 +9,9 @@ default: &default pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 10 } %> timeout: 5000 default_transaction_mode: immediate - # Checkpoint on a background connection instead (SqliteWalCheckpoint). Every - # non-test writer process runs a contender so this is never left without one. + # Checkpoint on a background connection instead (SqliteWalCheckpoint). + # Non-test processes start a contender; forking servers stop before fork and + # start again in the child so the flock is never inherited. pragmas: wal_autocheckpoint: 0 diff --git a/config/initializers/sqlite_wal_checkpoint.rb b/config/initializers/sqlite_wal_checkpoint.rb index ccf901a..3e72802 100644 --- a/config/initializers/sqlite_wal_checkpoint.rb +++ b/config/initializers/sqlite_wal_checkpoint.rb @@ -1,10 +1,5 @@ -# Start a checkpoint contender in console, runner, rake and other non-Puma writers. -# Puma skips this path: config/puma.rb and config/puma_dev.rb start after the -# correct process is chosen (worker boot vs single-process), so the master that -# only forks workers never holds the lock alone. Resque starts after_prefork. +# Start a checkpoint contender in every non-test process. Puma and Resque pool +# stop before fork and start again in the child so the flock is never inherited. Rails.application.config.after_initialize do - next if Rails.env.test? - next if defined?(Puma::CLI) - - SqliteWalCheckpoint.start + SqliteWalCheckpoint.start unless Rails.env.test? end diff --git a/config/puma.rb b/config/puma.rb index 6e1c7fc..e626470 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -33,10 +33,7 @@ pidfile ENV.fetch("PIDFILE") { "tmp/pids/server.pid" } # processes). # worker_count = (Concurrent.processor_count * 0.666).ceil -# Keep the raw setting: WEB_CONCURRENCY=auto must not be coerced with to_i (that -# is 0 and would look like single-process mode while Puma still forks workers). -configured_workers = ENV.fetch("WEB_CONCURRENCY") { worker_count } -workers configured_workers +workers ENV.fetch("WEB_CONCURRENCY") { worker_count } ENV["JOB_CONCURRENCY"] ||= worker_count.to_s @@ -53,14 +50,11 @@ plugin :tmp_restart # Reset all membership connections Membership.disconnect_all -# Only the literal 0 is single-process. "auto" and positive counts fork workers; -# those must start the checkpointer after boot so a surviving worker can take -# the lock if the previous leader exits. -if SqliteWalCheckpoint.single_puma_process?(configured_workers) - SqliteWalCheckpoint.start -else - on_worker_boot { SqliteWalCheckpoint.start } -end +# Initializer starts a contender in this process. Stop before fork so workers +# do not inherit the flock; each worker starts its own contender. Single-process +# mode never forks, so the initializer's contender keeps running. +before_fork { SqliteWalCheckpoint.stop } +on_worker_boot { SqliteWalCheckpoint.start } Signal.trap :SIGPROF do Thread.list.each do |t| diff --git a/config/puma_dev.rb b/config/puma_dev.rb index 2c7e512..371759a 100644 --- a/config/puma_dev.rb +++ b/config/puma_dev.rb @@ -1,7 +1,5 @@ require File.expand_path("../config/environment", File.dirname(__FILE__)) -SqliteWalCheckpoint.start - Signal.trap :SIGPROF do Thread.list.each do |t| puts t diff --git a/lib/sqlite_wal_checkpoint.rb b/lib/sqlite_wal_checkpoint.rb index 1060b3c..4fc3e94 100644 --- a/lib/sqlite_wal_checkpoint.rb +++ b/lib/sqlite_wal_checkpoint.rb @@ -2,9 +2,9 @@ # writer and fsyncs during the request. database.yml sets wal_autocheckpoint=0; # this module copies pages off the request thread with PASSIVE checkpoints. # -# Every non-test writer process starts a contender thread (initializer, Puma, -# Resque). A file lock elects one leader; if that process exits, another takes -# over on the next interval so auto-checkpoint stays off safely. +# Every non-test process starts a contender. Callers that fork (Puma, Resque pool) +# must stop before fork and start again in the child so the flock is never shared +# across an inherited file descriptor. module SqliteWalCheckpoint INTERVAL = 0.25 MAX_BACKOFF = 30.0 @@ -21,7 +21,6 @@ module SqliteWalCheckpoint return if @thread&.alive? @stop = false - install_exit_handler @thread = Thread.new { run(interval) } @thread.report_on_exception = false end @@ -29,13 +28,13 @@ module SqliteWalCheckpoint @thread end + # Signal the contender to exit and wait for it. Only the contender thread + # acquires or releases the flock (see run's ensure). def stop @stop = true thread = @mutex&.synchronize { @thread } thread&.join(2) - ensure @mutex&.synchronize { @thread = nil if @thread && !@thread.alive? } - release_lock end def checkpoint @@ -64,13 +63,6 @@ module SqliteWalCheckpoint @lock_path = nil @database_path_override = nil @stop = false - @exit_handler_installed = false - end - - # Puma's WEB_CONCURRENCY=auto must not use to_i (that is 0). Only the literal - # 0 means single-process mode; everything else forks workers. - def single_puma_process?(configured_workers) - configured_workers.to_s == "0" end private @@ -81,12 +73,12 @@ module SqliteWalCheckpoint until @stop begin unless database_path - interruptible_sleep interval + sleep interval next end unless acquire_lock - interruptible_sleep interval + sleep interval next end @@ -95,7 +87,7 @@ module SqliteWalCheckpoint until @stop checkpoint_on(database) backoff = interval - interruptible_sleep interval + sleep interval end end ensure @@ -103,7 +95,7 @@ module SqliteWalCheckpoint end rescue => error Rails.logger.warn "SQLite WAL checkpoint failed: #{error.class}: #{error.message}" - interruptible_sleep backoff + sleep backoff backoff = [ backoff * 2, MAX_BACKOFF ].min end end @@ -153,29 +145,8 @@ module SqliteWalCheckpoint @lock_file.flock(File::LOCK_UN) @lock_file.close - rescue Errno::EBADF, IOError - # Already closed by a racing ensure / exit handler. ensure @lock_file = nil end - - def interruptible_sleep(seconds) - deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + seconds - while !@stop && (remaining = deadline - Process.clock_gettime(Process::CLOCK_MONOTONIC)) > 0 - sleep [ remaining, 0.05 ].min - end - end - - def install_exit_handler - return if @exit_handler_installed - - @exit_handler_installed = true - at_exit do - if @lock_file - checkpoint rescue nil - end - stop - end - end end end diff --git a/lib/tasks/resque.rake b/lib/tasks/resque.rake index 5b8bf1a..6e04c74 100644 --- a/lib/tasks/resque.rake +++ b/lib/tasks/resque.rake @@ -4,6 +4,9 @@ end task "resque:pool:setup" do ActiveRecord::Base.connection.disconnect! + # Initializer may have started a contender in the pool master; drop it before + # workers fork so they do not inherit the flock. + SqliteWalCheckpoint.stop Resque::Pool.after_prefork do |job| ActiveRecord::Base.establish_connection diff --git a/test/lib/sqlite_wal_checkpoint_test.rb b/test/lib/sqlite_wal_checkpoint_test.rb index e38a82c..2e973a3 100644 --- a/test/lib/sqlite_wal_checkpoint_test.rb +++ b/test/lib/sqlite_wal_checkpoint_test.rb @@ -35,15 +35,6 @@ class SqliteWalCheckpointTest < ActiveSupport::TestCase assert_empty checkpoint_threads end - test "single_puma_process? treats only literal zero as single-process" do - assert SqliteWalCheckpoint.single_puma_process?(0) - assert SqliteWalCheckpoint.single_puma_process?("0") - assert_not SqliteWalCheckpoint.single_puma_process?("auto") - assert_not SqliteWalCheckpoint.single_puma_process?(2) - assert_not SqliteWalCheckpoint.single_puma_process?("2") - assert_not SqliteWalCheckpoint.single_puma_process?(8) - end - test "tick checkpoints through the elected lock holder" do db_path = build_wal_database(rows: 50) SqliteWalCheckpoint.database_path_override = db_path