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

Prevent app crash when postgres prepared query keys become out of sync with Rails #41356

Merged
merged 1 commit into from Feb 11, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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