Skip to content

Commit b4c19d0

Browse files
committed
Make internal query() retryable
When `permanent_connection_checkout` was [introduced][1], Active Record was changed to always use `with_connection` and [never][2] `lease_connection`. This causes apps that never call `lease_connection` on their own (or in a gem) during a request to no longer have a connection pinned to the current Thread/Fiber. By checking in/out connections as needed, the number of times a connection must be verified during a single request has significantly increased (more pings = more latency). The good news is that Active Record also now [defers][3] connection verification and can use retryable queries as verification instead of explicit pings. This commit makes all internal SELECT `query()`s retryable so that they can be used as connection verification. While this likely won't help as much in production for applications using a schema cache dump, it will definitely decrease latency for applications in development and those that do not use a dump in production. There are a few places where `query_value` was used to update sequences. Those have been changed to use `internal_execute` since only idempotent SELECT queries are being automatically retried and Active Record doesn't actually care about the return value of these queries anyways (so no reason to use `internal_exec_query`). [1]: 4db9f51 [2]: 7c68c52 [3]: 7fe221d
1 parent fecd069 commit b4c19d0

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ def query_values(...) # :nodoc:
110110
query(...).map(&:first)
111111
end
112112

113-
def query(...) # :nodoc:
114-
internal_exec_query(...).rows
113+
def query(sql, name = nil, allow_retry: true, materialize_transactions: true) # :nodoc:
114+
internal_exec_query(sql, name, allow_retry:, materialize_transactions:).rows
115115
end
116116

117117
# Determines whether the SQL statement is a write query.

activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ def set_pk_sequence!(table, value) # :nodoc:
320320
if sequence
321321
quoted_sequence = quote_table_name(sequence)
322322

323-
query_value("SELECT setval(#{quote(quoted_sequence)}, #{value})", "SCHEMA")
323+
internal_execute("SELECT setval(#{quote(quoted_sequence)}, #{value})", "SCHEMA")
324324
else
325325
@logger.warn "#{table} has primary key #{pk} with no default sequence." if @logger
326326
end
@@ -351,7 +351,7 @@ def reset_pk_sequence!(table, pk = nil, sequence = nil) # :nodoc:
351351
end
352352
end
353353

354-
query_value("SELECT setval(#{quote(quoted_sequence)}, #{max_pk || minvalue}, #{max_pk ? true : false})", "SCHEMA")
354+
internal_execute("SELECT setval(#{quote(quoted_sequence)}, #{max_pk || minvalue}, #{max_pk ? true : false})", "SCHEMA")
355355
end
356356
end
357357

0 commit comments

Comments
 (0)