chore(spanner): implement transaction affinity resolution, R/W counting, and CAS conflict handling in channel pool - #6737
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Spanner channel pool and transaction affinity management. Key changes include tracking RwTransactionAffinityGuard within TransactionAffinity to manage active read/write transaction counts, introducing a robust compare-and-swap (CAS) conflict resolution mechanism (resolve_cas_conflict) for concurrent transaction pinning, simplifying ChannelEntry by removing redundant fields, and implementing Deref for ChannelLease. The feedback identifies an unresolved rustdoc link in the documentation of default_channel referencing ChannelPool::next_channel instead of pick_channel, which should be corrected to prevent CI build failures.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6737 +/- ##
==========================================
+ Coverage 96.94% 96.96% +0.02%
==========================================
Files 315 315
Lines 105078 105470 +392
==========================================
+ Hits 101864 102271 +407
+ Misses 3214 3199 -15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ng, and CAS conflict handling in channel pool
077f898 to
dd6ede2
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors and robustifies the Spanner channel pool's transaction affinity and channel leasing mechanisms. Key changes include introducing RwTransactionAffinityGuard management within TransactionAffinity to prevent premature channel reclamation during active read-write transactions, implementing transparent Deref to Channel for ChannelLease, and refactoring the compare-and-swap (CAS) conflict resolution logic in ChannelPool::resolve_affinity into a dedicated, loop-based resolve_cas_conflict method. Additionally, ChannelEntry was simplified by removing the redundant logical_channel_id field in favor of a getter, and extensive unit tests were added to verify the new guard behaviors and CAS resolution paths. I have no further feedback to provide as the changes are well-structured, idiomatic, and thoroughly tested.
Implements transaction affinity resolution and concurrency handling for multi-statement transactions inside
ChannelPool.Key changes: