Skip to content

Commit

Permalink
fix: transaction state on rollback
Browse files Browse the repository at this point in the history
Fixes #258
  • Loading branch information
BuonOmo committed Jan 31, 2025
1 parent 56a37d8 commit 5cfdfa0
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 1 deletion.
5 changes: 4 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,7 @@ jobs:
done
cat ${{ github.workspace }}/setup.sql | cockroach sql --insecure
- name: Test
run: bundle exec rake test TESTOPTS='--profile=5'
run: bundle exec rake test
env:
TESTOPTS: "--profile=5"
RAILS_MINITEST_PLUGIN: "1" # Make sure that we use the minitest plugin for profiling.
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,27 @@ def within_new_transaction(isolation: nil, joinable: true, attempts: 0)
within_new_transaction(isolation: isolation, joinable: joinable, attempts: attempts + 1) { yield }
end

# OVERRIDE: the `rescue ActiveRecord::StatementInvalid` block is new, see comment.
def rollback_transaction(transaction = nil)
@connection.lock.synchronize do
transaction ||= @stack.last
begin
transaction.rollback
rescue ActiveRecord::StatementInvalid => err
# This is important to make Active Record aware the record was not inserted/saved
# Otherwise Active Record will assume save was successful and it doesn't retry the transaction
# See this thread for more details:
# https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/258#issuecomment-2256633329
transaction.rollback_records if err.cause.is_a?(PG::NoActiveSqlTransaction)

raise
ensure
@stack.pop if @stack.last == transaction
end
transaction.rollback_records
end
end

def retryable?(error)
return true if serialization_error?(error)
return true if error.is_a? ActiveRecord::SerializationFailure
Expand Down
48 changes: 48 additions & 0 deletions test/cases/transactions_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# frozen_string_literal: true

require "cases/helper_cockroachdb"

module CockroachDB
class TransactionsTest < ActiveRecord::TestCase
self.use_transactional_tests = false

class Avenger < ActiveRecord::Base
singleton_class.attr_accessor :cyclic_barrier

validate :validate_unique_username

def validate_unique_username
self.class.cyclic_barrier.wait
duplicate = self.class.where(name: name).any?
errors.add("Duplicate username!") if duplicate
end
end

def test_concurrent_insert_with_processes
conn = ActiveRecord::Base.lease_connection
conn.create_table :avengers, force: true do |t|
t.string :name
end
ActiveRecord::Base.reset_column_information

avengers = %w[Hulk Thor Loki]
Avenger.cyclic_barrier = Concurrent::CyclicBarrier.new(avengers.size - 1)
Thread.current[:name] = "Main" # For debug logs.

assert_queries_match(/ROLLBACK/) do # Ensure we are properly testing the retry mechanism.
avengers.map do |name|
Thread.fork do
Thread.current[:name] = name # For debug logs.
Avenger.create!(name: name)
end
end.each(&:join)
end

assert_equal avengers.size, Avenger.count
ensure
Thread.current[:name] = nil
conn = ActiveRecord::Base.lease_connection
conn.drop_table :avengers, if_exists: true
end
end
end

0 comments on commit 5cfdfa0

Please sign in to comment.