FIX: Falling back to primary PG server not reliable on Rails 7.1

This commit drops the reliance on
`ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#active?` in favor
or using `ActiveRecord::ConnectionAdatpers::PostgreSQLAdatper#execute`
to check if the app can connect to the primary PG server.

This is because `ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#active?`
was changed in Rails 7.1 to return `false` if the connection has not be used to do
something meaningful. Ref:
rails/rails@8551e64.
Due to this change, our fallback to primary checker will keep thinking
that the primary PG server is down. Note that before Rails 7.1, `ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#active?`
will always execute a fake query to check if the query can be executed.

Instead of relying on ActiveRecord's internal API, we will instead rely
on `ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#execute` to
execute a blank query as our means of verifying if the PG server is up
and ready to receive connections.

This commit also updates the ActiveRecord tests to be more reliable by
reducing the Unicorn worker processes to 1 so that we don't have to rely
on flooding the app with requests to get all the Unicorn processes to
failover/fallback.

Co-authored-by: Loïc Guitaut <loic@discourse.org>

.github/workflows/ci.yml

@@ -16,7 +16,7 @@ jobs:
       - name: Setup ruby
         uses: ruby/setup-ruby@v1
         with:
-          ruby-version: '3.2'
+          ruby-version: "3.2"
           bundler-cache: true
 
       - name: Rubocop
@@ -29,13 +29,13 @@ jobs:
           bundle exec stree check Gemfile rails_failover.gemspec $(git ls-files '*.rb')
 
   redis:
-    name: 'Redis (Ruby ${{ matrix.ruby }})'
+    name: "Redis (Ruby ${{ matrix.ruby }})"
     runs-on: ubuntu-latest
 
     strategy:
       fail-fast: false
       matrix:
-        ruby: ['3.4', '3.3', '3.2', '3.1']
+        ruby: ["3.4", "3.3", "3.2", "3.1"]
 
     steps:
       - uses: actions/checkout@v3
@@ -54,21 +54,21 @@ jobs:
 
   active_record:
     runs-on: ubuntu-latest
-    name: 'ActiveRecord ~>${{ matrix.rails }} (Ruby ${{ matrix.ruby }})'
+    name: "ActiveRecord ~>${{ matrix.rails }} (Ruby ${{ matrix.ruby }})"
 
     strategy:
       fail-fast: false
       matrix:
-        ruby: ['3.4', '3.3', '3.2', '3.1']
-        rails: ['7.1.0', '7.0.0']
+        ruby: ["3.4", "3.3", "3.2", "3.1"]
+        rails: ["7.1.0", "7.0.0"]
         include:
-          - ruby: '3.2'
-            rails: '6.1.0'
+          - ruby: "3.2"
+            rails: "6.1.0"
         exclude:
-          - ruby: '3.4'
-            rails: '7.0.0'
-          - ruby: '3.3'
-            rails: '7.0.0'
+          - ruby: "3.4"
+            rails: "7.0.0"
+          - ruby: "3.3"
+            rails: "7.0.0"
 
     steps:
       - uses: actions/checkout@v3
@@ -81,14 +81,17 @@ jobs:
       - name: Setup gems
         run: bundle install
 
+      - name: echo bundler
+        run: echo $(which bundle)
+
       - name: Setup postgres
         run: |
           make setup_pg
-          make start_pg
 
       - name: ActiveRecord specs
         env:
           RAILS_VERSION: ${{ matrix.rails }}
+          VERBOSE: 1
         run: bin/rspec active_record
 
   publish:

.gitignore

@@ -16,4 +16,4 @@
 *.rdb
 
 *.gem
-Gemfile.lock
+Gemfile.lock

.rspec

@@ -1,3 +1,4 @@
 --color
 --require spec_helper
 --format documentation
+--order random

CHANGELOG.md

@@ -5,7 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.1.1] - 2024-07-02
+
+- FIX: Falling back to primary PG server not reliable on Rails 7.1
+
 ## [2.1.0] - 2024-05-29
+
 - DEV: Update dependencies to officially support Rails 7.1
 
 ## [2.0.1] - 2023-05-30

README.md

@@ -107,7 +107,6 @@ To install this gem onto your local machine, run `bundle exec rake install`. To
 The ActiveRecord failover tests are run against a dummy Rails server. Run the following commands to run the test:
 
 1. `make setup_pg`
-2. `make start_pg`
 3. `bin/rspec active_record`. You may also run the tests with more unicorn workers by adding the `UNICORN_WORKERS` env variable.
 
 #### Redis

lib/rails_failover/active_record/handler.rb

@@ -64,7 +64,8 @@ def initiate_fallback_to_primary
                 spec_name,
                 role: handler_key,
               )
-            connection_active = connection.active?
+
+            connection_active = connection.verify!
           rescue => e
             logger.debug "#{Process.pid} Connection to server for '#{handler_key} #{spec_name}' failed with '#{e.message}'"
           ensure

lib/rails_failover/active_record/railtie.rb

@@ -11,7 +11,7 @@ class Railtie < ::Rails::Railtie
         app.config.active_record_rails_failover = true
         ::ActiveSupport.on_load(:active_record) do
           begin
-            ::ActiveRecord::Base.connection
+            ::ActiveRecord::Base.connection.verify!
           rescue ::ActiveRecord::NoDatabaseError
             # Do nothing since database hasn't been created
           rescue ::PG::Error, ::ActiveRecord::ConnectionNotEstablished

lib/rails_failover/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module RailsFailover
-  VERSION = "2.1.0"
+  VERSION = "2.1.1"
 end

makefile

@@ -3,19 +3,7 @@ include redis.mk
 
 all: redis
 
-active_record: teardown_dummy_rails_server setup_dummy_rails_server test_active_record
+active_record: test_active_record
 
 test_active_record:
-	@ACTIVE_RECORD=1 bundle exec rspec --tag type:active_record ${RSPEC_PATH}
-
-setup_dummy_rails_server:
-	@cd spec/support/dummy_app && BUNDLE_GEMFILE=Gemfile bundle install --quiet && BUNDLE_GEMFILE=Gemfile RAILS_ENV=production $(BUNDLER_BIN) exec rails db:create db:migrate db:seed
-
-start_dummy_rails_server:
-	@cd spec/support/dummy_app && BUNDLE_GEMFILE=Gemfile SECRET_KEY_BASE=somekey bundle exec unicorn -c config/unicorn.conf.rb -D -E production
-
-stop_dummy_rails_server:
-	@kill -TERM $(shell cat spec/support/dummy_app/tmp/pids/unicorn.pid)
-
-teardown_dummy_rails_server:
-	@cd spec/support/dummy_app && (! (bundle check > /dev/null 2>&1) || BUNDLE_GEMFILE=Gemfile DISABLE_DATABASE_ENVIRONMENT_CHECK=1 RAILS_ENV=production $(BUNDLER_BIN) exec rails db:drop)
+	@ACTIVE_RECORD=1 bundle exec rspec --tag type:active_record ${RSPEC_PATH}

postgresql.mk

@@ -41,9 +41,6 @@ start_pg_primary:
 start_pg_replica:
 	@$(PG_BIN_DIR)/pg_ctl --silent --log /dev/null -w -D $(PG_REPLICA_DATA_DIR) -o "-p $(PG_REPLICA_PORT)" -o "-k $(PG_REPLICA_RUN_DIR)" start
 
-restart_pg_primary:
-	@$(PG_BIN_DIR)/pg_ctl --silent --log /dev/null -w -D $(PG_PRIMARY_DATA_DIR) -o "-p $(PG_PRIMARY_PORT)" -o "-k $(PG_PRIMARY_RUN_DIR)" restart
-
 stop_pg_primary:
 	@$(PG_BIN_DIR)/pg_ctl --silent --log /dev/null -w -D $(PG_PRIMARY_DATA_DIR) -o "-p $(PG_PRIMARY_PORT)" -o "-k $(PG_PRIMARY_RUN_DIR)" stop
 

spec/helpers/generic_helper.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+module GenericHelper
+  def wait_for(timeout:, &blk)
+    till = Time.now + (timeout.to_f / 1000)
+    sleep 0.001 while Time.now < till && !blk.call
+  end
+end

