Skip to content

Commit 2d7bc98

Browse files
committed
Allow open transactions to be restored upon DB reconnect
If we know that any open transactions had not been queried (and raw_connection has not been used), then it's safe for us to pick up with a new database connection mid-request.
1 parent dc4420c commit 2d7bc98

File tree

7 files changed

+83
-9
lines changed

7 files changed

+83
-9
lines changed

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,8 +330,24 @@ def transaction_open?
330330
current_transaction.open?
331331
end
332332

333-
def reset_transaction # :nodoc:
333+
def reset_transaction(restore: false) # :nodoc:
334+
# Store the existing transaction state to the side
335+
old_state = @transaction_manager if restore && @transaction_manager&.restorable?
336+
334337
@transaction_manager = ConnectionAdapters::TransactionManager.new(self)
338+
339+
if block_given?
340+
# Reconfigure the connection without any transaction state in the way
341+
result = yield
342+
343+
# Now the connection's fully established, we can swap back
344+
if old_state
345+
@transaction_manager = old_state
346+
@transaction_manager.restore_transactions
347+
end
348+
349+
result
350+
end
335351
end
336352

337353
# Register a record with the current transaction so that its after_commit and after_rollback callbacks

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ def materialized?
140140
@materialized
141141
end
142142

143+
def restore!
144+
@materialized = false
145+
end
146+
143147
def rollback_records
144148
return unless records
145149
ite = records.uniq(&:__id__)
@@ -348,6 +352,20 @@ def dirty_current_transaction
348352
current_transaction.dirty!
349353
end
350354

355+
def restore_transactions
356+
return false unless restorable?
357+
358+
@stack.each(&:restore!)
359+
360+
materialize_transactions unless @lazy_transactions_enabled
361+
362+
true
363+
end
364+
365+
def restorable?
366+
@stack.none?(&:dirty?)
367+
end
368+
351369
def materialize_transactions
352370
return if @materializing_transactions
353371

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ def initialize(connection, logger = nil, config = {}) # :nodoc:
113113

114114
@default_timezone = self.class.validate_default_timezone(config[:default_timezone])
115115

116+
@raw_connection_dirty = false
117+
116118
configure_connection
117119
end
118120

@@ -555,10 +557,11 @@ def active?
555557
# new connection with the database. Implementors should call super
556558
# immediately after establishing the new connection (and while still
557559
# holding @lock).
558-
def reconnect!
559-
clear_cache!(new_connection: true)
560-
reset_transaction
561-
configure_connection
560+
def reconnect!(restore_transactions: false)
561+
reset_transaction(restore: restore_transactions) do
562+
clear_cache!(new_connection: true)
563+
configure_connection
564+
end
562565
end
563566

564567
# Disconnects from the database if already connected. Otherwise, this
@@ -637,6 +640,7 @@ def verify!
637640
# PostgreSQL's lo_* methods.
638641
def raw_connection
639642
disable_lazy_transactions!
643+
@raw_connection_dirty = true
640644
@raw_connection
641645
end
642646

@@ -789,6 +793,10 @@ def extract_limit(sql_type)
789793
EXTENDED_TYPE_MAPS = Concurrent::Map.new
790794

791795
private
796+
def reconnect_can_restore_state?
797+
transaction_manager.restorable? && !@raw_connection_dirty
798+
end
799+
792800
def extended_type_map_key
793801
if @default_timezone
794802
{ default_timezone: @default_timezone }

activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ def active?
123123
@raw_connection.ping
124124
end
125125

126-
def reconnect!
126+
def reconnect!(restore_transactions: false)
127127
@lock.synchronize do
128-
disconnect!
128+
@raw_connection.close
129129
connect
130130
super
131131
end

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ def reload_type_map # :nodoc:
322322
end
323323

324324
# Close then reopen the connection.
325-
def reconnect!
325+
def reconnect!(restore_transactions: false)
326326
@lock.synchronize do
327327
begin
328328
@raw_connection.reset

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ def active?
163163
!@raw_connection.closed?
164164
end
165165

166-
def reconnect!
166+
def reconnect!(restore_transactions: false)
167167
@lock.synchronize do
168168
if active?
169169
@raw_connection.rollback rescue nil

activerecord/test/cases/adapter_test.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,23 @@ def setup
394394
assert_not raw_transaction_open?(@connection)
395395
end
396396

397+
test "materialized transaction state can be restored after a reconnect" do
398+
@connection.begin_transaction
399+
assert_predicate @connection, :transaction_open?
400+
# +materialize_transactions+ currently automatically dirties the
401+
# connection, which would make it unrestorable
402+
@connection.transaction_manager.stub(:dirty_current_transaction, nil) do
403+
@connection.materialize_transactions
404+
end
405+
assert raw_transaction_open?(@connection)
406+
@connection.reconnect!(restore_transactions: true)
407+
assert_predicate @connection, :transaction_open?
408+
assert_not raw_transaction_open?(@connection)
409+
ensure
410+
@connection.reconnect!
411+
assert_not_predicate @connection, :transaction_open?
412+
end
413+
397414
test "materialized transaction state is reset after a disconnect" do
398415
@connection.begin_transaction
399416
assert_predicate @connection, :transaction_open?
@@ -417,6 +434,21 @@ def setup
417434
assert_not raw_transaction_open?(@connection)
418435
end
419436

437+
test "unmaterialized transaction state can be restored after a reconnect" do
438+
@connection.begin_transaction
439+
assert_predicate @connection, :transaction_open?
440+
assert_not raw_transaction_open?(@connection)
441+
@connection.reconnect!(restore_transactions: true)
442+
assert_predicate @connection, :transaction_open?
443+
assert_not raw_transaction_open?(@connection)
444+
@connection.materialize_transactions
445+
assert raw_transaction_open?(@connection)
446+
ensure
447+
@connection.reconnect!
448+
assert_not_predicate @connection, :transaction_open?
449+
assert_not raw_transaction_open?(@connection)
450+
end
451+
420452
test "unmaterialized transaction state is reset after a disconnect" do
421453
@connection.begin_transaction
422454
assert_predicate @connection, :transaction_open?

0 commit comments

Comments
 (0)