From 045713ee240fff815edb5962b25d668512649478 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 4 Sep 2008 12:17:56 +0200 Subject: PostgreSQL: introduce transaction_active? rather than tracking activity ourselves --- .../connection_adapters/postgresql_adapter.rb | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0c2532f21d..e95db1d9d8 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -518,6 +518,38 @@ module ActiveRecord execute "ROLLBACK" end + # ruby-pg defines Ruby constants for transaction status, + # ruby-postgres does not. + PQTRANS_IDLE = defined?(PGconn::PQTRANS_IDLE) ? PGconn::PQTRANS_IDLE : 0 + + # Check whether a transaction is active. + def transaction_active? + @connection.transaction_status != PQTRANS_IDLE + end + + # Wrap a block in a transaction. Returns result of block. + def transaction(start_db_transaction = true) + begin + if block_given? + begin_db_transaction if start_db_transaction + yield + end + rescue Exception => database_transaction_rollback + rollback_db_transaction if transaction_active? + raise unless database_transaction_rollback.is_a? ActiveRecord::Rollback + end + ensure + if transaction_active? + begin + commit_db_transaction + rescue Exception => database_transaction_rollback + rollback_db_transaction + raise + end + end + end + + # SCHEMA STATEMENTS ======================================== def recreate_database(name) #:nodoc: -- cgit v1.2.3 From 039d78a7d81c8ac49dcb6ba0304d32af671d56c9 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 4 Sep 2008 13:17:00 +0200 Subject: still need to track whether we're the toplevel transaction --- .../active_record/connection_adapters/postgresql_adapter.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index e95db1d9d8..bebab5d05d 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -529,17 +529,24 @@ module ActiveRecord # Wrap a block in a transaction. Returns result of block. def transaction(start_db_transaction = true) + transaction_open = false begin if block_given? - begin_db_transaction if start_db_transaction + if start_db_transaction + begin_db_transaction + transaction_open = true + end yield end rescue Exception => database_transaction_rollback - rollback_db_transaction if transaction_active? + if transaction_open && transaction_active? + transaction_open = false + rollback_db_transaction + end raise unless database_transaction_rollback.is_a? ActiveRecord::Rollback end ensure - if transaction_active? + if transaction_open && transaction_active? begin commit_db_transaction rescue Exception => database_transaction_rollback -- cgit v1.2.3 From f54be2cb3105856dc1fb31afc6c9012a91e53823 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 4 Sep 2008 13:24:55 +0200 Subject: Fix transaction exception test --- activerecord/test/cases/transactions_test.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activerecord') diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 8383ba58e9..a8016cd984 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -216,6 +216,7 @@ class TransactionTest < ActiveRecord::TestCase uses_mocha 'mocking connection.commit_db_transaction' do def test_rollback_when_commit_raises Topic.connection.expects(:begin_db_transaction) + Topic.connection.expects(:transaction_active?).returns(true) if current_adapter?(:PostgreSQLAdapter) Topic.connection.expects(:commit_db_transaction).raises('OH NOES') Topic.connection.expects(:rollback_db_transaction) -- cgit v1.2.3 From 7ba28726150f5d4ee3b3cb676d6b73cc3b62e186 Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Thu, 4 Sep 2008 14:03:19 +0200 Subject: Deprecate verification_timeout and verify before reset Signed-off-by: Michael Koziarski --- .../connection_adapters/abstract/connection_pool.rb | 6 ++---- .../abstract/connection_specification.rb | 15 ++++++++++----- .../active_record/connection_adapters/abstract_adapter.rb | 12 ++++-------- activerecord/test/cases/base_test.rb | 2 +- activerecord/test/cases/connection_test_mysql.rb | 2 +- 5 files changed, 18 insertions(+), 19 deletions(-) (limited to 'activerecord') 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 838b0434b0..6b4bc2574d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -35,7 +35,6 @@ module ActiveRecord # * +wait_timeout+: number of seconds to block and wait for a connection # before giving up and raising a timeout error (default 5 seconds). class ConnectionPool - delegate :verification_timeout, :to => "::ActiveRecord::Base" attr_reader :spec def initialize(spec) @@ -60,7 +59,6 @@ module ActiveRecord # held in a hash keyed by the thread id. def connection if conn = @reserved_connections[current_connection_id] - conn.verify!(verification_timeout) conn else @reserved_connections[current_connection_id] = checkout @@ -118,7 +116,7 @@ module ActiveRecord def verify_active_connections! #:nodoc: clear_stale_cached_connections! @connections.each do |connection| - connection.verify!(verification_timeout) + connection.verify! end end @@ -200,8 +198,8 @@ module ActiveRecord end def checkout_and_verify(c) + c.verify! c.run_callbacks :checkout - c.verify!(verification_timeout) @checked_out << c c end 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 417a333aab..a968fc0fd3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -7,11 +7,6 @@ module ActiveRecord end end - # Check for activity after at least +verification_timeout+ seconds. - # Defaults to 0 (always check.) - cattr_accessor :verification_timeout, :instance_writer => false - @@verification_timeout = 0 - # The connection handler cattr_accessor :connection_handler, :instance_writer => false @@connection_handler = ConnectionAdapters::ConnectionHandler.new @@ -101,6 +96,16 @@ module ActiveRecord 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 verification_timeout + ActiveSupport::Deprecation.warn("ActiveRecord::Base.verification_timeout has been deprecated and no longer has any effect. Please remove all references to verification_timeout.") + end + + # Deprecated and no longer has any effect. + def verification_timeout=(flag) + ActiveSupport::Deprecation.warn("ActiveRecord::Base.verification_timeout= has been deprecated and no longer has any effect. Please remove all references to verification_timeout=.") + 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 005be9d72f..ad6e2171d1 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -123,14 +123,10 @@ module ActiveRecord false end - # Lazily verify this connection, calling active? only if it - # hasn't been called for +timeout+ seconds. - def verify!(timeout) - now = Time.now.to_i - if (now - @last_verification) > timeout - reconnect! unless active? - @last_verification = now - end + # Verify this connection by calling active? and reconnecting if + # the connection is no longer active. + def verify!(*ignored) + reconnect! unless active? end # Provides access to the underlying database connection. Useful for diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index ac9081e003..67358fedd6 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, :schema_format, :verification_timeout, :lock_optimistically, :record_timestamps].each do |method| + :default_timezone, :schema_format, :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/connection_test_mysql.rb b/activerecord/test/cases/connection_test_mysql.rb index 1adbf18e73..40ddcf5420 100644 --- a/activerecord/test/cases/connection_test_mysql.rb +++ b/activerecord/test/cases/connection_test_mysql.rb @@ -24,7 +24,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase assert @connection.active? @connection.update('set @@wait_timeout=1') sleep 2 - @connection.verify!(0) + @connection.verify! assert @connection.active? end end -- cgit v1.2.3 From a3f12f575d4bf216a15188ecab2d26a11162bc3b Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Thu, 4 Sep 2008 15:16:29 +0200 Subject: Default connection allow_concurrency to false (for PostgreSQL) Signed-off-by: Jeremy Kemper --- .../connection_adapters/abstract/connection_pool.rb | 3 +-- activerecord/lib/active_record/test_case.rb | 15 +++++++++++++++ activerecord/test/cases/locking_test.rb | 2 ++ activerecord/test/cases/transactions_test.rb | 2 ++ 4 files changed, 20 insertions(+), 2 deletions(-) (limited to 'activerecord') 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 6b4bc2574d..5806dea061 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -164,8 +164,7 @@ module ActiveRecord private def new_connection - config = spec.config.reverse_merge(:allow_concurrency => true) - ActiveRecord::Base.send(spec.adapter_method, config) + ActiveRecord::Base.send(spec.adapter_method, spec.config) end def current_connection_id #:nodoc: diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb index ffaa41282f..eabf06fc3b 100644 --- a/activerecord/lib/active_record/test_case.rb +++ b/activerecord/lib/active_record/test_case.rb @@ -43,5 +43,20 @@ module ActiveRecord def assert_no_queries(&block) assert_queries(0, &block) end + + def self.use_concurrent_connections + setup :connection_allow_concurrency_setup + teardown :connection_allow_concurrency_teardown + end + + def connection_allow_concurrency_setup + @connection = ActiveRecord::Base.remove_connection + ActiveRecord::Base.establish_connection(@connection.merge({:allow_concurrency => true})) + end + + def connection_allow_concurrency_teardown + ActiveRecord::Base.clear_all_connections! + ActiveRecord::Base.establish_connection(@connection) + end end end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index bbe8582466..0a14b1d906 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -257,6 +257,8 @@ unless current_adapter?(:SQLServerAdapter, :SybaseAdapter, :OpenBaseAdapter) end if current_adapter?(:PostgreSQLAdapter, :OracleAdapter) + use_concurrent_connections + def test_no_locks_no_wait first, second = duel { Person.find 1 } assert first.end > second.end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index a8016cd984..b12ec36455 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -284,6 +284,8 @@ end if current_adapter?(:PostgreSQLAdapter) class ConcurrentTransactionTest < TransactionTest + use_concurrent_connections + # 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 From 0d9e238cc938c96708d36bbd2cd4736e9ec93a1e Mon Sep 17 00:00:00 2001 From: Nick Sieger Date: Thu, 4 Sep 2008 15:35:55 +0200 Subject: Remove flawed execute("ROLLBACK") approach; #reset! defaults to nothing Will need community help to fill out what #reset! should do for each adapter Signed-off-by: Jeremy Kemper --- .../lib/active_record/connection_adapters/abstract_adapter.rb | 4 +--- activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index ad6e2171d1..14dde57fa5 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -112,9 +112,7 @@ module ActiveRecord # ROLLBACK and swallows any exceptions which is probably not enough to # ensure the connection is clean. def reset! - silence_stderr do # postgres prints on stderr when you do this w/o a txn - execute "ROLLBACK" rescue nil - end + # this should be overridden by concrete adapters end # Returns true if its safe to reload the connection between requests for development mode. diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 14c76ac455..f1d13698c3 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -286,8 +286,6 @@ module ActiveRecord # reset the connection is to change the user to the same user. @connection.change_user(@config[:username], @config[:password], @config[:database]) configure_connection - else - super end end -- cgit v1.2.3 From ca5ffd10b9749ef0d55d7b5b08e2e7820fc2f8a5 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Thu, 4 Sep 2008 16:58:44 +0200 Subject: Handle connection timeouts with a slightly nicer error message. --- .../lib/active_record/connection_adapters/abstract/connection_pool.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') 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 5806dea061..54a39db1eb 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -145,7 +145,7 @@ module ActiveRecord if @queue.wait(@timeout) checkout_existing_connection else - raise ConnectionTimeoutError, "could not obtain a database connection in a timely fashion" + raise ConnectionTimeoutError, "could not obtain a database connection within #{@timeout} seconds. The pool size is currently #{@size}, perhaps you need to increase it?" end end end -- cgit v1.2.3 From cd498e25887cafc5d2b0d427b3f87af9f648aff5 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Thu, 4 Sep 2008 17:20:07 +0200 Subject: Rescue spurious failures in case dummy postgresql user or schema already exists --- activerecord/test/cases/schema_authorization_test_postgresql.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/schema_authorization_test_postgresql.rb b/activerecord/test/cases/schema_authorization_test_postgresql.rb index 7a0796ef53..ba7754513d 100644 --- a/activerecord/test/cases/schema_authorization_test_postgresql.rb +++ b/activerecord/test/cases/schema_authorization_test_postgresql.rb @@ -18,8 +18,8 @@ class SchemaAuthorizationTest < ActiveRecord::TestCase @connection.execute "SET search_path TO '$user',public" set_session_auth USERS.each do |u| - @connection.execute "CREATE USER #{u}" - @connection.execute "CREATE SCHEMA AUTHORIZATION #{u}" + @connection.execute "CREATE USER #{u}" rescue nil + @connection.execute "CREATE SCHEMA AUTHORIZATION #{u}" rescue nil set_session_auth u @connection.execute "CREATE TABLE #{TABLE_NAME} (#{COLUMNS.join(',')})" @connection.execute "INSERT INTO #{TABLE_NAME} (name) VALUES ('#{u}')" -- cgit v1.2.3 From de0e7507de826bf77fd57cc2a51e27388cb62ff7 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 5 Sep 2008 14:22:10 +0200 Subject: Changed all benchmarking reports to be in milliseconds --- activerecord/CHANGELOG | 2 ++ activerecord/lib/active_record/base.rb | 2 +- .../lib/active_record/connection_adapters/abstract_adapter.rb | 6 +++--- 3 files changed, 6 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 58d0669770..f6b9913790 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Changed benchmarks to be reported in milliseconds [DHH] + * Connection pooling. #936 [Nick Sieger] * Merge scoped :joins together instead of overwriting them. May expose scoping bugs in your code! #501 [Andrew White] diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 3419aad580..cd66844dae 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1313,7 +1313,7 @@ module ActiveRecord #:nodoc: if logger && logger.level <= log_level result = nil seconds = Benchmark.realtime { result = use_silence ? silence { yield } : yield } - logger.add(log_level, "#{title} (#{'%.5f' % seconds})") + logger.add(log_level, "#{title} (#{'%.2f' % (seconds * 1000)}ms)") result else yield diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 14dde57fa5..a3cb6f3214 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -147,10 +147,10 @@ module ActiveRecord @open_transactions -= 1 end - def log_info(sql, name, runtime) + def log_info(sql, name, seconds) if @logger && @logger.debug? - name = "#{name.nil? ? "SQL" : name} (#{sprintf("%f", runtime)})" - @logger.debug format_log_entry(name, sql.squeeze(' ')) + name = "#{name.nil? ? "SQL" : name} (#{sprintf("%.2f", seconds * 1000)}ms)" + @logger.debug(format_log_entry(name, sql.squeeze(' '))) end end -- cgit v1.2.3 From 227ee2ecb46f1609938a83ed82abde1a45ebe2eb Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 5 Sep 2008 14:58:34 +0200 Subject: Use a more sensible resolution on the new millisecond benchmarks --- activerecord/lib/active_record/base.rb | 2 +- activerecord/lib/active_record/connection_adapters/abstract_adapter.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index cd66844dae..907495a416 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1313,7 +1313,7 @@ module ActiveRecord #:nodoc: if logger && logger.level <= log_level result = nil seconds = Benchmark.realtime { result = use_silence ? silence { yield } : yield } - logger.add(log_level, "#{title} (#{'%.2f' % (seconds * 1000)}ms)") + logger.add(log_level, "#{title} (#{'%.1f' % (seconds * 1000)}ms)") result else yield diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index a3cb6f3214..7c37916367 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -149,7 +149,7 @@ module ActiveRecord def log_info(sql, name, seconds) if @logger && @logger.debug? - name = "#{name.nil? ? "SQL" : name} (#{sprintf("%.2f", seconds * 1000)}ms)" + name = "#{name.nil? ? "SQL" : name} (#{sprintf("%.1f", seconds * 1000)}ms)" @logger.debug(format_log_entry(name, sql.squeeze(' '))) end end -- cgit v1.2.3