Stop WAL checkpointer before fork; one thread owns the flock

Unify startup: initializer starts every non-test process, Puma and Resque
stop before fork and start again in the child. Only the contender thread
releases the lock. Drop Puma::CLI / single_puma_process? special cases.

Co-authored-by: Thomas Klemm <github@tklemm.eu>

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
 

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

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|

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

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

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

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