Skip to content

Batch etsource cache reads with fetch_multi to reduce N+1 queries#1748

Open
aaccensi wants to merge 3 commits into
masterfrom
warm-cache-etsource-classes
Open

Batch etsource cache reads with fetch_multi to reduce N+1 queries#1748
aaccensi wants to merge 3 commits into
masterfrom
warm-cache-etsource-classes

Conversation

@aaccensi
Copy link
Copy Markdown
Member

@aaccensi aaccensi commented May 11, 2026

Context

Since adopting Solid Cache, several controller actions that trigger a GQL calculation are flagged by Sentry as N+1 queries. This was not visible before because Memcached round-trips are cheap and don't use SQL; with Solid Cache every cache operation is a SQL query. The fix implemented is cache-backend agnostic.

Implemented changes

MeritOrder collapses 6 individual cache fetches into one fetch_multi call. Molecules does the same for its 2 keys. Both use Thread.current to ensure fetch_multi runs at most once per request.

Notice that Fever and Reconciliation were not changed: Fever has a single key so batching adds no value, and Reconciliation has a dependency between its two keys (reconciliation_hash is computed using reconciliation_carriers, so they cannot be fetched in a single fetch_multi)

Related

Closes #1747

Checklist

  • I have tested these changes (I believe this cannot be tester properly locally)
  • I have updated documentation as needed
  • I have tagged the relevant people for review

@aaccensi aaccensi requested review from louispt1 and noracato May 11, 2026 15:32
MeritOrder collapses 6 individual cache fetches into one fetch_multi call.
Molecules does the same for its 2 keys. Both use Thread.current to ensure
fetch_multi runs at most once per request.

Closes #1747
@aaccensi aaccensi force-pushed the warm-cache-etsource-classes branch from 985694c to 673325d Compare May 18, 2026 07:08
aaccensi and others added 2 commits May 18, 2026 09:11
Atlas data only changes on ETSource import, so per-request scoping is
wasteful. Class-level memoization is reset via NastyCache#expire_local!
whenever an import clears Rails.cache.

Closes #1747
@louispt1
Copy link
Copy Markdown
Member

I've looked at this a couple of times but I am not confident about it without more investigation.
This cache means that the merit order is preserved for the lifetime of a rails thread, I guess I'm concerned the cache won't be invalidated correctly? But not sure how to test/verify this. I see that this gem notes the issue and provides a solution, but I am not entirely sure we will even face the issue to begin with. I think we may need to test this carefully

I also see as I was typing this you've updated the cache clearance @aaccensi so maybe all moot now - will take a look later on

@aaccensi
Copy link
Copy Markdown
Member Author

I was looking at the situation with stale data that we should not get after an ETSource import that the solid cache investigation flagged and decided to try to factor also this in here as a test making it more optimal.

@louispt1
Copy link
Copy Markdown
Member

I've looked at this a couple of times but I am not confident about it without more investigation. This cache means that the merit order is preserved for the lifetime of a rails thread, I guess I'm concerned the cache won't be invalidated correctly? But not sure how to test/verify this. I see that this gem notes the issue and provides a solution, but I am not entirely sure we will even face the issue to begin with. I think we may need to test this carefully

I also see as I was typing this you've updated the cache clearance @aaccensi so maybe all moot now - will take a look later on

Discussed with @aaccensi and I understand the approach now - I was a little confused before but the approach makes sense to me now and seems practical!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

N+1 Cache Queries During GQL Initialization

2 participants