Skip to content

Commit

Permalink
Instrument Active Record transactions
Browse files Browse the repository at this point in the history
Tracking Active Record-managed transactions seems to be a common need,
but there's currently not a great way to do it. Here's a few examples
I've seen:

* GitHub has custom transaction tracking that monkey patches the Active
  Record `TransactionManager` and `RealTransaction`. We use the tracking
  to prevent opening a transaction to one database cluster inside a
  transaction to a different database cluster, and to report slow
  transactions (we get slow transaction data directly from MySQL as well,
  but it's still helpful to report from the application with backtraces to
  help track them down).
* https://github.com/palkan/isolator tracks transactions to prevent
  non-atomic interactions like external network calls inside a
  transaction. The gem works by subscribing to `sql.active_record`, then
  piecing together the transactions by looking for `BEGIN`, `COMMIT`,
  `SAVEPOINT`, etc., but this is unreliable:
    - palkan/isolator#65
    - palkan/isolator#64
* It looks like GitLab patches `TransactionManager` and `RealTransaction`
  to track nested savepoints. See palkan/isolator#46

This commit adds a new `transaction.active_record` event that should
provide a more reliable solution for these various use cases. It
includes the connection in the payload (useful, for example, in
differentiating transactions to different databases), but if this change
gets merged we're also planning to add details about what type of
transaction it is (savepoint or real) and what the outcome is (commit,
rollback, restarted, errored).

This instrumentation needs to start and finish at fairly specific times:
  - start on materialize
  - finish after committing or rolling back, but before the after_commit
    or after_rollback callbacks
  - finish and start again when the transaction restarts (at least for
    real transactions—we've done it for savepoints as well but I'm not
    certain we should)
  - ensure it finishes if commit and rollback fail (e.g. if the
    connection goes away)
To make all that work, this commit uses the lower-level `#build-handle`
API instead of `#instrument`.

Co-authored-by: Ian Candy <ipc103@github.com>
  • Loading branch information
composerinteralia and ipc103 committed Sep 12, 2023
1 parent c9f4b1e commit ce49fa9
Show file tree
Hide file tree
Showing 3 changed files with 337 additions and 2 deletions.
14 changes: 14 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,17 @@
* Add instrumentation for Active Record transactions

Allows subscribing to transaction events for tracking/instrumentation. The event payload contains the connection, as well as timing details.

```ruby
ActiveSupport::Notifications.subscribe("transaction.active_record") do |event|
puts "Transaction event occurred!"
connection = event.payload[:connection]
puts "Connection: #{connection.inspect}"
end
```

*Daniel Colson*, *Ian Candy*

* Support composite foreign keys via migration helpers.

```ruby
Expand Down
Expand Up @@ -74,6 +74,29 @@ def nullify!
end
end

class TransactionInstrumenter
def initialize(payload = {})
@handle = nil
@started = false
@payload = payload
end

def start
return if @started
@started = true

@handle = ActiveSupport::Notifications.instrumenter.build_handle("transaction.active_record", @payload)
@handle.start
end

def finish
return unless @started
@started = false

@handle.finish
end
end

class NullTransaction # :nodoc:
def initialize; end
def state; end
Expand Down Expand Up @@ -104,6 +127,7 @@ def initialize(connection, isolation: nil, joinable: true, run_commit_callbacks:
@run_commit_callbacks = run_commit_callbacks
@lazy_enrollment_records = nil
@dirty = false
@instrumenter = TransactionInstrumenter.new(connection: connection)
end

def dirty!
Expand Down Expand Up @@ -138,8 +162,13 @@ def restartable?
joinable? && !dirty?
end

def incomplete!
@instrumenter.finish
end

def materialize!
@materialized = true
@instrumenter.start
end

def materialized?
Expand Down Expand Up @@ -303,19 +332,26 @@ def materialize!
end

def restart
connection.rollback_to_savepoint(savepoint_name) if materialized?
return unless materialized?

@instrumenter.finish
@instrumenter.start

connection.rollback_to_savepoint(savepoint_name)
end

def rollback
unless @state.invalidated?
connection.rollback_to_savepoint(savepoint_name) if materialized?
end
@state.rollback!
@instrumenter.finish
end

def commit
connection.release_savepoint(savepoint_name) if materialized?
@state.commit!
@instrumenter.finish
end

def full_rollback?; false; end
Expand All @@ -336,7 +372,10 @@ def materialize!
def restart
return unless materialized?

@instrumenter.finish

if connection.supports_restart_db_transaction?
@instrumenter.start
connection.restart_db_transaction
else
connection.rollback_db_transaction
Expand All @@ -347,11 +386,13 @@ def restart
def rollback
connection.rollback_db_transaction if materialized?
@state.full_rollback!
@instrumenter.finish
end

def commit
connection.commit_db_transaction if materialized?
@state.full_commit!
@instrumenter.finish
end
end

Expand Down Expand Up @@ -526,7 +567,10 @@ def within_new_transaction(isolation: nil, joinable: true)
end
end
ensure
@connection.throw_away! unless transaction&.state&.completed?
unless transaction&.state&.completed?
@connection.throw_away!
transaction&.incomplete!
end
end
end

Expand Down

0 comments on commit ce49fa9

Please sign in to comment.