spec/helpers/postgres_helper.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+module PostgresHelper
+  def start_pg_primary
+    return if pg_primary_is_up?
+    system("make start_pg_primary")
+    wait_for_pg_primary_to_be_up
+  end
+
+  def stop_pg_primary
+    return if pg_primary_is_down?
+    system("make stop_pg_primary")
+    wait_for_pg_primary_to_be_down
+  end
+
+  def start_pg_replica
+    system("make start_pg_replica")
+    wait_for_pg_replica_to_be_up
+  end
+
+  def stop_pg_replica
+    system("make stop_pg_replica")
+    wait_for_pg_replica_to_be_down
+  end
+
+  private
+
+  def pg_primary_is_up?
+    File.exist?(pg_primary_pid_path)
+  end
+
+  def pg_primary_is_down?
+    !File.exist?(pg_primary_pid_path)
+  end
+
+  def wait_for_pg_primary_to_be_up
+    wait_for_pg_to_be_up(role: :primary)
+  end
+
+  def wait_for_pg_primary_to_be_down
+    wait_for_pg_to_be_down(role: :primary)
+  end
+
+  def wait_for_pg_replica_to_be_up
+    wait_for_pg_to_be_up(role: :replica)
+  end
+
+  def wait_for_pg_replica_to_be_down
+    wait_for_pg_to_be_down(role: :replica)
+  end
+
+  def wait_for_pg_to_be_up(role:)
+    wait_for(timeout: 5) { File.exist?(self.send("pg_#{role}_pid_path")) }
+  end
+
+  def wait_for_pg_to_be_down(role:)
+    wait_for(timeout: 5) { !File.exist?(self.send("pg_#{role}_pid_path")) }
+  end
+
+  def pg_replica_pid_path
+    "#{gem_root}/tmp/replica/data/postmaster.pid"
+  end
+
+  def pg_primary_pid_path
+    "#{gem_root}/tmp/primary/data/postmaster.pid"
+  end
+
+  def gem_root
+    File.expand_path("../..", __dir__)
+  end
+end

spec/helpers/rails_server_helper.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+module RailsServerHelper
+  def setup_rails_server
+    execute_command(
+      "cd spec/support/dummy_app && BUNDLE_GEMFILE=Gemfile $(which bundle) install --quiet",
+    )
+
+    execute_command(
+      "cd spec/support/dummy_app && BUNDLE_GEMFILE=Gemfile RAILS_ENV=production bin/bundle exec rails db:create db:migrate db:seed",
+    )
+  end
+
+  def start_rails_server
+    if (
+         (unicorn_master_pid = get_unicorn_master_pid) != 0 &&
+           (get_unicorn_worker_pids(unicorn_master_pid).size == 1.to_i)
+       )
+      return
+    end
+
+    system(
+      "cd spec/support/dummy_app && BUNDLE_GEMFILE=Gemfile SECRET_KEY_BASE=somekey bin/bundle exec unicorn -c config/unicorn.conf.rb -D -E production",
+    )
+
+    count = 0
+    timeout = 10
+
+    while (unicorn_master_pid = get_unicorn_master_pid) == 0
+      raise "Timeout while waiting for unicorn master to be up" if count == timeout
+      count += 1
+      sleep 1
+    end
+
+    count = 0
+    timeout = 10
+
+    while get_unicorn_worker_pids(unicorn_master_pid).size != 1.to_i
+      raise "Timeout while waiting for unicorn worker to be up" if count == timeout
+      count += 1
+      sleep 1
+    end
+
+    true
+  end
+
+  def stop_rails_server
+    system("kill -15 #{get_unicorn_master_pid}")
+
+    count = 0
+    timeout = 10
+
+    while get_unicorn_master_pid != 0
+      raise "Timeout while waiting for unicorn master to be down" if count == timeout
+      count += 1
+      sleep 1
+    end
+
+    true
+  end
+
+  def teardown_rails_server
+    execute_command(
+      "cd spec/support/dummy_app && BUNDLE_GEMFILE=Gemfile DISABLE_DATABASE_ENVIRONMENT_CHECK=1 RAILS_ENV=production bin/bundle exec rails db:drop",
+    )
+  end
+
+  private
+
+  def execute_command(command)
+    output = `#{command}`
+    raise "Command failed: #{command}\nOutput: #{output}" unless $?.success?
+
+    puts output if ENV["VERBOSE"]
+
+    output
+  end
+
+  def get_unicorn_master_pid
+    execute_command(
+      "ps aux | grep \"unicorn master\" | grep -v \"grep\" | awk '{print $2}'",
+    ).strip.to_i
+  end
+
+  def get_unicorn_worker_pids(unicorn_master_pid)
+    execute_command("pgrep -P #{unicorn_master_pid}").split("\n").map(&:to_i)
+  end
+end