Invalidate all query caches for current thread#35089
Conversation
5a27799 to
be4b1a4
Compare
There was a problem hiding this comment.
I don't think we can iterate over pool.connections here without holding the pool's lock.
Regardless, we don't need to clear the query cache on all connections; we only need read-your-writes consistency for the current thread. If it's acceptable for a concurrent request in another process to see stale data, it should be acceptable for a concurrent request in another thread, too.
| pool.connections.each(&:clear_query_cache!) | |
| pool.connection.clear_query_cache! if pool.active_connection? |
There was a problem hiding this comment.
👍
Beyond it being acceptable, having one thread's wipe out every cache in the system would be not-great for hit rates / performance.
Along that line, I think I'd actually go further, and limit this to only the current-thread-owned connection in the peer pools -- the corresponding same-named pool in other handlers -- not all pools.
There was a problem hiding this comment.
Yea this clearly blows up in the tests but also @eugeneius version doesn't pass either.
I'm starting to get really frustrated with the query cache. It seems like it just wasn't built for multiple connections and we may need to rethink it sooner rather than later.
There was a problem hiding this comment.
Ok I found a fix I think and I'm not proud of it but the tests pass locally. I'm not sure what consequences to test for at this point if the tests pass on CI.
30c6f9b to
cdf6aa2
Compare
|
@tenderlove and I looked at this today and came up with a different solution. We've fixed the issue by adding a new method that clears the query cache for all the connections but only for the current thread. The reason the thread test was failing before with @eugeneius's suggested change was because when we call |
This change ensures that all query cahces are cleared across all connections per handler for the current thread so if you write on one connection the read will have the query cache cleared.
cdf6aa2 to
183c0eb
Compare
This change ensures that all query caches are cleared across connection
handlers and pools so that if you write on one connection the read
connection will have the update that occurred.
cc/ @kamipo as followup to your concerns in #35073
cc/ @tenderlove