Skip to content

chore(spanner): implement key range cache for location-aware routing - #6215

Open
olavloite wants to merge 1 commit into
googleapis:mainfrom
olavloite:spanner-key-range-cache
Open

chore(spanner): implement key range cache for location-aware routing#6215
olavloite wants to merge 1 commit into
googleapis:mainfrom
olavloite:spanner-key-range-cache

Conversation

@olavloite

Copy link
Copy Markdown
Contributor

Add KeyRangeCache, an in-memory interval cache that maps table key range boundaries (start_key and limit_key) to Paxos replica groups and tablet endpoints for location-aware routing.

@olavloite
olavloite requested review from a team as code owners July 29, 2026 17:43
@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Jul 29, 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 an in-memory interval cache (KeyRangeCache) for Spanner location-aware routing, which maps table key range split boundaries to tablet replica groups and supports interval lookups under CoveringSplit and PickRandom routing modes. Feedback on the implementation suggests improving the tablet selection logic in select_tablet to prevent defeating zone-aware routing, avoid load imbalance, and handle fallback scenarios more optimally by tracking the minimum distance and utilizing reservoir sampling.

Comment thread src/spanner/src/routing/key_range_cache.rs Outdated
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.86364% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.07%. Comparing base (334b540) to head (5ef5c5b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/spanner/src/routing/key_range_cache.rs 98.86% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6215      +/-   ##
==========================================
+ Coverage   96.02%   96.07%   +0.05%     
==========================================
  Files         269      270       +1     
  Lines       67282    68426    +1144     
==========================================
+ Hits        64607    65741    +1134     
- Misses       2675     2685      +10     

☔ 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 force-pushed the spanner-key-range-cache branch from 4addb84 to d0a57b2 Compare July 29, 2026 18:03
@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 introduces a location-aware routing module for Spanner, implementing an in-memory interval cache (KeyRangeCache) to map table key range split boundaries to tablet replica groups. The review feedback highlights a critical bug in the cache implementation: keying the BTreeMap by limit_key causes the last split range (which has an empty limit_key representing infinity) to be incorrectly sorted as the smallest element, leading to cache misses and overlapping ranges. To resolve this, the reviewer suggests keying the map by start_key instead and provides refactored implementations for replace_range_if_newer_locked and find_range. Additionally, the reviewer recommends removing the unused MAX_SAME_REGION_DISTANCE constant.

Comment thread src/spanner/src/routing/key_range_cache.rs Outdated
Comment thread src/spanner/src/routing/key_range_cache.rs
Comment thread src/spanner/src/routing/key_range_cache.rs
Comment thread src/spanner/src/routing/key_range_cache.rs Outdated
@olavloite
olavloite force-pushed the spanner-key-range-cache branch from d0a57b2 to dfb34e3 Compare July 29, 2026 18:24
@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 introduces KeyRangeCache to support location-aware routing in Spanner by caching split boundaries and mapping them to replica groups. The feedback highlights a critical correctness bug in the find_range method under RangeMode::PickRandom when the requested key is out of range, which can lead to incorrect routing or false gap detection. Additionally, a documentation correction is needed for the ranges map in CacheState to accurately reflect that it maps start_key rather than limit_key.

Comment thread src/spanner/src/routing/key_range_cache.rs
Comment thread src/spanner/src/routing/key_range_cache.rs Outdated
@olavloite
olavloite force-pushed the spanner-key-range-cache branch from dfb34e3 to a5fb379 Compare July 29, 2026 18:42
@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 introduces a location-aware routing module for Spanner, implementing an in-memory interval cache (KeyRangeCache) to map table key range split boundaries to tablet replica groups. Feedback on the implementation highlights a performance bottleneck in replace_range_if_newer_locked, where inserting a range performs an O(N) scan of the BTreeMap starting from Bound::Unbounded. It is recommended to optimize this to O(log N) by locating the first potentially overlapping range using a targeted B-tree lookup.

Comment thread src/spanner/src/routing/key_range_cache.rs
Add `KeyRangeCache`, an in-memory interval cache that maps table key range boundaries
(`start_key` and `limit_key`) to Paxos replica groups and tablet endpoints for location-aware routing.
@olavloite
olavloite force-pushed the spanner-key-range-cache branch from a5fb379 to 5ef5c5b Compare July 29, 2026 19:00
@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 introduces a thread-safe, in-memory interval cache (KeyRangeCache) for Spanner location-aware routing, along with its corresponding module structure. The cache maps table key range split boundaries to tablet replica groups and supports interval lookups for point keys and key ranges under CoveringSplit and PickRandom routing modes. It also includes a comprehensive suite of unit tests to verify its behavior under various scenarios such as overlapping ranges, different generations, and tablet selection. There are no review comments, and the implementation appears solid and well-tested, so I have no feedback to provide.

@olavloite
olavloite requested a review from sakthivelmanii July 29, 2026 19:03
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.

1 participant