chore(spanner): add capacity limit and CLOCK eviction to KeyRecipeCache - #6507
chore(spanner): add capacity limit and CLOCK eviction to KeyRecipeCache#6507olavloite wants to merge 1 commit into
Conversation
olavloite
commented
Aug 22, 2026
- Bound query key recipes to prevent unbounded memory growth from dynamic ad-hoc queries.
- Implement scan-resistant CLOCK (Second-Chance) eviction using lock-free atomic reference tracking on shared read lookups.
- Remove redundant inner Arc wrapper in KeyRecipeCache to align with workspace synchronization standards.
- Add unit tests verifying capacity constraints, second-chance survival, and scan resistance against query floods.
There was a problem hiding this comment.
Code Review
This pull request introduces a bounded CLOCK (Second-Chance) eviction mechanism for SQL query recipes in the KeyRecipeCache to prevent unbounded memory growth. It replaces the outer Arc wrapping of RecipeStore with a direct RwLock, establishes a default query capacity of 2,000, and tracks referenced entries using lock-free atomic stores. Feedback on the changes suggests an improvement to the insert_query function: when overwriting an existing query recipe, the referenced bit should be set to true to prevent the freshly updated entry from being prematurely evicted. A refactored implementation using the Entry API was provided to handle this cleanly.
- Bound query key recipes to prevent unbounded memory growth from dynamic ad-hoc queries. - Implement scan-resistant CLOCK (Second-Chance) eviction using lock-free atomic reference tracking on shared read lookups. - Remove redundant inner Arc wrapper in KeyRecipeCache to align with workspace synchronization standards. - Add unit tests verifying capacity constraints, second-chance survival, and scan resistance against query floods.
0cbcd4a to
fa58fdb
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a bounded CLOCK (Second-Chance) eviction cache for SQL query recipes in KeyRecipeCache to prevent unbounded memory growth. It introduces a capacity limit (defaulting to 2,000) and tracks query references using lock-free atomic flags. Feedback on the implementation highlights a potential "lost insert" bug under high cache load, where newly inserted entries initialized with referenced: false can be immediately evicted during the same insertion cycle. Initializing new entries with referenced: true is recommended to resolve this issue.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6507 +/- ##
==========================================
- Coverage 96.39% 96.39% -0.01%
==========================================
Files 301 301
Lines 84762 84980 +218
==========================================
+ Hits 81710 81920 +210
- Misses 3052 3060 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|