diff options
author | Samuel Cochran <sj26@sj26.com> | 2016-09-30 15:26:19 +1000 |
---|---|---|
committer | Matthew Draper <matthew@trebex.net> | 2016-10-26 15:42:23 -0500 |
commit | fa7efca553e325b2aabb087a4eddf4560c356094 (patch) | |
tree | 6397ac9dd2d763d62d33dbf1ca9244e1d74d958f | |
parent | 3b50fb6b2f413b4bfe638b3c9839fe7db5077f73 (diff) | |
download | rails-fa7efca553e325b2aabb087a4eddf4560c356094.tar.gz rails-fa7efca553e325b2aabb087a4eddf4560c356094.tar.bz2 rails-fa7efca553e325b2aabb087a4eddf4560c356094.zip |
Clear the correct query cache
This executor currently relies on `ActiveRecord::Base.connection` not
changing between `prepare` and `complete`. If something else returns
the current ActiveRecord connection to the pool early then this
`complete` call will fail to clear the correct query cache and restore
the original `query_cache_enabled` status.
This has for example been happening in Sidekiq:
https://github.com/mperham/sidekiq/pull/3166
We can just keep track of the connection as part of the exector state.
-rw-r--r-- | activerecord/lib/active_record/query_cache.rb | 8 | ||||
-rw-r--r-- | activerecord/test/cases/query_cache_test.rb | 23 |
2 files changed, 27 insertions, 4 deletions
diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb index c45c8c1697..c42c22ab09 100644 --- a/activerecord/lib/active_record/query_cache.rb +++ b/activerecord/lib/active_record/query_cache.rb @@ -28,12 +28,12 @@ module ActiveRecord enabled = connection.query_cache_enabled connection.enable_query_cache! - enabled + [connection, enabled] end - def self.complete(enabled) - ActiveRecord::Base.connection.clear_query_cache - ActiveRecord::Base.connection.disable_query_cache! unless enabled + def self.complete((connection, enabled)) + connection.clear_query_cache + connection.disable_query_cache! unless enabled unless ActiveRecord::Base.connected? && ActiveRecord::Base.connection.transaction_open? ActiveRecord::Base.clear_active_connections! diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 4cd258695d..29b2deea26 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -51,6 +51,29 @@ class QueryCacheTest < ActiveRecord::TestCase assert !ActiveRecord::Base.connection.query_cache_enabled, "cache off" end + def test_exceptional_middleware_cleans_up_correct_cache + connection = ActiveRecord::Base.connection + called = false + + mw = middleware { |env| + Task.find 1 + Task.find 1 + assert_equal 1, connection.query_cache.length + + # Checkin connection early + ActiveRecord::Base.clear_active_connections! + # Make sure ActiveRecord::Base.connection doesn't checkout the same connection + ActiveRecord::Base.connection_pool.remove(connection) + + called = true + } + mw.call({}) + + assert called + assert_equal 0, connection.query_cache.length + assert !connection.query_cache_enabled, "cache off" + end + def test_exceptional_middleware_leaves_enabled_cache_alone ActiveRecord::Base.connection.enable_query_cache! |