fix(lapis): expire siloQueryCache entries 30h after last access - #1853
fix(lapis): expire siloQueryCache entries 30h after last access#1853fhennig wants to merge 1 commit into
Conversation
The query cache was only ever cleared on a SILO data version change, which can be days apart. Add a 30h idle expiry so heap held by one-off queries (e.g. an analytics script sweeping many filter combinations) is released instead of lingering until the next data update. This is a minimal, low-risk step. Bounding the cache by the memory size of its entries instead of by entry count is tracked separately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q5mqrj8VN12QZfxqGNEb1d
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟢 Approval recommended
The focused configuration change is valid and covered by an appropriate test.
Pull request overview
Adds idle expiration to the SILO query cache, preventing one-off query results from persisting until the next data update.
Changes:
- Configures cache entries to expire 30 hours after last access.
- Adds an integration test verifying the expiration policy.
File summaries
| File | Description |
|---|---|
lapis/src/main/resources/application.properties |
Adds the 30-hour idle expiration policy. |
lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryCacheConfigurationTest.kt |
Verifies the configured expiration duration. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fengelniederhammer
left a comment
There was a problem hiding this comment.
Why do we need to expire those queries? In my understanding those are evicted anyway when the cache is full and new entries come in.
|
Ok, we decided to not do this, as we're not sure if it actually helps or by how much, and we'd rather keep it simple for now. Ideally we'd use a weighted cache later on, so we can get rid of the |
The query cache was only ever cleared on a SILO data version change, which can be days apart. Add a 30h idle expiry so heap held by one-off queries (e.g. an analytics script sweeping many filter combinations) is released instead of lingering until the next data update.
This is a minimal, low-risk step. Bounding the cache by the memory size of its entries instead of by entry count is tracked separately.
PR Checklist
llms.txt.