-
Notifications
You must be signed in to change notification settings - Fork 132
[spanner] Add TransactionManager #405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
danielnorberg
wants to merge
6
commits into
yoshidan:main
Choose a base branch
from
danielnorberg:dano/transaction-manager
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
[spanner] Add TransactionManager #405
danielnorberg
wants to merge
6
commits into
yoshidan:main
from
danielnorberg:dano/transaction-manager
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ith session reuse Implements the TransactionManager API as described in issue yoshidan#375. This allows manual control over transaction execution while maintaining session reuse across retries, which helps retain lock priority and improves commit success rates for ABORTED transactions. Key changes: - Add TransactionManager struct that holds a session across multiple transaction attempts - Add Client::transaction_manager() method to create a TransactionManager - Add Debug implementation for BeginError to support unwrap() in user code - Add comprehensive integration tests for TransactionManager functionality The API enables users to handle transaction retry logic manually while benefiting from session reuse: ```rust let mut tm = client.transaction_manager().await?; let retry = &mut TransactionRetry::new(); loop { let tx = tm.begin_read_write_transaction().await?; let result = run_in_transaction(tx).await; match tx.end(result, None).await { Ok((commit_result, success)) => return Ok(success), Err(err) => retry.next(err).await? } } ``` Fixes yoshidan#375 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…fy Drop handling Improvements: - Use client::Error instead of BeginError for better API consistency with Client::begin_read_write_transaction() - Handle session preservation internally when begin fails, storing it back for retry - Remove redundant Drop implementation - ManagedSession::drop() already handles session pool return - Remove Debug implementation from BeginError as it's no longer needed Benefits: - Consistent error types across the API - Simpler, more idiomatic code - Better encapsulation of session management - Automatic cleanup through Rust's Drop chain 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…g common logic Refactoring: - Extract common transaction begin logic into private begin_internal() helper method - Simplify public begin_read_write_transaction() methods to thin delegates - Consolidates 31 lines of duplicate code (90% identical) into single implementation Benefits: - Single source of truth for transaction begin logic - Bug fixes and changes only needed in one place - 11 lines removed (11% reduction in file size) - Improved maintainability with no behavior changes Before: Two 23-line methods with duplicate logic After: Two 2-line delegates + one 23-line helper 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…method Refactoring: - Remove begin_internal() private helper method - Change begin_read_write_transaction() to directly delegate to begin_read_write_transaction_with_options() - Move implementation logic into begin_read_write_transaction_with_options() Benefits: - Simpler, more idiomatic Rust pattern (convenience method → full method) - Removes unnecessary indirection through private helper - 8 lines removed (8% reduction) - Clearer code flow with direct delegation This follows the common Rust pattern where simple methods call more complex ones with default parameters, similar to Vec::new() → Vec::with_capacity(0). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Features: - Add transaction() method that returns Option<&mut ReadWriteTransaction> - Returns None if no transaction is active, Some if active - Enables checking transaction state and accessing existing transaction - Includes comprehensive test coverage Use cases: - Check if a transaction exists without starting one - Access existing transaction for additional operations - Conditional logic based on transaction state Test added: - test_transaction_accessor() validates behavior before/after begin - Tests functional access through the accessor - Verifies data written through accessor is committed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
537b5b6 to
b23f1f4
Compare
Add disable_route_to_leader field to TransactionManager and pass it through to ReadWriteTransaction::begin. This ensures that TransactionManager respects the client's disable_route_to_leader setting when creating transactions. Changes: - Add disable_route_to_leader field to TransactionManager struct - Update TransactionManager::new to accept disable_route_to_leader parameter - Pass disable_route_to_leader to ReadWriteTransaction::begin in begin_read_write_transaction_with_options - Update Client::transaction_manager to pass disable_route_to_leader from client config 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements the TransactionManager API as described in issue #375. This allows manual control over transaction execution while maintaining session reuse across retries, which helps retain lock priority and improves commit success rates for ABORTED transactions.
Key changes:
The API enables users to handle transaction retry logic manually while benefiting from session reuse:
Fixes #375
🤖 Generated with Claude Code