From ca6d71753f3a2e8a0a29108b7c55ba3b7c8cd943 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Fri, 22 Aug 2008 12:19:29 -0500 Subject: Deprecate allow_concurrency and make it have no effect --- activerecord/lib/active_record/base.rb | 20 +---------- .../abstract/connection_pool.rb | 25 +++---------- .../abstract/connection_specification.rb | 30 ++++++---------- .../connection_adapters/abstract_adapter.rb | 4 ++- activerecord/test/cases/base_test.rb | 2 +- activerecord/test/cases/locking_test.rb | 7 ---- .../test/cases/threaded_connections_test.rb | 42 ++++------------------ activerecord/test/cases/transactions_test.rb | 11 ------ 8 files changed, 27 insertions(+), 114 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index b5ffc471bc..bc6d61301f 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -452,13 +452,6 @@ module ActiveRecord #:nodoc: cattr_accessor :default_timezone, :instance_writer => false @@default_timezone = :local - # Determines whether to use a connection for each thread, or a single shared connection for all threads. - # Defaults to false. If you're writing a threaded application, set to true - # and periodically call verify_active_connections! to clear out connections - # assigned to stale threads. - cattr_accessor :allow_concurrency, :instance_writer => false - @@allow_concurrency = false - # Specifies the format to use when dumping the database schema with Rails' # Rakefile. If :sql, the schema is dumped as (potentially database- # specific) SQL statements. If :ruby, the schema is dumped as an @@ -1943,22 +1936,11 @@ module ActiveRecord #:nodoc: end end - def thread_safe_scoped_methods #:nodoc: + def scoped_methods #:nodoc: scoped_methods = (Thread.current[:scoped_methods] ||= {}) scoped_methods[self] ||= [] end - def single_threaded_scoped_methods #:nodoc: - @scoped_methods ||= [] - end - - # pick up the correct scoped_methods version from @@allow_concurrency - if @@allow_concurrency - alias_method :scoped_methods, :thread_safe_scoped_methods - else - alias_method :scoped_methods, :single_threaded_scoped_methods - end - def current_scoped_methods #:nodoc: scoped_methods.last end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 365c80fe1d..04c8361c64 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -154,7 +154,7 @@ module ActiveRecord private def new_connection - config = spec.config.reverse_merge(:allow_concurrency => ActiveRecord::Base.allow_concurrency) + config = spec.config.reverse_merge(:allow_concurrency => true) ActiveRecord::Base.send(spec.adapter_method, config) end @@ -285,9 +285,12 @@ module ActiveRecord end end - module ConnectionHandlerMethods + class ConnectionHandler + attr_reader :connection_pools_lock + def initialize(pools = {}) @connection_pools = pools + @connection_pools_lock = Monitor.new end def connection_pools @@ -361,24 +364,6 @@ module ActiveRecord klass = klass.superclass end end - end - - # This connection handler is not thread-safe, as it does not protect access - # to the underlying connection pools. - class SingleThreadConnectionHandler - include ConnectionHandlerMethods - end - - # This connection handler is thread-safe. Each access or modification of a thread - # pool is synchronized by an internal monitor. - class MultipleThreadConnectionHandler - attr_reader :connection_pools_lock - include ConnectionHandlerMethods - - def initialize(pools = {}) - super - @connection_pools_lock = Monitor.new - end # Apply monitor to all public methods that access the pool. synchronize :establish_connection, :retrieve_connection, diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 0910db1951..47fc11a620 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -14,25 +14,7 @@ module ActiveRecord # The connection handler cattr_accessor :connection_handler, :instance_writer => false - @@connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new - - # Turning on allow_concurrency changes the single threaded connection handler - # for a multiple threaded one, so that multi-threaded access of the - # connection pools is synchronized. - def self.allow_concurrency=(flag) - if @@allow_concurrency != flag - @@allow_concurrency = flag - # When switching connection handlers, preserve the existing pools so that - # #establish_connection doesn't need to be called again. - if @@allow_concurrency - self.connection_handler = ConnectionAdapters::MultipleThreadConnectionHandler.new( - self.connection_handler.connection_pools) - else - self.connection_handler = ConnectionAdapters::SingleThreadConnectionHandler.new( - self.connection_handler.connection_pools) - end - end - end + @@connection_handler = ConnectionAdapters::ConnectionHandler.new # Returns the connection currently associated with the class. This can # also be used to "borrow" the connection to do database work that isn't @@ -109,6 +91,16 @@ module ActiveRecord end class << self + # Deprecated and no longer has any effect. + def allow_concurrency + ActiveSupport::Deprecation.warn("ActiveRecord::Base.allow_concurrency has been deprecated and no longer has any effect. Please remove all references to allow_concurrency.") + end + + # Deprecated and no longer has any effect. + def allow_concurrency=(flag) + ActiveSupport::Deprecation.warn("ActiveRecord::Base.allow_concurrency= has been deprecated and no longer has any effect. Please remove all references to allow_concurrency=.") + end + # Returns the connection currently associated with the class. This can # also be used to "borrow" the connection to do database work unrelated # to any of the specific Active Records. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index c37ebf1410..7ef8834547 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -109,7 +109,9 @@ module ActiveRecord # ROLLBACK and swallows any exceptions which is probably not enough to # ensure the connection is clean. def reset! - execute "ROLLBACK" rescue nil + silence_stderr do # postgres prints on stderr when you do this w/o a txn + execute "ROLLBACK" rescue nil + end end # Returns true if its safe to reload the connection between requests for development mode. diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index c8111358e3..ac9081e003 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -880,7 +880,7 @@ class BasicsTest < ActiveRecord::TestCase def test_mass_assignment_protection_against_class_attribute_writers [:logger, :configurations, :primary_key_prefix_type, :table_name_prefix, :table_name_suffix, :pluralize_table_names, :colorize_logging, - :default_timezone, :allow_concurrency, :schema_format, :verification_timeout, :lock_optimistically, :record_timestamps].each do |method| + :default_timezone, :schema_format, :verification_timeout, :lock_optimistically, :record_timestamps].each do |method| assert Task.respond_to?(method) assert Task.respond_to?("#{method}=") assert Task.new.respond_to?(method) diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 701187223f..bbe8582466 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -210,13 +210,6 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter) def setup # Avoid introspection queries during tests. Person.columns; Reader.columns - - @allow_concurrency = ActiveRecord::Base.allow_concurrency - ActiveRecord::Base.allow_concurrency = true - end - - def teardown - ActiveRecord::Base.allow_concurrency = @allow_concurrency end # Test typical find. diff --git a/activerecord/test/cases/threaded_connections_test.rb b/activerecord/test/cases/threaded_connections_test.rb index 892a99f85a..7246a8a62b 100644 --- a/activerecord/test/cases/threaded_connections_test.rb +++ b/activerecord/test/cases/threaded_connections_test.rb @@ -4,53 +4,23 @@ require 'models/reply' unless %w(FrontBase).include? ActiveRecord::Base.connection.adapter_name class ThreadedConnectionsTest < ActiveRecord::TestCase - self.use_transactional_fixtures = false - - fixtures :topics - - def setup - @connection = ActiveRecord::Base.remove_connection - @connections = [] - @allow_concurrency = ActiveRecord::Base.allow_concurrency - ActiveRecord::Base.allow_concurrency = true - end - - def teardown - # clear the connection cache - ActiveRecord::Base.clear_active_connections! - # set allow_concurrency to saved value - ActiveRecord::Base.allow_concurrency = @allow_concurrency - # reestablish old connection - ActiveRecord::Base.establish_connection(@connection) - end - - def gather_connections - ActiveRecord::Base.establish_connection(@connection) - - 5.times do - Thread.new do - Topic.find :first - @connections << ActiveRecord::Base.active_connections.values.first - end.join + def test_allow_concurrency_is_deprecated + assert_deprecated('ActiveRecord::Base.allow_concurrency') do + ActiveRecord::Base.allow_concurrency + end + assert_deprecated('ActiveRecord::Base.allow_concurrency=') do + ActiveRecord::Base.allow_concurrency = true end - end - - def test_threaded_connections - gather_connections - assert_equal @connections.length, 5 end end class PooledConnectionsTest < ActiveRecord::TestCase def setup @connection = ActiveRecord::Base.remove_connection - @allow_concurrency = ActiveRecord::Base.allow_concurrency - ActiveRecord::Base.allow_concurrency = true end def teardown ActiveRecord::Base.clear_all_connections! - ActiveRecord::Base.allow_concurrency = @allow_concurrency ActiveRecord::Base.establish_connection(@connection) end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index af3ee6ddba..8383ba58e9 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -283,17 +283,6 @@ end if current_adapter?(:PostgreSQLAdapter) class ConcurrentTransactionTest < TransactionTest - def setup - @allow_concurrency = ActiveRecord::Base.allow_concurrency - ActiveRecord::Base.allow_concurrency = true - super - end - - def teardown - super - ActiveRecord::Base.allow_concurrency = @allow_concurrency - end - # This will cause transactions to overlap and fail unless they are performed on # separate database connections. def test_transaction_per_thread -- cgit v1.2.3