feat: Spanner.V1 BatchWrite #15122

robertvoinescu-work · 2025-10-01T19:51:58Z

User need access to the BatchWrite API through the Spanner SDK. This is important because BatchWrite allows users to submit a set of bulk mutations in batch for efficient processing. To address this, We introduce a BatchWriteAsync method in PooledSession exposing the API in Spanner.V1.

Details

BatchWrite does not exist in a transaction managed by the client - all transaction logic is server side. So, the BatchWriteAsync method avoids transaction management.
Integration tests have been added
Unit testing has been avoided since the returned type from SpannerClient.BatchWrite, AsyncStream<>, is a sealed class (i.e. cannot be mocked) with no public constructor (i.e. cannot be explicitly created in our tests). We could introduce wrappers to allow for mocking, but that adds a lot of abstraction layers I'm not sure we want.

Non Goals

Incorporate spanner into the Spanner ADO layer, this will be handled by another PR.

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

amanda-tarafa · 2025-10-09T05:06:05Z

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

+        /// Executes a BatchWrite RPC asynchronously, returning a stream of responses.
+        /// </summary>
+        /// <remarks>
+        /// This operation does not participate in the session's active transaction.


Is this true? Or does this operation requires a session with no active transaction?
Even if this is true, would this operation also work with a session with no active transaction?

The answer to these questions is fundamental to how we implement this, so I'll wait for the answers before continuing the review.

I will need to explicitly verify with spanner team, but I've tested this against my own spanner instance (non-emulated):

BatchWrite succeeds when working on a fresh session with no active transaction.

BatchWrite succeeds when working on a session on which I've begun a transaction.

Will verify with spanner team since transaction management is opaque to the caller for BatchWrite.

For 2. what happens to that transaction, any changes made within it, etc.?

I tried to use that transaction for read-write and received the following. It looks like batch write invalidates the currently active transaction

Error Message: Grpc.Core.RpcException : Status(StatusCode="FailedPre condition", Detail="This transaction has been invalidated by a later transaction in the same session."

OK, so if using a session with a transaction breaks that transaction, let's not allowed that. BatchWrite should only run on a session with no transaction, that is a pooled session with TransactionMode.None. The way to validate this is to go through ExecuteMaybeWithTransaction and fail on all the callbacks that attempt to either set or extract a transaction from the BatchWrite request/response. Let me know if you want to chat about that. Else, let me know when the code reflects these changes by re-requesting my review. Thanks.

amanda-tarafa

I've left all the comments, but I think we should wait for MUX to implement this, let's chat. I'm marking as do not merge for the time being.

amanda-tarafa · 2025-10-13T17:58:36Z

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

+        /// should set this parameter to false.</param>
+        /// <param name="cancellationToken">The cancellation token for the operation.</param>
+        /// <returns>A task whose result will be the result from having executed <paramref name="command"/>.</returns>
+        internal TResponse ExecuteMaybeWithTransactionSelector<TResponse>(


We don't need a sync version of this method. If the actual exection is syncrhonous, the async method will return synchronously anyway, and here, . Please undo and call the async version instead.

amanda-tarafa · 2025-10-13T18:00:19Z

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

+        /// </remarks>
+        /// <param name="request">The batch write request. Must not be null.</param>
+        /// <param name="callSettings">If not null, applies overrides to this RPC call.</param>
+        /// <returns>An asynchronous stream of <see cref="BatchWriteResponse"/> messages.</returns>


Suggested change

/// <returns>An asynchronous stream of <see cref="BatchWriteResponse"/> messages.</returns>

/// <returns>An asynchronous stream of <see cref="BatchWriteResponse"/> objects.</returns>

amanda-tarafa · 2025-10-13T18:01:48Z

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

+            CheckNotDisposed();
+            GaxPreconditions.CheckNotNull(request, nameof(request));
+            // Fail early to prevent the creation of an unused transaction on the session.
+            GaxPreconditions.CheckState(TransactionOptions.ModeCase == ModeOneofCase.None, $"{nameof(BatchWriteAsync)} can only be used when the session is configured to use no transaction.");


We don't need this, the ExecuteMaybeWithTransactionSelector will fail early enough (i.e. without making any RPC) if we configure the right callbacks. See below.

amanda-tarafa · 2025-10-13T18:06:50Z

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

+            }
+
+            // Batch write gRPC manages the transaction itself, if we are trying to explicitly set a transaction or inline, this is a bug.
+            void SetCommandTransaction(TransactionSelector transactionSelector) => throw new InvalidOperationException($"{nameof(BatchWriteAsync)} must be executed with no transaction. If you see this there is a bug in the code.");


This one is sufficient to fail early. This would be our validation point, BatchWrite has not caused any RPCs to be executed at this point.
Message, or similar:
"BatchWrite does not support explicitly set transactions, it won't be executed within an active transaction. The sessions most be acquired with nameof(ModeOneOfCase.TransactionNone) or BatchWrite should complete execution before any other commands that would create a transaction are executed."

amanda-tarafa · 2025-10-13T18:09:11Z

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

+
+            // Batch write gRPC manages the transaction itself, if we are trying to explicitly set a transaction or inline, this is a bug.
+            void SetCommandTransaction(TransactionSelector transactionSelector) => throw new InvalidOperationException($"{nameof(BatchWriteAsync)} must be executed with no transaction. If you see this there is a bug in the code.");
+            Transaction GetInlinedTransaction(IAsyncEnumerable<BatchWriteResponse> response) => throw new InvalidOperationException($"{nameof(BatchWriteAsync)} cannot be executed with an inlined transaction. If you see this there is a bug in the code.");


I think this delegate may be set to null on the paremeter call. That's a signal to the "ExecuteMaybeWithTransactionAsync" that this method does not support inlining. Because we are also asking it to skip transaction creation, this will never be called.

SetCommandTransaction will be called is some other method created a transaction and that's when we want to fail.

amanda-tarafa · 2025-10-13T19:35:42Z

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

@@ -876,6 +944,22 @@ private async Task RecordSuccessAndExpiredSessions(Task task)
            UpdateRefreshTime();
        }

