feat: Multiplex session v1 implementation #15322

purva9413 · 2025-12-11T21:31:13Z

No description provided.

gemini-code-assist · 2025-12-11T21:31:45Z

Summary of Changes

Hello @purva9413, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request delivers the foundational implementation for multiplexed sessions in the Google Cloud Spanner client library. The primary goal is to optimize resource usage and enhance performance by enabling multiple transactions to operate concurrently over a single session. This involves the introduction of new abstractions for managing these sessions and their associated transactions, along with updates to the session pooling infrastructure and a thorough suite of tests to ensure stability and correctness.

Highlights

Multiplexed Session Implementation: Introduced core classes ManagedSession, ManagedTransaction, and ManagedSessionOptions to support multiplexed sessions, allowing multiple transactions to share a single Spanner session for improved performance and resource utilization.
Session Management Enhancements: The SessionPoolManager has been updated to include functionality for acquiring and managing these new ManagedSession instances, alongside existing session pool mechanisms.
Transaction Handling within Multiplexed Sessions: The ManagedTransaction class encapsulates the logic for handling transactions within a multiplexed session, including managing transaction IDs, precommit tokens, and integrating with various Spanner RPC calls like Commit, Rollback, ExecuteSql, and Read operations.
Session Refresh and Expiry Logic: The ManagedSession includes robust logic for proactively refreshing sessions (soft refresh) and enforcing hard refreshes when sessions are nearing or past their expiry, ensuring session validity and optimal performance.
Comprehensive Testing: New integration and unit tests have been added (ManagedSessionTests.cs in both IntegrationTests and Tests projects) to validate the functionality of multiplexed sessions, covering session creation, read-write transactions, concurrent operations, and session expiry. Existing tests related to directed reads and leader routing have been adapted to use the new ManagedTransaction model.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a major new feature: multiplexed sessions. The implementation adds several new classes like ManagedSession, ManagedTransaction, and ManagedSessionOptions, and updates the SessionPoolManager to handle these new session types. The changes also include extensive refactoring of existing tests to use the new ManagedTransaction instead of PooledSession, and adds new integration and unit tests for the multiplexed session functionality.

Overall, the implementation is comprehensive and well-tested. I've identified a critical thread-safety issue in SessionPoolManager that needs to be addressed. Additionally, there are several opportunities for code cleanup and simplification, such as removing unused variables, commented-out code, and duplicate test cases, as well as improving documentation by filling in TODOs and empty XML comments. My detailed feedback is in the comments below.

apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data/SessionPoolManager.cs

...ogle.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.IntegrationTests/V1/ManagedSessionTests.cs

apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data.Tests/V1/DirectedReadTests.cs

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSessionOptions.cs

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs

gemini-code-assist · 2025-12-11T21:33:38Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs

