Skip to content

Commit a4cefc1

Browse files
committed
Maintain connection verification briefly after use
This means that a database connection that fails in between two requests that are less than two seconds apart may cause the failure-following request to die (if its first query is not retryable). That's not a big concern in practice: per-request verification is intended to protect against the case that the database connection failed some large time before the request arrives. A request beginning within seconds of the failure is morally equivalent to a request that was already in flight. Between automatically retryable select queries and BEGINs, the first query of a request will very commonly be retryable anyway. This change's value is in our new use of short-term leasing via with_connection, where we can otherwise end up re-verifying for every non-retryable query.
1 parent 614d1d9 commit a4cefc1

File tree

3 files changed

+40
-1
lines changed

3 files changed

+40
-1
lines changed

activerecord/CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
* Remember when a database connection has recently been verified (for
2+
two seconds, by default), to avoid repeated reverifications during a
3+
single request.
4+
5+
This should recreate a similar rate of verification as in Rails 7.1,
6+
where connections are leased for the duration of a request, and thus
7+
only verified once.
8+
9+
*Matthew Draper*
10+
111
* Allow to reset cache counters for multiple records.
212

313
```

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,10 @@ def connection_retries
218218
(@config[:connection_retries] || 1).to_i
219219
end
220220

221+
def verify_timeout
222+
(@config[:verify_timeout] || 2).to_i
223+
end
224+
221225
def retry_deadline
222226
if @config[:retry_deadline]
223227
@config[:retry_deadline].to_f
@@ -1003,6 +1007,9 @@ def with_raw_connection(allow_retry: false, materialize_transactions: true)
10031007
if @verified
10041008
# Cool, we're confident the connection's ready to use. (Note this might have
10051009
# become true during the above #materialize_transactions.)
1010+
elsif (last_activity = seconds_since_last_activity) && last_activity < verify_timeout
1011+
# We haven't actually verified the connection since we acquired it, but it
1012+
# has been used very recently. We're going to assume it's still okay.
10061013
elsif reconnectable
10071014
if allow_retry
10081015
# Not sure about the connection yet, but if anything goes wrong we can

activerecord/test/cases/adapter_test.rb

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -611,11 +611,14 @@ def teardown
611611
assert_predicate @connection, :active?
612612
end
613613

614-
test "querying a 'clean' failed connection restores and succeeds" do
614+
test "querying a 'clean' long-failed connection restores and succeeds" do
615615
remote_disconnect @connection
616616

617617
@connection.clean! # this simulates a fresh checkout from the pool
618618

619+
# Backdate last activity to simulate a connection we haven't used in a while
620+
@connection.instance_variable_set(:@last_activity, Process.clock_gettime(Process::CLOCK_MONOTONIC) - 5.minutes)
621+
619622
# Clean did not verify / fix the connection
620623
assert_not_predicate @connection, :active?
621624

@@ -627,11 +630,30 @@ def teardown
627630
assert_predicate @connection, :active?
628631
end
629632

633+
test "querying a 'clean' recently-used but now-failed connection skips verification" do
634+
remote_disconnect @connection
635+
636+
@connection.clean! # this simulates a fresh checkout from the pool
637+
638+
# Clean did not verify / fix the connection
639+
assert_not_predicate @connection, :active?
640+
641+
# Because the query cannot be retried, and we (mistakenly) believe the
642+
# connection is still good, the query will fail. This is what we want,
643+
# because the alternative would be excessive reverification.
644+
assert_raises(ActiveRecord::AdapterError) do
645+
Post.delete_all
646+
end
647+
end
648+
630649
test "quoting a string on a 'clean' failed connection will not prevent reconnecting" do
631650
remote_disconnect @connection
632651

633652
@connection.clean! # this simulates a fresh checkout from the pool
634653

654+
# Backdate last activity to simulate a connection we haven't used in a while
655+
@connection.instance_variable_set(:@last_activity, Process.clock_gettime(Process::CLOCK_MONOTONIC) - 5.minutes)
656+
635657
# Clean did not verify / fix the connection
636658
assert_not_predicate @connection, :active?
637659

0 commit comments

Comments
 (0)