diff --git a/CHANGELOG.md b/CHANGELOG.md index 08687d93..2acc222b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Fixed the `aws_mysql2` ActiveRecord adapter dropping the `flags`, `encoding`, `socket`, and `reconnect` client options, which removed ActiveRecord's `FOUND_ROWS` flag so that statements such as `update_all` reported changed rows instead of matched rows ([PR #193](https://github.com/aws/aws-advanced-ruby-driver-wrapper/pull/193)). - Fixed the `aws_postgresql` and `aws_mysql2` ActiveRecord adapters not translating connection errors the way the standard adapters do, so a missing database was not reported as `ActiveRecord::NoDatabaseError` and `db:prepare` (and `bin/setup`) failed instead of creating it ([PR #193](https://github.com/aws/aws-advanced-ruby-driver-wrapper/pull/193)). - Fixed the `aws_postgresql` and `aws_mysql2` ActiveRecord adapters keeping ActiveRecord's prepared statement cache after a successful failover, so every query prepared before the failover failed on that connection with `Method invoked against old connection` until the process restarted ([PR #194](https://github.com/aws/aws-advanced-ruby-driver-wrapper/pull/194), [documentation](https://aws.github.io/aws-advanced-wrapper-docs/ruby/enhanced-failover)). +- Fixed `db:drop`, `db:reset`, and `db:test:prepare` failing on Aurora PostgreSQL with `database "" is being accessed by other users`. The same error occurred when running `bin/rails test` after a schema change. The cause was the wrapper's topology and Blue/Green monitors, which kept their own connections open to the database being dropped. The wrapper now stops these monitors before dropping a database and when ActiveRecord clears all its connections ([PR #195](https://github.com/aws/aws-advanced-ruby-driver-wrapper/pull/195)). ## [1.0.0] - 2026-10-05 diff --git a/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_connection_handler.rb b/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_connection_handler.rb new file mode 100644 index 00000000..785f0809 --- /dev/null +++ b/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_connection_handler.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). +# You may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require 'active_record' +require_relative '../services/service_utility' +require_relative '../host/rds_host_list_provider' +require_relative '../plugins/blue_green/blue_green_plugin' + +module ActiveRecord + module ConnectionAdapters + # Stops the wrapper's background monitors and Blue/Green status providers when ActiveRecord closes every + # connection pool, and forgets the state they cached, so that the next connection starts new ones straight away. + # + # The monitors keep their own connections to the database, outside ActiveRecord's pools. Closing + # everything is what ActiveRecord does before another process takes over the database, for example + # when bin/rails test runs db:test:prepare in a child process to drop and reload the test database, + # and PostgreSQL refuses to drop a database that a monitor is still connected to. The next + # connection that needs a monitor starts a new one. + module AwsConnectionHandler + def clear_all_connections!(...) + super + core = AwsAdvancedRubyDriverWrapper::Services::CoreServices + core.monitor_service.stop_and_remove_all + AwsAdvancedRubyDriverWrapper::Plugins::BlueGreen::BlueGreenPlugin.clean_up_providers + # Without this, connections would keep reading cached state that nothing refreshes any more. + cached = [AwsAdvancedRubyDriverWrapper::Host::RdsHostListProvider::TOPOLOGY_CACHE_NAME, + AwsAdvancedRubyDriverWrapper::Plugins::BlueGreen::BlueGreenPlugin::BLUE_GREEN_NAME] + cached.each { |name| core.storage_service.clear(name) if core.storage_service.registered?(name) } + end + end + + ConnectionHandler.prepend(AwsConnectionHandler) + end +end diff --git a/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_mysql2_adapter.rb b/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_mysql2_adapter.rb index 5ec71b8e..df2de6f7 100644 --- a/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_mysql2_adapter.rb +++ b/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_mysql2_adapter.rb @@ -17,6 +17,7 @@ require 'active_record/connection_adapters/mysql2_adapter' require_relative '../mysql' require_relative '../errors' +require_relative 'aws_connection_handler' module ActiveRecord module ConnectionAdapters diff --git a/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_postgresql_adapter.rb b/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_postgresql_adapter.rb index 9687f080..4b5078f5 100644 --- a/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_postgresql_adapter.rb +++ b/lib/aws_advanced_ruby_driver_wrapper/active_record/aws_postgresql_adapter.rb @@ -17,6 +17,7 @@ require 'active_record/connection_adapters/postgresql_adapter' require_relative '../postgresql' require_relative '../errors' +require_relative 'aws_connection_handler' module ActiveRecord module ConnectionAdapters @@ -90,6 +91,16 @@ def active? super end + # Used by db:drop, db:reset, db:test:prepare and db:purge. ActiveRecord disconnects its own + # connections first, but the topology monitor shared by this cluster's connections, and the Blue/Green + # status providers, keep their own connections to the database, and PostgreSQL refuses to drop a + # database other sessions are using. + def drop_database(name) + raw_connection.stop_topology_monitor + AwsAdvancedRubyDriverWrapper::Plugins::BlueGreen::BlueGreenPlugin.clean_up_providers + super + end + def translate_exception(exception, message:, sql:, binds:) return super unless exception.is_a?(AwsAdvancedRubyDriverWrapper::Errors::AwsError) diff --git a/lib/aws_advanced_ruby_driver_wrapper/postgresql.rb b/lib/aws_advanced_ruby_driver_wrapper/postgresql.rb index 4346b76c..c84f7f69 100644 --- a/lib/aws_advanced_ruby_driver_wrapper/postgresql.rb +++ b/lib/aws_advanced_ruby_driver_wrapper/postgresql.rb @@ -224,6 +224,14 @@ def close alias finish close + # Stops the background topology monitor of this connection's cluster, which closes the connections it + # holds to the database it was started for. The next connection that needs topology starts a new one. + # PostgreSQL refuses to drop a database that other sessions are connected to, so this has to happen + # before dropping the database the monitor was started for. + def stop_topology_monitor + @service_container.host_service.host_list_provider&.stop_monitor + end + # Resets the connection through the pipeline and returns this wrapper, so the reset connection stays # usable through it. The driver's own reset tears down and re-establishes the underlying socket, which # clears any server-side session state, so the tracked session state is reset to match. diff --git a/lib/aws_advanced_ruby_driver_wrapper/utils/storage/storage_service.rb b/lib/aws_advanced_ruby_driver_wrapper/utils/storage/storage_service.rb index eadebdf9..40ed4ef5 100644 --- a/lib/aws_advanced_ruby_driver_wrapper/utils/storage/storage_service.rb +++ b/lib/aws_advanced_ruby_driver_wrapper/utils/storage/storage_service.rb @@ -107,6 +107,12 @@ def remove(name, key) fetch_cache!(name).remove(key) end + # @param name [Symbol] cache name. + # @return [Boolean] whether a cache is registered under the name + def registered?(name) + @lock.synchronize { @caches.key?(name) } + end + # Clears all items for a given name. # @param name [Symbol] registered cache name. def clear(name) diff --git a/spec/integration/container/failover_activerecord_spec.rb b/spec/integration/container/failover_activerecord_spec.rb index f61daa60..e41e371f 100644 --- a/spec/integration/container/failover_activerecord_spec.rb +++ b/spec/integration/container/failover_activerecord_spec.rb @@ -26,6 +26,7 @@ require_relative 'utils/database_engine' require_relative 'utils/rds_test_utility' require_relative 'utils/retry_helper' +require_relative 'utils/topology_helper' require 'aws_advanced_ruby_driver_wrapper' require 'aws_advanced_ruby_driver_wrapper/active_record/aws_mysql2_adapter' require 'aws_advanced_ruby_driver_wrapper/active_record/aws_postgresql_adapter' @@ -100,23 +101,12 @@ def establish_failover_connection(host:, port:, variables: nil, prepared_stateme ) end - def warm_failover_topology - config = Integration::DriverHelper.native_config( - drv, - host: proxy_info.cluster_endpoint, - port: proxy_info.cluster_endpoint_port, - user: proxy_info.username, - password: proxy_info.password, - dbname: proxy_info.default_dbname - ) - props = { - AwsAdvancedRubyDriverWrapper::PropertyDefinition::PLUGINS.name => 'failover', - AwsAdvancedRubyDriverWrapper::PropertyDefinition::CLUSTER_INSTANCE_HOST_PATTERN.name => - "?.#{proxy_info.instance_endpoint_suffix}:#{proxy_info.instance_endpoint_port}" - } - discovered = Integration::TopologyHelper.warm_topology_cache( - drv: drv, config: config, props: props, min_instances: proxy_info.instances.size - ) + # Blocks until the topology monitor started by the ActiveRecord connection has cached every instance. + # This must run after the connection is established: establish_failover_connection calls + # clear_all_connections!, which stops the monitors and clears the topology cache, so warming the cache + # beforehand would be undone. + def wait_for_failover_topology + discovered = Integration::TopologyHelper.wait_for_topology(min_instances: proxy_info.instances.size) expect(discovered).to be(true), 'Topology was not discovered before failover' end @@ -314,10 +304,6 @@ def instance_id_via(conn) features: [Integration::TestEnvironmentFeatures::NETWORK_OUTAGES_ENABLED] do enable_on_num_instances(min_instances: 2, max_instances: 2) - # Warm the topology cache first: this test connects to a bare reader instance, so without a cached - # writer host, writer failover would have only the dead reader to probe once connectivity is cut. - warm_failover_topology - probe = session_probe reader_instance = proxy_info.instances[1] establish_failover_connection( @@ -330,6 +316,10 @@ def instance_id_via(conn) # Session variables are applied via SET SESSION, so they set fine on a reader connection too. expect(adapter.select_value(probe[:read_sql])).to eq(probe[:expected]) + # This test connects to a bare reader instance, so without a cached writer host, writer failover + # would have only the dead reader to probe once connectivity is cut. + wait_for_failover_topology + Integration::ProxyHelper.disable_connectivity(reader_instance.instance_id) expect { current_instance_id }.to raise_error( diff --git a/spec/unit/aws_connection_handler_spec.rb b/spec/unit/aws_connection_handler_spec.rb new file mode 100644 index 00000000..632a0910 --- /dev/null +++ b/spec/unit/aws_connection_handler_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"). +# You may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +require_relative '../spec_helper' +require 'aws_advanced_ruby_driver_wrapper/active_record/aws_postgresql_adapter' +require 'aws_advanced_ruby_driver_wrapper/active_record/aws_mysql2_adapter' + +# Rails closes every pool and then drops the test database from a separate process (bin/rails test +# runs db:test:prepare as a child when the schema has changed). The wrapper's monitors keep their own +# connections to the database, so they have to stop when ActiveRecord closes everything. +RSpec.describe 'ActiveRecord::ConnectionAdapters::AwsConnectionHandler' do + let(:handler) { ActiveRecord::ConnectionAdapters::ConnectionHandler.new } + let(:monitor_service) { AwsAdvancedRubyDriverWrapper::Services::CoreServices.monitor_service } + let(:storage) { AwsAdvancedRubyDriverWrapper::Services::CoreServices.storage_service } + let(:topology) { AwsAdvancedRubyDriverWrapper::Host::RdsHostListProvider::TOPOLOGY_CACHE_NAME } + let(:blue_green) { AwsAdvancedRubyDriverWrapper::Plugins::BlueGreen::BlueGreenPlugin } + let(:pool) { double('pool') } + let(:calls) { [] } + + before do + allow(pool).to receive(:disconnect!) { calls << :disconnect_pool } + allow(monitor_service).to receive(:stop_and_remove_all) { calls << :stop_monitors } + allow(blue_green).to receive(:clean_up_providers) { calls << :stop_blue_green_providers } + end + + it 'stops the wrapper monitors and Blue/Green status providers after clearing all connections' do + allow(handler).to receive(:each_connection_pool).with(nil).and_return([pool]) + + handler.clear_all_connections! + + expect(calls).to eq(%i[disconnect_pool stop_monitors stop_blue_green_providers]) + end + + it 'clears the cached topology so the next connection starts a new monitor' do + storage.register(topology, ttl: 300) + storage.set(topology, 'my-cluster', [:host]) + allow(handler).to receive(:each_connection_pool).with(nil).and_return([pool]) + + handler.clear_all_connections! + + expect(storage.get(topology, 'my-cluster', register_access: false)).to be_nil + end + + it 'clears the cached Blue/Green status so the next connection starts a new provider' do + storage.register(blue_green::BLUE_GREEN_NAME, ttl: 3600) + storage.set(blue_green::BLUE_GREEN_NAME, '1', :status) + allow(handler).to receive(:each_connection_pool).with(nil).and_return([pool]) + + handler.clear_all_connections! + + expect(storage.get(blue_green::BLUE_GREEN_NAME, '1', register_access: false)).to be_nil + end + + it 'does not fail when no wrapper connection has registered the caches' do + allow(storage).to receive(:registered?).and_return(false) + allow(storage).to receive(:clear) + allow(handler).to receive(:each_connection_pool).with(nil).and_return([pool]) + + expect { handler.clear_all_connections! }.not_to raise_error + expect(storage).not_to have_received(:clear) + end + + it 'passes the role through to ActiveRecord' do + allow(handler).to receive(:each_connection_pool).with(:all).and_return([pool]) + + handler.clear_all_connections!(:all) + + expect(calls).to eq(%i[disconnect_pool stop_monitors stop_blue_green_providers]) + end + + it 'does not stop the monitors when only idle connections are flushed' do + allow(handler).to receive(:each_connection_pool).and_return([]) + + handler.flush_idle_connections! + + expect(monitor_service).not_to have_received(:stop_and_remove_all) + end +end diff --git a/spec/unit/aws_postgresql_adapter_spec.rb b/spec/unit/aws_postgresql_adapter_spec.rb index 53815cdf..3bb41472 100644 --- a/spec/unit/aws_postgresql_adapter_spec.rb +++ b/spec/unit/aws_postgresql_adapter_spec.rb @@ -108,6 +108,25 @@ def connect_failing_with(message, client_config = config) end end + describe '#drop_database' do + # DROP DATABASE fails while any other session is connected to the database, and the topology monitor + # keeps connections to the database it was created for after ActiveRecord has disconnected. + it 'stops the topology monitor and Blue/Green status providers before dropping the database' do + adapter = described_class.allocate + wrapper_connection = instance_double(AwsAdvancedRubyDriverWrapper::WrapperPgConnection) + calls = [] + allow(adapter).to receive(:raw_connection).and_return(wrapper_connection) + allow(wrapper_connection).to receive(:stop_topology_monitor) { calls << :stop_topology_monitor } + allow(adapter).to receive(:quote_table_name) { |name| %("#{name}") } + allow(adapter).to receive(:execute) { |sql| calls << sql } + allow(AwsAdvancedRubyDriverWrapper::Plugins::BlueGreen::BlueGreenPlugin).to receive(:clean_up_providers) { calls << :stop_blue_green_providers } + + adapter.drop_database('mydb') + + expect(calls).to eq([:stop_topology_monitor, :stop_blue_green_providers, 'DROP DATABASE IF EXISTS "mydb"']) + end + end + describe '#translate_exception' do let(:adapter) { described_class.allocate } let(:sql) { 'SELECT 1' } diff --git a/spec/unit/utils/storage/storage_service_spec.rb b/spec/unit/utils/storage/storage_service_spec.rb index 23878d64..3bc51415 100644 --- a/spec/unit/utils/storage/storage_service_spec.rb +++ b/spec/unit/utils/storage/storage_service_spec.rb @@ -100,6 +100,14 @@ end end + describe '#registered?' do + it 'is true only for a registered cache' do + service.register(:data, ttl: 60) + expect(service.registered?(:data)).to be true + expect(service.registered?(:other)).to be false + end + end + describe '#clear' do it 'removes all items for a name' do service.register(:data, ttl: 60) diff --git a/spec/unit/wrapper_pg_connection_spec.rb b/spec/unit/wrapper_pg_connection_spec.rb index faab6719..83defa67 100644 --- a/spec/unit/wrapper_pg_connection_spec.rb +++ b/spec/unit/wrapper_pg_connection_spec.rb @@ -525,6 +525,18 @@ def build_wrapper(container) end end + describe '#stop_topology_monitor' do + it 'stops the topology monitor of the host list provider without calling the plugins' do + provider = double('host_list_provider', stop_monitor: nil) + host_service = double('host_service', host_list_provider: provider) + wrapper = build_wrapper(double('service_container', host_service: host_service)) + + wrapper.stop_topology_monitor + + expect(provider).to have_received(:stop_monitor) + end + end + describe '#reset' do let(:session_state_service) { AwsAdvancedRubyDriverWrapper::Services::SessionStateService.new }