+    public partial class ManagedTransaction
+    {
+        // All these fields are derived from the ManagedSession
+        private readonly Session _multiplexSession;


The field _multiplexSession is of type Session, but its name suggests it's a ManagedSession, which can be confusing. Consider renaming it to _session or _underlyingSession for clarity. You will also need to update its usages in this class.

private readonly Session _session;

I think this is more expressive on which type of session (multiplex vs pooled) from the backend is expected in the ManagedSession. Plus it is a private variable.

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs

amanda-tarafa

Patial review for now.

amanda-tarafa · 2025-12-16T21:11:19Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+/// TODO: Add summary for mux sessions
+/// </summary>


apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

amanda-tarafa · 2025-12-16T21:19:11Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+    /// <summary>
+    /// The name of the session. This is never null.
+    /// </summary>
+    internal SessionName SessionName => Session.SessionName;


It can be null now, before we acquire the first session?

Well, the SessionBuilder.BuildAsync() is the only way for the customer to create a ManagedSession. And we do not return prematurely in the sense we only return a ManagedSession object once the underlying Session of the ManagedSession is created. Does that make sense?

amanda-tarafa · 2025-12-16T21:20:48Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+    public DatabaseName DatabaseName { get; }
+
+    /// <summary>
+    /// The database role of the managed session
+    /// </summary>
+    public string DatabaseRole { get; }


Arent these two on the ManagedSessionOptions already, and/or could we put them there?

I wanted to keep ManagedSessionOptions as optional configurable parameters for the ManagedSession (Similar to the SessionPoolOptions). The DatabaseName and DatabaseRole are required parameters for creating a multiplex session in Spanner (so by relation also for creating a ManagedSession), and are set on the SessionBuilder initially when the customer wants to build a ManagedSession.

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

amanda-tarafa · 2025-12-16T21:26:47Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+    public async Task<ManagedTransaction> CreateManagedTransactionWithOptions(TransactionOptions transactionOptions, bool singleUseTransaction)
+    {
+        await MaybeRefreshWithTimePeriodCheck().ConfigureAwait(false);
+        return new ManagedTransaction(this, transactionId: null, transactionOptions, singleUseTransaction, readTimestamp: null);


Do we need to pass this or could we just pass _session?

There are a couple of things needed from the ManagedSession object when we create the ManagedTransaction. These are set in the constructor of the ManagedTransaction through the this ManagedSession object:

The client

The underlying session

Configurable options from the ManagedSession.Options like the RPC timeouts.

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

chore: Remove Task.Run for creating the session

robertvoinescu-work · 2025-12-19T20:44:56Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+            try
+            {
+                // Non-blocking, fast check if the session is still "fresh" enough
+                if (_session != null && !SessionHasExpired(HardRefreshIntervalInDays))


The SessionHasExpired treats the null case as expired. The null check shouldn't be necessary.

robertvoinescu-work · 2025-12-19T20:57:18Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+                                {
+                                    // Clear the task after completion (success or failure)
+                                    // to allow the next refresh to run.
+                                    _ = Interlocked.Exchange(ref _sessionCreationTask, null);


NIT: I don't think you need an interlocked exchange here since it's already executing within the context of a lock. But It should be fine, if we want to be defensive.

robertvoinescu-work · 2025-12-19T21:02:59Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs

+        /// If this is true then <see cref="TransactionOptions"/> will be <see cref="TransactionOptions.ModeOneofCase.ReadOnly"/>
+        /// and <see cref="TransactionId"/> will be null.
+        /// </summary>
+        internal bool SingleUseTransaction { get; }


NIT: How do you feel aboutIsSingleUseTransaction to make it clear it's a boolean. Otherwise I see that name and think it might be a transaction object.

robertvoinescu-work · 2025-12-30T22:04:36Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs

                transactionId is null || TransactionOptions.ModeCase != ModeOneofCase.None,
                nameof(transactionOptions),
-                "No transaction options were specified for the given transasaction ID.");
+                $"No transaction options were specified for the given transasaction ID {transactionId is null}, {transactionId?.ToBase64()}, {TransactionOptions.ModeCase}.");


This message would be hard to interpret without looking at the code, could you update it to something more like this:

$"No transaction options were specified. TransactionId: {transactionId?.ToBase64() ?? "null"}, Mode: {TransactionOptions.ModeCase}."

robertvoinescu-work · 2025-12-30T22:15:57Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedTransaction.cs

        /// <param name="callSettings">If not null, applies overrides to this RPC call.</param>
        /// <returns>A task representing the asynchronous operation. When the task completes, the result is the response from the RPC.</returns>
-        internal Task<PartitionResponse> PartitionReadOrQueryAsync(PartitionReadOrQueryRequest request, CallSettings callSettings)
+        internal async Task<PartitionResponse> PartitionReadOrQueryAsync(PartitionReadOrQueryRequest request, CallSettings callSettings)


Why add the async modifier? I think it's marginally more efficient the original way. Same question other places like RollBackAsync

robertvoinescu-work · 2025-12-30T22:52:27Z

apis/Google.Cloud.Spanner.Data/Google.Cloud.Spanner.Data/SessionPoolManager.cs

+                var client = await _clientFactory.Invoke(options, SpannerSettings).ConfigureAwait(false);
+                var managedSessionBuilder = new SessionBuilder(dbName, client)
+                {
+                    Options = ManagedSessionOptions,


ManagedSessionOptions could be null here depending on the static constructor used to create the SessionPoolmanager do we need another null check here?

robertvoinescu-work · 2025-12-30T23:09:46Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+        public string DatabaseRole { get; set; }
+
+        /// <summary>
+        /// The client used for all operations in this managed session.


We should add a note here similar to DataBasename saying it is required.

robertvoinescu-work · 2025-12-30T23:16:02Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+    /// <summary>
+    /// Returns a ManagedTransaction for the same ManagedSession as this one with the given transaction related values.
+    /// </summary>
+    public async Task<ManagedTransaction> CreateManagedTransaction(ByteString transactionId, TransactionOptions transactionOptions, bool singleUseTransaction, Timestamp readTimestamp = null, CancellationToken cancellationToken = default)


The names need to be updated to CreateMangedTransactionAsync for all methods.

robertvoinescu-work · 2025-12-30T23:16:42Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+    /// <summary>
+    /// Returns a ManagedTransaction for the same ManagedSession as this one with the given transaction mode.
+    /// </summary>
+    public async Task<ManagedTransaction> CreateManagedTransaction(ByteString transactionId, ModeOneofCase transactionMode, Timestamp readTimestamp = null)


Should we take bool singleUseTransaction here also?

robertvoinescu-work · 2025-12-30T23:57:42Z

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/ManagedSession.cs

+        return false;
+    }
+
+    private async Task CreateOrRefreshSessionsAsync(CancellationToken cancellationToken, bool needsRefresh = false)


This is more of a high level comment. The current code works, but how do we feel about using SemaphoreSlim instead? It provides a natural way to deal with async locking without the need to manage the lifecycle of a _sessionCreationTask.

High level it'd be like this:

private readonly SemaphoreSlim _sessionLock = new(1, 1); private async Task TriggerRefresh(double refreshInterval, CancellationToken ct) { await _sessionLock.WaitAsync(ct); try { if (SessionHasExpired(refreshInterval)) { _session = await Client.CreateSessionAsync(...); } } finally { _sessionLock.Release(); } }

purva9413 requested a review from a team as a code owner December 11, 2025 21:31

gemini-code-assist bot reviewed Dec 11, 2025

View reviewed changes

amanda-tarafa reviewed Dec 16, 2025

View reviewed changes

purva9413 requested a review from amanda-tarafa December 17, 2025 23:29

purva9413 mentioned this pull request Dec 18, 2025

feat: Changes for Multiplex Session in Spanner.Data #15335

Open

purva9413 force-pushed the multiplex-split-v1 branch from 6de231e to d60cf55 Compare December 19, 2025 21:15

purvavasudeo added 5 commits December 22, 2025 09:25

chore: Copy PooledSession to ManagedTransaction for ease of review

177720e

feat: V1 changes to support multiplex sessions

f5697cf

test: Integration tests for ManagedSession and plumbing around those

0ee1b7e

chore: Resolve Gemini review comments

25bc8ed

chore: Resolve review comments, make session refresh only one method

6ffc5a5

chore: Remove Task.Run for creating the session

purva9413 force-pushed the multiplex-split-v1 branch from d60cf55 to 6ffc5a5 Compare December 22, 2025 17:26

robertvoinescu-work reviewed Dec 31, 2025

View reviewed changes

feat: Multiplex session v1 implementation #15322

Are you sure you want to change the base?

feat: Multiplex session v1 implementation #15322

Uh oh!

Conversation

purva9413 commented Dec 11, 2025

Uh oh!

gemini-code-assist bot commented Dec 11, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amanda-tarafa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone