Skip to content

Commit

Permalink
Avoid expensive tracking objects for prepared statements
Browse files Browse the repository at this point in the history
Per #36949 we introduce a race condition fix for #36763

This refines the fix to avoid using Concurrent::ThreadLocalVar

The implementation in the concurrent lib is rather expensive, culminating in
a finalizer per object that spins off a thread to do cleanup work.

None of this expense is needed as we can simply implement
the desired behavior using Ruby primitives. Additionally this moves to a Fiber bound implementation vs a thread bound implementation, something that is not desired for this particular usage.
  • Loading branch information
SamSaffron authored and rafaelfranca committed Sep 16, 2019
1 parent 89314d0 commit 46cc85d
Showing 1 changed file with 12 additions and 5 deletions.
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require "set"
require "active_record/connection_adapters/determine_if_preparable_visitor"
require "active_record/connection_adapters/schema_cache"
require "active_record/connection_adapters/sql_type_metadata"
Expand All @@ -11,7 +12,6 @@
require "arel/collectors/composite"
require "arel/collectors/sql_string"
require "arel/collectors/substitute_binds"
require "concurrent/atomic/thread_local_var"

module ActiveRecord
module ConnectionAdapters # :nodoc:
Expand Down Expand Up @@ -130,10 +130,10 @@ def initialize(connection, logger = nil, config = {}) # :nodoc:
@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)
@prepared_statements = true
@visitor.extend(DetermineIfPreparableVisitor)
else
@prepared_statement_status = Concurrent::ThreadLocalVar.new(false)
@prepared_statements = false
end

@advisory_locks_enabled = self.class.type_cast_config_to_boolean(
Expand Down Expand Up @@ -177,7 +177,11 @@ def schema_migration # :nodoc:
end

def prepared_statements
@prepared_statement_status.value
@prepared_statements && !prepared_statements_disabled_cache.include?(object_id)
end

def prepared_statements_disabled_cache # :nodoc:
Thread.current[:ar_prepared_statements_disabled_cache] ||= Set.new
end

class Version
Expand Down Expand Up @@ -264,7 +268,10 @@ def seconds_idle # :nodoc:
end

def unprepared_statement
@prepared_statement_status.bind(false) { yield }
cache = prepared_statements_disabled_cache.add(object_id) if @prepared_statements
yield
ensure
cache&.delete(object_id)
end

# Returns the human-readable name of the adapter. Use mixed case - one
Expand Down

0 comments on commit 46cc85d

Please sign in to comment.