Skip to content
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

Prevent race conditions on postgres #16

Conversation

samsondav
Copy link
Contributor

Tested with 50 simultaneous connections. This 100% guarantees correctness of the sequential_id in Postgres and makes no change to the previous behavior for other database adapters.

Unfortunately this is not so trivial to implement in MySQL because table locks must be manually released.

@samsondav
Copy link
Contributor Author

Here is the test code:

Model

class SequentialTest < ActiveRecord::Base
    acts_as_sequenced scope: :sequential_scope_id
end

Migration

class CreateSequentialTests < ActiveRecord::Migration
  def change
    create_table :sequential_tests do |t|
      t.integer :sequential_id, null: false
      t.integer :sequential_scope_id, null: false, default: 1
    end

    add_index :sequential_tests, [:sequential_id, :sequential_scope_id], unique: true
  end
end

Test

THREADS = 50
def test
  SequentialTest.destroy_all
  initial = 0

  threads = (1..THREADS).map do |i|
    Thread.new do
      ActiveRecord::Base.connection_pool.with_connection do
        SequentialTest.create!
      end
    end
  end

  threads.each {|t| t.join}

  final = SequentialTest.pluck('max(sequential_id)')[0]

  fail('not concurrent-safe') unless final == initial + THREADS
end

@samsondav samsondav force-pushed the sam/manage_concurrency_using_table_locks branch 4 times, most recently from 0f308e0 to b98beaa Compare November 20, 2015 01:05
@samsondav
Copy link
Contributor Author

@djreimer I enabled postgresql for your test suite and wrote a test case. It fails without the locking, and succeeds with locking enabled.

Travis CI will automatically run all tests on both databases. You can run the test manually using postgres like this: DB=postgresql rake test

@samsondav samsondav force-pushed the sam/manage_concurrency_using_table_locks branch from 8a77a94 to 50ec373 Compare November 20, 2015 20:22
@samsondav samsondav force-pushed the sam/manage_concurrency_using_table_locks branch from 50ec373 to 96b2f70 Compare November 20, 2015 20:29
@samsondav samsondav mentioned this pull request Nov 20, 2015
@derrickreimer
Copy link
Owner

Rock on! 🤘 This is great. I want to run some tests on my local machine, but looks like you've covered all the bases. This should be merged soon. Thanks!

@samsondav
Copy link
Contributor Author

@djreimer Hey man. Let me know if there's anything you need from me or comments you have on the code that I can fix 👍

@derrickreimer
Copy link
Owner

Cool will do. This is in on my todo list for a lazy post-Thanksgiving Friday

On Friday, November 27, 2015, Sam Philip notifications@github.com wrote:

@djreimer https://github.com/djreimer Hey man. Let me know if there's
anything you need from me or comments you have on the code that I can fix [image:
👍]


Reply to this email directly or view it on GitHub
#16 (comment).

Derrick

derrickreimer added a commit that referenced this pull request Nov 28, 2015
…able_locks

Prevent race conditions on postgres
@derrickreimer derrickreimer merged commit da3d266 into derrickreimer:master Nov 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants