Skip to content

Commit

Permalink
PERF: avoid spinning a thread each time we close a connection
Browse files Browse the repository at this point in the history
This is a temporary workaround for the issue in rails/rails#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.
  • Loading branch information
SamSaffron committed Sep 12, 2019
1 parent f4f566a commit 015051e
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 0 deletions.
50 changes: 50 additions & 0 deletions 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
56 changes: 56 additions & 0 deletions 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

2 comments on commit 015051e

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

https://meta.discourse.org/t/upgrading-discourse-to-rails-6/128004/12

@discoursereviewbot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.