From 0d57afd69883ba35e7f2d89eee365f7c40b8be3e Mon Sep 17 00:00:00 2001
From: abicky <takeshi.arabiki@gmail.com>
Date: Tue, 1 Oct 2024 00:05:19 +0900
Subject: [PATCH] Fix flaky test

This commit fixes the flaky test
"ActiveRecord::DebugErrors::DisplayConnectionOwners#execute when
ActiveRecord::LockWaitTimeout occurs displays transactions and
processlist."
I suspect there is more than 1 second time delay between the start
time of the two threads, and CyclicBarrier shortens the delay.
---
 .../abstract_mysql_adapter_spec.rb            | 26 +++++++++----------
 .../connection_pool_spec.rb                   | 17 ++++++------
 spec/support/cyclic_barrier.rb                | 21 +++++++++++++++
 3 files changed, 42 insertions(+), 22 deletions(-)
 create mode 100644 spec/support/cyclic_barrier.rb

diff --git a/spec/activerecord/debug_errors/ext/connection_adapters/abstract_mysql_adapter_spec.rb b/spec/activerecord/debug_errors/ext/connection_adapters/abstract_mysql_adapter_spec.rb
index 44f03ac..d76df70 100644
--- a/spec/activerecord/debug_errors/ext/connection_adapters/abstract_mysql_adapter_spec.rb
+++ b/spec/activerecord/debug_errors/ext/connection_adapters/abstract_mysql_adapter_spec.rb
@@ -1,4 +1,5 @@
 require "spec_helper"
+require "support/cyclic_barrier"
 
 RSpec.describe ActiveRecord::DebugErrors::DisplayConnectionOwners do
   let(:log) { StringIO.new }
@@ -19,12 +20,14 @@
   describe "#execute" do
     context "when ActiveRecord::Deadlocked occurs" do
       def cause_deadlock(role:)
+        barrier = CyclicBarrier.new(2)
+
         ths = []
         ths << Thread.new do
           ActiveRecord::Base.connected_to(role: role) do
             User.transaction do
               User.lock.find_by!(name: 'foo')
-              sleep 0.1
+              barrier.await(1)
               User.lock.find_by!(name: 'bar')
             end
           end
@@ -34,7 +37,7 @@ def cause_deadlock(role:)
           ActiveRecord::Base.connected_to(role: role) do
             User.transaction do
               User.lock.find_by!(name: 'bar')
-              sleep 0.1
+              barrier.await(1)
               User.lock.find_by!(name: 'foo')
             end
           end
@@ -67,18 +70,15 @@ def cause_deadlock(role:)
 
     context "when ActiveRecord::LockWaitTimeout occurs" do
       it "displays transactions and processlist" do
-        ths = []
-        ths << Thread.new do
-          User.transaction do
-            User.lock.find_by!(name: 'foo')
-            sleep 2
-          end
-        end
+        barrier = CyclicBarrier.new(2)
 
-        ths << Thread.new do
-          User.transaction do
-            User.lock.find_by!(name: 'foo')
-            sleep 2
+        ths = Array.new(2) do
+          Thread.new do
+            User.transaction do
+              barrier.await(1)
+              User.lock.find_by!(name: 'foo')
+              sleep 2
+            end
           end
         end
 
diff --git a/spec/activerecord/debug_errors/ext/connection_adapters/connection_pool_spec.rb b/spec/activerecord/debug_errors/ext/connection_adapters/connection_pool_spec.rb
index 8f5b932..556bf4d 100644
--- a/spec/activerecord/debug_errors/ext/connection_adapters/connection_pool_spec.rb
+++ b/spec/activerecord/debug_errors/ext/connection_adapters/connection_pool_spec.rb
@@ -1,4 +1,5 @@
 require "spec_helper"
+require "support/cyclic_barrier"
 
 RSpec.describe ActiveRecord::DebugErrors::DisplayConnectionOwners do
   let(:log) { StringIO.new }
@@ -20,20 +21,18 @@
     it "displays connection owners and other threads" do
       Thread.new { sleep 10 } # another thread
 
-      mutex = Mutex.new
-      cv = ConditionVariable.new
+      barrier = CyclicBarrier.new(ActiveRecord::Base.connection_pool.size)
 
       expect {
         ActiveRecord::Base.connection # Ensure to acquire a connection
         Array.new(ActiveRecord::Base.connection_pool.size) do
           Thread.new do
-            mutex.synchronize do
-              ActiveRecord::Base.connection_pool.checkout(0.1)
-              cv.wait(mutex, 1)
-            rescue
-              cv.broadcast
-              raise
-            end
+            ActiveRecord::Base.connection_pool.checkout(0.1)
+            barrier.await(1)
+          rescue Timeout::Error
+            # CyclicBarrier#await is expected to raise Timeout::Error
+            # because it is not called ActiveRecord::Base.connection_pool.size times
+            # due to ActiveRecord::ConnectionTimeoutError
           end
         end.each(&:join)
       }.to raise_error(ActiveRecord::ConnectionTimeoutError)
diff --git a/spec/support/cyclic_barrier.rb b/spec/support/cyclic_barrier.rb
new file mode 100644
index 0000000..a588f83
--- /dev/null
+++ b/spec/support/cyclic_barrier.rb
@@ -0,0 +1,21 @@
+# This class is a simple implementation of CyclicBarrier in Java
+class CyclicBarrier
+  def initialize(parties)
+    @cv = ConditionVariable.new
+    @mutex = Mutex.new
+    @parties = parties
+    @number_waiting = 0
+  end
+
+  def await(timeout = nil)
+    @mutex.synchronize do
+      @number_waiting += 1
+      if @number_waiting == @parties
+        @cv.broadcast
+      else
+        @cv.wait(@mutex, timeout)
+        raise Timeout::Error if @number_waiting != @parties
+      end
+    end
+  end
+end