Skip to content

Commit 49c9cf8

Browse files
committed
Store query caches in IsolatedExecutionState to avoid memory bloat
Fix: rails#52617 Closes: rails#52618 It isn't per say a leak because the connection reaper eventually prune these, and also it's not expected that new Threads are constantly making entries into that cache. But it's true that with Fiber it's probably a bit more common, and the default reaper frequency isn't adapted to clear this fast enough. So instead of waiting for the reaper to trigger, which may take a long time, we keep the caches in IsolatedExcutionState so that when the owning thread or fiber is garbage collected its cache is too. However since we need to be able to clear all threads caches, we keep a cache version on the pool, and bump it to invalidate all caches of a pool at once.
1 parent cc2c43b commit 49c9cf8

File tree

3 files changed

+28
-19
lines changed

3 files changed

+28
-19
lines changed

activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -630,8 +630,6 @@ def reap
630630
remove conn
631631
end
632632
end
633-
634-
prune_thread_cache
635633
end
636634

637635
# Disconnect all connections that have been idle for at least

activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,27 @@ class Store # :nodoc:
3535
alias_method :enabled?, :enabled
3636
alias_method :dirties?, :dirties
3737

38-
def initialize(max_size)
38+
def initialize(pool, max_size)
39+
@pool = pool
40+
@version = pool.query_cache_version
3941
@map = {}
4042
@max_size = max_size
4143
@enabled = false
4244
@dirties = true
4345
end
4446

4547
def size
48+
check_version
4649
@map.size
4750
end
4851

4952
def empty?
53+
check_version
5054
@map.empty?
5155
end
5256

5357
def [](key)
58+
check_version
5459
return unless @enabled
5560

5661
if entry = @map.delete(key)
@@ -59,6 +64,7 @@ def [](key)
5964
end
6065

6166
def compute_if_absent(key)
67+
check_version
6268
return yield unless @enabled
6369

6470
if entry = @map.delete(key)
@@ -76,12 +82,21 @@ def clear
7682
@map.clear
7783
self
7884
end
85+
86+
private
87+
def check_version
88+
version = @pool.query_cache_version
89+
if version != @version
90+
@map.clear
91+
@version = version
92+
end
93+
end
7994
end
8095

8196
module ConnectionPoolConfiguration # :nodoc:
8297
def initialize(...)
8398
super
84-
@thread_query_caches = Concurrent::Map.new(initial_capacity: @size)
99+
@query_cache_version = 0
85100
@query_cache_max_size = \
86101
case query_cache = db_config&.query_cache
87102
when 0, false
@@ -93,6 +108,10 @@ def initialize(...)
93108
end
94109
end
95110

111+
def query_cache_version
112+
synchronize { @query_cache_version }
113+
end
114+
96115
def checkout_and_verify(connection)
97116
super
98117
connection.query_cache ||= query_cache
@@ -141,25 +160,17 @@ def clear_query_cache
141160
# With transactional fixtures, and especially systems test
142161
# another thread may use the same connection, but with a different
143162
# query cache. So we must clear them all.
144-
@thread_query_caches.each_value(&:clear)
145-
else
146-
query_cache.clear
163+
synchronize do
164+
@query_cache_version += 1
165+
end
147166
end
167+
query_cache.clear
148168
end
149169

150170
def query_cache
151-
@thread_query_caches.compute_if_absent(ActiveSupport::IsolatedExecutionState.context) do
152-
Store.new(@query_cache_max_size)
153-
end
171+
key = :"active_record_query_cache_#{object_id}"
172+
ActiveSupport::IsolatedExecutionState[key] ||= Store.new(self, @query_cache_max_size)
154173
end
155-
156-
private
157-
def prune_thread_cache
158-
dead_threads = @thread_query_caches.keys.reject(&:alive?)
159-
dead_threads.each do |dead_thread|
160-
@thread_query_caches.delete(dead_thread)
161-
end
162-
end
163174
end
164175

165176
attr_accessor :query_cache

activerecord/test/cases/query_cache_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,7 @@ def test_cache_is_expired_by_habtm_delete
10261026
end
10271027

10281028
def test_query_cache_lru_eviction
1029-
store = ActiveRecord::ConnectionAdapters::QueryCache::Store.new(2)
1029+
store = ActiveRecord::ConnectionAdapters::QueryCache::Store.new(ActiveRecord::Base.connection_pool, 2)
10301030
store.enabled = true
10311031

10321032
connection = Post.lease_connection

0 commit comments

Comments
 (0)