Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ jobs:
- name: Setup postgres
run: |
make setup_pg
make start_pg

- name: ActiveRecord specs
env:
Expand Down
1 change: 1 addition & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
--color
--require spec_helper
--format documentation
--order random
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Copy link
Contributor Author

@tgxworld tgxworld Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now managed by the RSpec test itself so that we can ensure that each test starts with the Postgres servers being in the same state.

3. `bin/rspec active_record`. You may also run the tests with more unicorn workers by adding the `UNICORN_WORKERS` env variable.

#### Redis
Expand Down
9 changes: 5 additions & 4 deletions lib/rails_failover/active_record/handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ def initiate_fallback_to_primary

begin
connection =
::ActiveRecord::Base.connection_handler.retrieve_connection(
spec_name,
role: handler_key,
)
::ActiveRecord::Base
.connection_handler
.retrieve_connection(spec_name, role: handler_key)
.tap(&:verify!)

tgxworld marked this conversation as resolved.
Show resolved Hide resolved
connection_active = connection.active?
rescue => e
logger.debug "#{Process.pid} Connection to server for '#{handler_key} #{spec_name}' failed with '#{e.message}'"
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_failover/active_record/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actual fix is also here.

rescue ::ActiveRecord::NoDatabaseError
# Do nothing since database hasn't been created
rescue ::PG::Error, ::ActiveRecord::ConnectionNotEstablished
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_failover/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module RailsFailover
VERSION = "2.1.0"
VERSION = "2.1.1"
end
15 changes: 2 additions & 13 deletions makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,8 @@ 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:
@BUNDLE_GEMFILE=./spec/support/dummy_app/Gemfile bundle install --quiet
@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)
Comment on lines -11 to -21
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved these to be managed by the RSpec tests so that things are easier to reason about.

3 changes: 0 additions & 3 deletions postgresql.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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

Comment on lines -44 to -46
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command is no longer needed.

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

Expand Down
8 changes: 8 additions & 0 deletions spec/helpers/generic_helper.rb
Original file line number Diff line number Diff line change
@@ -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
71 changes: 71 additions & 0 deletions spec/helpers/postgres_helper.rb
Original file line number Diff line number Diff line change
@@ -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
84 changes: 84 additions & 0 deletions spec/helpers/rails_server_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
# frozen_string_literal: true

module RailsServerHelper
def setup_rails_server
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
Loading
Loading