+        private async IAsyncEnumerable<TResponse> StreamResponsesAndRecordSuccessAsync<TResponse>(AsyncResponseStream<TResponse> responseStream)


I'm not sure we want to do this on V1. AsyncResponseStream is a wrapper that works for both pre and post C# 8 users. And we don't want to loose that just so we can keep better track of the refresh time/expired session.

I think we should not wrap the response, and then, to make certain that we don't return a bad session to the pool, after the succesful call to BatchWrite, before returning the AsyncResponseStream, I'd update the refresh time to now, to make certain the session will be refreshed before releasing.

But, wrting about all of this, and knowing that we are about to release MUX, I think we should pause. Let's chat.

robertvoinescu-work requested a review from a team as a code owner October 1, 2025 19:51

product-auto-label bot added the api: spanner Issues related to the Spanner API. label Oct 1, 2025

robertvoinescu-work marked this pull request as draft October 1, 2025 19:52

robertvoinescu-work force-pushed the users/robertvoinescu/spanner-batchwrite branch from 4c23ade to f2c5f92 Compare October 1, 2025 19:55

robertvoinescu-work changed the title ~~Spanner.V1 BatchWrite~~ feat: Spanner.V1 BatchWrite Oct 1, 2025

robertvoinescu-work force-pushed the users/robertvoinescu/spanner-batchwrite branch 9 times, most recently from 606bdfd to d803b83 Compare October 1, 2025 21:12

robertvoinescu-work marked this pull request as ready for review October 1, 2025 21:17

robertvoinescu-work assigned robertvoinescu-work, purva9413 and amanda-tarafa and unassigned robertvoinescu-work Oct 1, 2025

purva9413 reviewed Oct 2, 2025

View reviewed changes

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/PooledSession.cs Outdated Show resolved Hide resolved

apis/Google.Cloud.Spanner.V1/Google.Cloud.Spanner.V1/PooledSession.cs Outdated Show resolved Hide resolved

robertvoinescu-work force-pushed the users/robertvoinescu/spanner-batchwrite branch from d803b83 to a91b1b4 Compare October 6, 2025 21:38

robertvoinescu-work requested a review from purva9413 October 7, 2025 17:35

amanda-tarafa reviewed Oct 9, 2025

View reviewed changes

robertvoinescu-work force-pushed the users/robertvoinescu/spanner-batchwrite branch from a91b1b4 to 89082c8 Compare October 10, 2025 16:13

feat: Add BatchWrite to Spanner.V1

0ccf88d

robertvoinescu-work force-pushed the users/robertvoinescu/spanner-batchwrite branch from 89082c8 to 0ccf88d Compare October 10, 2025 16:43

robertvoinescu-work requested a review from amanda-tarafa October 10, 2025 17:12

amanda-tarafa reviewed Oct 13, 2025

View reviewed changes

amanda-tarafa added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 13, 2025

amanda-tarafa marked this pull request as draft November 21, 2025 21:08

	/// <returns>An asynchronous stream of <see cref="BatchWriteResponse"/> messages.</returns>
	/// <returns>An asynchronous stream of <see cref="BatchWriteResponse"/> objects.</returns>

feat: Spanner.V1 BatchWrite #15122

Are you sure you want to change the base?

feat: Spanner.V1 BatchWrite #15122

Uh oh!

Conversation

robertvoinescu-work commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Non Goals

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertvoinescu-work Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertvoinescu-work Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amanda-tarafa left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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

Development

Uh oh!

3 participants

robertvoinescu-work commented Oct 1, 2025 •

edited

Loading

robertvoinescu-work Oct 9, 2025 •

edited

Loading

robertvoinescu-work Oct 9, 2025 •

edited

Loading

amanda-tarafa left a comment •

edited

Loading