From 2416da997500b381cc3de506ee43d88591355842 Mon Sep 17 00:00:00 2001 From: Bill Harding Date: Wed, 6 Jan 2021 08:34:37 -0800 Subject: [PATCH] Increment @counter of prepared postgres statements prior to running the query. If the next query or the prepared statement itself get interrupted, this prevents the database session getting stuck perpetually retrying to recreate the same prepared statement. --- activerecord/CHANGELOG.md | 6 ++++++ .../connection_adapters/postgresql_adapter.rb | 6 +----- .../adapters/postgresql/statement_pool_test.rb | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 8bdc7d2c33..f0b78070f6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Increment postgres prepared statement counter before making a prepared statement, so if the statement is aborted + without Rails knowledge (e.g., if app gets kill -9d during long-running query or due to Rack::Timeout), app won't end + up in perpetual crash state for being inconsistent with Postgres. + + *wbharding*, *Martin Tepper* + * Switch to database adapter return type for `ActiveRecord::Calculations.calculate` when called with `:average` (aliased as `ActiveRecord::Calculations.average`) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index f3243f8884..fd9571af1e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -228,11 +228,7 @@ module ActiveRecord end def next_key - "a#{@counter + 1}" - end - - def []=(sql, key) - super.tap { @counter += 1 } + "a#{@counter += 1}" end private diff --git a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb index fef4b02b04..6029630d89 100644 --- a/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb +++ b/activerecord/test/cases/adapters/postgresql/statement_pool_test.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require "cases/helper" +require "models/computer" +require "models/developer" module ActiveRecord module ConnectionAdapters @@ -16,6 +18,8 @@ module ActiveRecord end class StatementPoolTest < ActiveRecord::PostgreSQLTestCase + fixtures :developers + if Process.respond_to?(:fork) def test_cache_is_per_pid cache = StatementPool.new nil, 10 @@ -37,6 +41,20 @@ module ActiveRecord cache["foo"] = "bar" assert_nothing_raised { cache.clear } end + + def test_prepared_statements_do_not_get_stuck_on_query_interruption + pg_connection = ActiveRecord::Base.connection.instance_variable_get(:@connection) + pg_connection.stub(:get_last_result, -> { raise "random error" }) do + assert_raises(RuntimeError) do + Developer.where(name: "David").last + end + + # without fix, this raises PG::DuplicatePstatement: ERROR: prepared statement "a3" already exists + assert_raises(RuntimeError) do + Developer.where(name: "David").last + end + end + end end end end