New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce new ActiveRecord transaction error classes #25107

Merged
merged 1 commit into from Aug 3, 2016

Conversation

Projects
None yet
6 participants
@Erol
Contributor

Erol commented May 22, 2016

Summary

This is a WIP follow-up to #25093.

I went ahead with @jeremy's recommendation of introducing ActiveRecord::TransactionRollbackError, with subclasses to distinguish between a serialization failure or deadlock.

Checked around, and it looks like MySQL is just returning an Error: 1213 SQLSTATE: 40001, whereas PostgreSQL tries to distinguish between serialization failures and deadlocks. Given the following test scenarios:

thread = Thread.new do
  Sample.transaction isolation: :serializable do
    Sample.delete_all

    10.times do |i|
      sleep 0.1

      Sample.create value: i
    end
  end
end

sleep 0.1

Sample.transaction isolation: :serializable do
  Sample.delete_all

  10.times do |i|
    sleep 0.1

    Sample.create value: i
  end
end

thread.join
s1 = Sample.create value: 1
s2 = Sample.create value: 2

thread = Thread.new do
  Sample.transaction do
    s1.lock!
    sleep 1
    s2.update_attributes value: 1
  end
end

sleep 0.5

Sample.transaction do
  s2.lock!
  sleep 1
  s1.update_attributes value: 2
end

thread.join

PostgreSQL returns 40001 for the first scenario and 40P01 for the second. MySQL returns 40001 for both.

Also checked MS SQL Server and Oracle and - like MySQL - they only have error mappings for 40001, which they use for serialization failures.

Other Information:

PostgreSQL: http://www.postgresql.org/docs/devel/static/errcodes-appendix.html
MySQL 5.7: https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html
MS SQL Server: https://msdn.microsoft.com/en-us/library/ms714687.aspx
Oracle Database 12c: https://docs.oracle.com/database/121/ZZMOD/appd.htm#ZZMOD338

@rails-bot

This comment has been minimized.

Show comment
Hide comment
@rails-bot

rails-bot May 22, 2016

r? @arthurnn

(@rails-bot has picked a reviewer for you, use r? to override)

rails-bot commented May 22, 2016

r? @arthurnn

(@rails-bot has picked a reviewer for you, use r? to override)

@Erol

This comment has been minimized.

Show comment
Hide comment
@Erol

Erol May 22, 2016

Contributor

r? @jeremy

Contributor

Erol commented May 22, 2016

r? @jeremy

@rails-bot rails-bot assigned jeremy and unassigned arthurnn May 22, 2016

@Erol

This comment has been minimized.

Show comment
Hide comment
@Erol

Erol May 22, 2016

Contributor

Also did a quick check and it seems like the MS SQL Server adapter is only translating deadlock errors - which is also 40001 - via a custom ActiveRecord::DeadlockVictim exception. The Oracle Enhanced adapter does not translate any form of transaction errors at all.

@jeremy Your thoughts? Can we proceed with this current set of changes?

Contributor

Erol commented May 22, 2016

Also did a quick check and it seems like the MS SQL Server adapter is only translating deadlock errors - which is also 40001 - via a custom ActiveRecord::DeadlockVictim exception. The Oracle Enhanced adapter does not translate any form of transaction errors at all.

@jeremy Your thoughts? Can we proceed with this current set of changes?

@Erol Erol changed the title from [WIP] Introduce new ActiveRecord transaction error classes to Introduce new ActiveRecord transaction error classes May 23, 2016

@Erol

This comment has been minimized.

Show comment
Hide comment
@Erol

Erol May 30, 2016

Contributor

Just wanted to ask. Any feedback on this?

Contributor

Erol commented May 30, 2016

Just wanted to ask. Any feedback on this?

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Aug 3, 2016

Member

Perfect investigation. Thank you so much.

Member

rafaelfranca commented Aug 3, 2016

Perfect investigation. Thank you so much.

@rafaelfranca rafaelfranca merged commit f5c0c7f into rails:master Aug 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

rafaelfranca added a commit that referenced this pull request Aug 3, 2016

Merge pull request #25107 from Erol/introduce-new-ar-transaction-erro…
…r-classes

Introduce new ActiveRecord transaction error classes

Closes #26018

y-yagi added a commit to y-yagi/rails that referenced this pull request Aug 20, 2016

correct exception class in `retry_on` example [ci skip]
If the deadlock has occurred `ActiveRecord::Deadlocked` will raise.
Ref: rails#25107, rails#26059

y-yagi added a commit to y-yagi/rails that referenced this pull request Oct 8, 2016

correct exception class in `retry_on` example [ci skip]
If the deadlock has occurred `ActiveRecord::Deadlocked` will raise.
Ref: rails#25107, rails#26059

@Erol Erol deleted the Erol:introduce-new-ar-transaction-error-classes branch Oct 24, 2016

y-yagi added a commit to y-yagi/rails that referenced this pull request Mar 19, 2017

Fix changelog entry about transaction error classes [ci skip]
`ActiveRecord::TransactionSerializationError` was introduces in rails#25093.
However, refactored in rails#25107, `TransactionSerializationError` is gone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment