Skip to content

Commit

Permalink
Increment @counter of prepared postgres statements prior to running t…
Browse files Browse the repository at this point in the history
…he 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.
  • Loading branch information
wbharding authored and MGPalmer committed Feb 11, 2021
1 parent 56ea638 commit 2416da9
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 5 deletions.
6 changes: 6 additions & 0 deletions 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`)

Expand Down
Expand Up @@ -228,11 +228,7 @@ def initialize(connection, max)
end

def next_key
"a#{@counter + 1}"
end

def []=(sql, key)
super.tap { @counter += 1 }
"a#{@counter += 1}"
end

private
Expand Down
18 changes: 18 additions & 0 deletions 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
Expand All @@ -16,6 +18,8 @@ def status
end

class StatementPoolTest < ActiveRecord::PostgreSQLTestCase
fixtures :developers

if Process.respond_to?(:fork)
def test_cache_is_per_pid
cache = StatementPool.new nil, 10
Expand All @@ -37,6 +41,20 @@ def test_dealloc_does_not_raise_on_inactive_connection
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
Expand Down

0 comments on commit 2416da9

Please sign in to comment.