PERF: avoid spinning a thread each time we close a connection

PERF: avoid spinning a thread each time we close a connection

This is a temporary workaround for the issue in https://github.com/rails/rails/pull/36949

Discussing a proper fix in Rails with the Rails team.

Prior to this fix we were spinning up a thread every time we closed a connection to the db.

diff --git a/lib/freedom_patches/rails6.rb b/lib/freedom_patches/rails6.rb
new file mode 100644
index 0000000..06029e1
--- /dev/null
+++ b/lib/freedom_patches/rails6.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+# see: https://github.com/rails/rails/pull/36949#issuecomment-530698779
+#
+# Without this patch each time we close a DB connection we spin a thread
+
+class ::ActiveRecord::ConnectionAdapters::AbstractAdapter
+  class StaticThreadLocalVar
+    attr_reader :value
+
+    def initialize(value)
+      @value = value
+    end
+
+    def bind(value)
+      raise "attempting to change immutable local var" if value != @value
+    end
+  end
+
+  # we have no choice but to perform an aggressive patch here
+  # if we simply hook the method we will still call a finalizer
+  # on Concurrent::ThreadLocalVar
+
+  def initialize(connection, logger = nil, config = {}) # :nodoc:
+    super()
+
+    @connection          = connection
+    @owner               = nil
+    @instrumenter        = ActiveSupport::Notifications.instrumenter
+    @logger              = logger
+    @config              = config
+    @pool                = ActiveRecord::ConnectionAdapters::NullPool.new
+    @idle_since          = Concurrent.monotonic_time
+    @visitor = arel_visitor
+    @statements = build_statement_pool
+    @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new
+
+    if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
+      @prepared_statement_status = Concurrent::ThreadLocalVar.new(true)
+      @visitor.extend(DetermineIfPreparableVisitor)
+    else
+      @prepared_statement_status = StaticThreadLocalVar.new(false)
+    end
+
+    @advisory_locks_enabled = self.class.type_cast_config_to_boolean(
+      config.fetch(:advisory_locks, true)
+    )
+  end
+
+end
diff --git a/script/thread_detective.rb b/script/thread_detective.rb
new file mode 100644
index 0000000..c654158
--- /dev/null
+++ b/script/thread_detective.rb
@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+class Thread
+  attr_accessor :origin
+end
+
+class ThreadDetective
+  def self.test_thread
+    Thread.new { sleep 1 }
+  end
+  def self.start(max_threads)
+    @thread ||= Thread.new do
+      self.new.monitor(max_threads)
+    end
+
+    @trace = TracePoint.new(:thread_begin) do |tp|
+      Thread.current.origin = Thread.current.inspect
+    end
+    @trace.enable
+  end
+
+  def self.stop
+    @thread&.kill
+    @thread = nil
+    @trace&.disable
+    @trace.stop
+  end
+
+  def monitor(max_threads)
+    STDERR.puts "Monitoring threads in #{Process.pid}"
+
+    while true
+      threads = Thread.list
+
+      if threads.length > max_threads
+        str = +("-" * 60)
+        str << "#{threads.length} found in Process #{Process.pid}!\n"
+
+        threads.each do |thread|
+          str << "\n"
+          if thread.origin
+            str << thread.origin
+          else
+            str << thread.inspect
+          end
+          str << "\n"
+        end
+        str << ("-" * 60)
+
+        STDERR.puts str
+      end
+      sleep 1
+    end
+  end
+
+end

GitHub sha: 015051ec

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

DEV: bind for thread local vars should yield block