Test postgres read committed mode if work on Sequel.

[zw963]

I consider this is not a issue, so create discuss here.

I want to test following feature works with Sequel.

https://www.postgresql.org/docs/11/transaction-iso.html#XACT-READ-COMMITTED

Following one file runner is a attempt, but get raise error.

https://gist.github.com/4bacc4500f25cd2723c024d03d5faef6

Following is error output.

I, [2022-01-15T18:34:04.332844 #401108]  INFO -- : (0.000156s) BEGIN
I, [2022-01-15T18:34:04.333108 #401108]  INFO -- : (0.000122s) SAVEPOINT autopoint_1
I, [2022-01-15T18:34:04.333561 #401108]  INFO -- : (0.000128s) SAVEPOINT autopoint_2
I, [2022-01-15T18:34:04.334239 #401108]  INFO -- : (0.000433s) INSERT INTO "insiders" ("number_of_shares") VALUES (200) RETURNING *
I, [2022-01-15T18:34:04.334448 #401108]  INFO -- : (0.000119s) RELEASE SAVEPOINT autopoint_2
I, [2022-01-15T18:34:04.335042 #401108]  INFO -- : (0.000360s) SELECT count(*) AS "count" FROM "insiders" LIMIT 1
Insider count outer thread: 1
I, [2022-01-15T18:34:04.476604 #401108]  INFO -- : (0.000496s) SET standard_conforming_strings = ON
I, [2022-01-15T18:34:04.477129 #401108]  INFO -- : (0.000306s) SET client_min_messages = 'WARNING'
I, [2022-01-15T18:34:04.477514 #401108]  INFO -- : (0.000235s) SET DateStyle = 'ISO'
I, [2022-01-15T18:34:04.477883 #401108]  INFO -- : (0.000180s) BEGIN
I, [2022-01-15T18:34:04.479598 #401108]  INFO -- : (0.001514s) SELECT count(*) AS "count" FROM "insiders" LIMIT 1
Insider count inner first thread: 0
I, [2022-01-15T18:34:04.481243 #401108]  INFO -- : (0.000956s) SELECT * FROM "insiders" ORDER BY "id" DESC LIMIT 1
I, [2022-01-15T18:34:04.481913 #401108]  INFO -- : (0.000352s) ROLLBACK
#<Thread:0x00007f1e2c077820 test.rb:58 run> terminated with exception (report_on_exception is true):
test.rb:63:in `block (4 levels) in <main>': undefined method `number_of_shares' for nil:NilClass (NoMethodError)

As you can see, i even can't saw the created Insider object in the new created thread, because transaction? i guess.

What i want is just want to ensure the same effect as following test which run on ActiveRecord 6 is works on Sequel too.

require 'test_helper'

class UserTransactionTest < ActiveSupport::TestCase
  test 'Postgres Read Committed mode default lock' do
    user = User.create!(name: 'user001', password: '123456', account: 'user001')
    UserTransaction.create(user: user, type: :admin_deposit, amount: 100)

    thread1 = Thread.new do
      sleep rand(0.1..0.2)
      ActiveRecord::Base.transaction do
        user_transaction_1 = UserTransaction.last
        user_transaction_1.update(amount: user_transaction_1.amount - 10)
      end
    end

    thread2 = Thread.new do
      sleep rand(0.1..0.2)
      ActiveRecord::Base.transaction do
        user_transaction_2 = UserTransaction.last
        user_transaction_2.update(amount: user_transaction_2.amount - 10)
      end
    end

    thread1.join
    thread2.join

    assert_equal 80.0, UserTransaction.last.amount.to_f
  end
end

above test works on rails 6.1, and run in a transaction too, because i can check this use self.use_transactional_tests? method like this:

class ActiveSupport::TestCase
   puts self.use_transactional_tests? # => true
end

---

So, following is my question:

1. How to fix above test to make it work as ActiveRecord?
2. How should i test if postgres read committed mode works on sequel?

Thank you.

[zw963]

BTW:

if test above code without use transaction, instead, i just use database_cleaner-sequel, set policy to :truncation, those test works, but i consider this is not same as rails 6 running context, and will slow down my project test speed.

https://gist.github.com/56c95fa75d22a6d134730485a0b0f08a

anyway, i switch from database_cleaner to use minitest-hooks + transaction instead, as we discuss on jeremyevans/minitest-hooks#21, i don't want introduce unnecessary gems even only use it on my test, so, i am curious if i can test this feature
within transaction.

Thank you.

[janko]

When you create a new thread, database queries inside it will use a new database connection. Since the database connection assigned to the main thread hasn't yet committed the insertion of the insider record (that happens at the end of the around_all hook), database connections inside the two threads don't see the record. It doesn't matter that the threads were created inside a database transaction, they will still use a new database connection that only sees committed changes.

The Active Record example works, because Active Record uses a single shared connection in tests, which is used by all threads (including the web server thread running in the background during system tests). To prevent race conditions in situations where two threads try to use the connection at the same time, Active Record internally adds mutex locking around any code that uses the connection.

With Sequel you could use the single-threaded connection pool, but then you'd have to ensure that two threads don't access the shared connection at the same time. When I make the changes below to your example, it completes successfully. FWIW, there is some more discussion regarding this use case in this thread.

changes

diff --git a/test.rb b/test.rb
index 6c457da..3c4e3dc 100644
--- a/test.rb
+++ b/test.rb
@@ -15,11 +15,10 @@ gemfile(true) do
   gem 'minitest-hooks'
 end
 
-system("sudo -u postgres dropdb --if-exists check_sequel_db")
-system("sudo -u postgres createdb check_sequel_db")
+system("dropdb --if-exists check_sequel_db")
+system("createdb check_sequel_db")
 
-DB_URL="postgres://postgres:postgres@localhost:5432/check_sequel_db"
-DB = Sequel.connect(DB_URL)
+DB = Sequel.postgres("check_sequel_db", single_threaded: true)
 
 DB.create_table(:insiders, :ignore_index_errors=>true) do
   primary_key :id
@@ -52,23 +51,28 @@ DB.loggers << LOGGER
 describe 'insider' do
   it 'test postgres read committed mode default lock' do
     Insider.create(number_of_shares: 200)
+    mutex = Mutex.new
 
     puts "Insider count outer thread: #{Insider.count}"
 
     thread1 = Thread.new do
       sleep rand(0.1..0.2)
-      DB.transaction do
-        puts "Insider count inner first thread: #{Insider.count}"
-        insider_1 = Insider.last
-        insider_1.update(number_of_shares: insider_1.number_of_shares - 10)
+      mutex.synchronize do
+        DB.transaction do
+          puts "Insider count inner first thread: #{Insider.count}"
+          insider_1 = Insider.last
+          insider_1.update(number_of_shares: insider_1.number_of_shares - 10)
+        end
       end
     end
 
     thread2 = Thread.new do
       sleep rand(0.1..0.2)
-      DB.transaction do
-        insider_2 = Insider.last
-        insider_2.update(number_of_shares: insider_2.number_of_shares - 10)
+      mutex.synchronize do
+        DB.transaction do
+          insider_2 = Insider.last
+          insider_2.update(number_of_shares: insider_2.number_of_shares - 10)
+        end
       end
     end

[zw963]

Sequel already supports transaction isolation levels, and has for many years: https://sequel.jeremyevans.net/rdoc/files/doc/transactions_rdoc.html#label-Transaction+Isolation+Levels

Thank you, i read it.

Then, sorry for disturb again, because this answer still not solve the issue in my heart.

When use sequel with postgres + default read committed mode, we assume we have on a very high concurrently multi-thread production environment, we assume following code will happen synchronously occasionally, i want to know, if we still need add a Mutex lock when update number_of_shares for update correctly as @janko 's above example if we have been use DB.transaction, and all query and update process both wrapped by DB.transaction ?

    DB.transaction do
        insider_1 = Insider.last # we assume insider was exist on DB for now.
        insider_1.update(number_of_shares: insider_1.number_of_shares - 10) # <= if this code is safe?
      end

[janko]

@zw963 That mutex was only related to the single-threaded connection pool, which you'd only potentially want to use for local testing. But, as we saw, even in an isolated test a race condition happened, I suggest you to use the default threaded connection pool, in which case no mutexes are needed.

Note that for your example to be concurrency-safe, you'd want to do the increment in SQL:

insider_1.update(number_of_shares: Sequel[:number_of_shares] - 10)

[zw963]

insider_1.update(number_of_shares: Sequel[:number_of_shares] - 10)

Sorry, i don't aware what this Sequel[:number_of_shares] means at current case, number_of_shares is a field name, what is Sequel[:number_of_shares] stand for?

[janko]

It's a way to build SQL expressions in Sequel. Sequel[:number_of_shares] - 10 translates to number_of_shares - 10 in SQL. If you don't do this, a decrement could happen in between fetching the insider object and doing the update, which would result in a "lost" decrement. This is the classic example of race conditions.

[zw963]

If you don't do this, a decrement could happen in between fetching the insider object and doing the update, which would result in a "lost" decrement. This is the classic example of race conditions.

Okay, i understand, after change from number_of_shares to Sequel[:number_of_shares] will avoid race condition, still have one question, this guaranteed by sequel or postgresql MVCC ?

Thank you.