Skip to content

chore(spanner): add capacity limit and CLOCK eviction to KeyRecipeCache - #6507

Merged
olavloite merged 1 commit into
googleapis:mainfrom
olavloite:spanner-key-recipe-cache-limit
Aug 26, 2026
Merged

chore(spanner): add capacity limit and CLOCK eviction to KeyRecipeCache#6507
olavloite merged 1 commit into
googleapis:mainfrom
olavloite:spanner-key-recipe-cache-limit

Conversation

@olavloite

Copy link
Copy Markdown
Contributor
  • 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.

@olavloite
olavloite requested review from a team as code owners August 22, 2026 11:29
@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Aug 22, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/spanner/src/routing/key_recipe_cache.rs
- 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.
@olavloite
olavloite force-pushed the spanner-key-recipe-cache-limit branch from 0cbcd4a to fa58fdb Compare August 22, 2026 11:41
@olavloite

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/spanner/src/routing/key_recipe_cache.rs
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.28326% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.39%. Comparing base (4084332) to head (fa58fdb).

Files with missing lines Patch % Lines
src/spanner/src/routing/key_recipe_cache.rs 98.28% 4 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@olavloite
olavloite merged commit 22163f0 into googleapis:main Aug 26, 2026
42 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants