Permalink
Browse files

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:

     mperham/sidekiq#3166

We can just keep track of the connection as part of the exector state.
  • Loading branch information...
sj26 authored and matthewd committed Sep 30, 2016
1 parent 3b50fb6 commit fa7efca553e325b2aabb087a4eddf4560c356094
Showing with 27 additions and 4 deletions.
  1. +4 −4 activerecord/lib/active_record/query_cache.rb
  2. +23 −0 activerecord/test/cases/query_cache_test.rb
@@ -28,12 +28,12 @@ def self.run
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!
@@ -51,6 +51,29 @@ def test_exceptional_middleware_clears_and_disables_cache_on_error
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!

1 comment on commit fa7efca

@matthewd

This comment has been minimized.

Show comment
Hide comment
@matthewd

matthewd Oct 27, 2016

Member

Backported to 5-0-stable: e90a3c7

Member

matthewd commented on fa7efca Oct 27, 2016

Backported to 5-0-stable: e90a3c7

Please sign in to comment.