Merge commit 'mainstream/master'

13 files changed, 88 insertions, 33 deletions

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..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} (#{'%.5f' % seconds})")
+          logger.add(log_level, "#{title} (#{'%.1f' % (seconds * 1000)}ms)")
           result
         else
           yield
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..54a39db1eb 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
 
@@ -147,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
@@ -166,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:
@@ -200,8 +197,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..7c37916367 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.
@@ -123,14 +121,10 @@ module ActiveRecord
         false
       end
 
-      # Lazily verify this connection, calling <tt>active?</tt> 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 <tt>active?</tt> 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
@@ -153,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("%.1f", seconds * 1000)}ms)"
+          @logger.debug(format_log_entry(name, sql.squeeze(' ')))
         end
       end
 
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
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 0c2532f21d..bebab5d05d 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -518,6 +518,45 @@ 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)
+        transaction_open = false
+        begin
+          if block_given?
+            if start_db_transaction
+              begin_db_transaction
+              transaction_open = true
+            end
+            yield
+          end
+        rescue Exception => database_transaction_rollback
+          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_open && 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:
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/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
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/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}')"
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index 8383ba58e9..b12ec36455 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)
 
@@ -283,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