chore: add routing hints and cache updates for begin/commit by rahul2393 · Pull Request #4392 · googleapis/java-spanner

rahul2393 · 2026-03-25T07:01:20Z

Summary

This change extends key-aware routing to include mutation-based BeginTransaction and Commit request payloads, and teaches the response path to consume CacheUpdate from Transaction and CommitResponse.

What changed

Populate RoutingHint on BeginTransaction when a mutationKey is present
Populate RoutingHint on Commit from the first mutation in the request
Keep existing transaction-affinity routing for Commit/Rollback
Apply CacheUpdate from:
- Transaction
- CommitResponse
Add focused KeyAwareChannel tests for the new request/response behavior

gemini-code-assist · 2026-03-25T07:01:42Z

Summary of Changes

Hello, 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 significantly enhances the Spanner client library's key-aware routing capabilities by integrating routing hints for mutation-based BeginTransaction and Commit operations. It also improves the client's ability to process and apply CacheUpdate messages received from both Transaction and CommitResponse objects, leading to more efficient and intelligent request routing based on the latest database topology information.

Highlights

Enhanced Routing for BeginTransaction: Extended key-aware routing to BeginTransaction requests when a mutationKey is present, ensuring more efficient initial transaction placement.
Commit Request Routing Hints: Implemented routing hint population for Commit requests, deriving the hint from the first mutation in the request payload.
Cache Update Processing: Enabled the application of CacheUpdate messages received from both Transaction and CommitResponse objects, allowing the client to dynamically update its routing knowledge.
New Unit Tests: Added new unit tests for the KeyAwareChannel to thoroughly cover the new routing hint generation and cache update consumption behaviors for BeginTransaction and Commit.

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.

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 enhances the Spanner client library by introducing routing hints for mutation-based BeginTransaction and Commit requests. The ChannelFinder now includes logic to apply routing hints to CommitRequests, and KeyAwareChannel has been updated to process CacheUpdates from Transaction and CommitResponse messages, enabling dynamic routing. A new computeKeys method was added to KeyRecipeCache to handle mutations. Corresponding tests have been added to validate these new routing behaviors. A suggestion was made to improve readability in KeyAwareChannel.java by removing a redundant null check and combining if statements.

gemini-code-assist · 2026-03-25T07:05:02Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyAwareChannel.java

+          if (databaseId != null) {
+            finder = parentChannel.getOrCreateChannelFinder(databaseId);
+          }
+          if (finder != null) {
+            finder.fillRoutingHint(reqBuilder);
+          }


Since getOrCreateChannelFinder never returns null, the null check for finder is redundant. You can combine these two if statements for better readability and conciseness.

if (databaseId != null) { finder = parentChannel.getOrCreateChannelFinder(databaseId); finder.fillRoutingHint(reqBuilder); }

olavloite · 2026-03-25T10:59:46Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/ChannelFinder.java

+        reqBuilder.getRoutingHintBuilder());
  }

  private ChannelEndpoint fillRoutingHint(


Maybe not directly related to this change, but I noticed that this method is unused. Can we remove it?

olavloite · 2026-03-25T11:08:02Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyAwareChannel.java

+          if (finder != null) {
+            finder.fillRoutingHint(reqBuilder);
+          }
+          if (!reqBuilder.getTransactionId().isEmpty()) {


TransactionId is empty if the CommitRequest has a single-use read/write transaction. Meaning that we skip this for 'write-at-least-once' transactions. Is that intentional? Normally, such a CommitRequest would contain mutations, meaning that we would be able to route in using the same logic as a BeginTransactionRequest that contains a mutation key, right?

olavloite · 2026-03-25T11:10:50Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/ChannelFinder.java

  public ChannelEndpoint findServer(BeginTransactionRequest.Builder reqBuilder) {
-    if (!reqBuilder.hasMutationKey()) {
+    if (!reqBuilder.hasMutationKey()
+        || !recipeCache.computeKeys(


I find some of this code very hard to read. E.g. the fact that computeKeys(..) returns a boolean and has the side-effect that it modifies the RoutingHintBuilder that it gets (but only if it returns true, but also sometimes when it returns false, see also below) makes this very hard to understand. I think it would be good if we would take a 'readability sweep' of both this method, but also the code in general.

olavloite · 2026-03-25T11:13:22Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyRecipeCache.java


+  boolean computeKeys(Mutation mutation, RoutingHint.Builder hintBuilder) {
+    if (!schemaGeneration.isEmpty()) {
+      hintBuilder.setSchemaGeneration(schemaGeneration);


This updates the hint builder, but the method might still return false if the TargetRange below is null. That seems weird. I would expect this method only to modify the hint builder if it also returns true. Can we otherwise at least add some documentation to the method that explains when it returns true/false, and when it modifies the hint builder?

olavloite · 2026-03-25T11:17:45Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyAwareChannel.java

-          if (!request.getTransactionId().isEmpty()) {
-            endpoint = parentChannel.affinityEndpoint(request.getTransactionId());
-            transactionIdToClear = request.getTransactionId();
+          CommitRequest.Builder reqBuilder = ((CommitRequest) message).toBuilder();


This could potentially be a very heavy operation, as a CommitRequest could contain a very large number of mutations (e.g. during a bulk load). Converting it back to a builder means copying all of those mutations. I think that we should only call toBuilder() if we really know that we need the builder (i.e. if we know that we will be setting a routing hint).

This is also true for the other types of requests here, but those are less likely to be as heavy as a CommitRequest.

olavloite · 2026-03-25T11:18:37Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/KeyAwareChannelTest.java

  }

+  @Test
+  public void beginTransactionWithMutationKeyAddsRoutingHint() throws Exception {


Can we add a test for CommitRequest with a single-use read/write transaction?

olavloite · 2026-03-25T11:19:14Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/KeyAwareChannelTest.java

+            harness.defaultManagedChannel.latestCall();
+
+    assertThat(beginDelegate.lastMessage).isNotNull();
+    assertThat(beginDelegate.lastMessage.getRoutingHint().getDatabaseId()).isEqualTo(7L);


nit: use assertEquals(..) (and in general try to stick to JUnit assertions)

olavloite · 2026-03-25T11:23:54Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyRecipeCache.java

    }
  }

+  boolean computeKeys(Mutation mutation, RoutingHint.Builder hintBuilder) {


This method in general is quite hard to read, as it both returns a value that indicates whether the method did something or not, and has a potential side-effect of (maybe) populating the hintBuilder. Is there a better way to do this?

removed that helper from the mutation routing flow.

olavloite · 2026-03-25T17:13:10Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/KeyAwareChannel.java

+            request = reqBuilder.build();
+          }
+          if (!request.getTransactionId().isEmpty()) {
+            endpoint = parentChannel.affinityEndpoint(request.getTransactionId());


This overrides the endpoint that was filled above, even in the case if parentChannel.affinityEndpoint(..) returns null, which means that it will fall back to the default endpoint. Is that intentional? (I'm not sure if it can happen in real life, though....)

In case when transactionID is present but no affinity, we should just forward it to the default channel, example

For ExecuteSql / Read, if the first statement falls back to the default endpoint, we do not record default-host affinity because allowDefaultAffinity is false on that path. That means a later CommitRequest can carry a valid transactionId while affinityEndpoint still returns null, in this case we should just forward to default host.

… use for beginTransaction RPC

chore(spanner): add routing hints and cache updates for begin/commit

82c3244

rahul2393 requested review from a team as code owners March 25, 2026 07:01

product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Mar 25, 2026

chore: generate libraries at Wed Mar 25 07:01:43 UTC 2026

3c99d2a

gemini-code-assist bot reviewed Mar 25, 2026

View reviewed changes

rahul2393 changed the title ~~chore(spanner): add routing hints and cache updates for begin/commit~~ chore: add routing hints and cache updates for begin/commit Mar 25, 2026

rahul2393 requested a review from olavloite March 25, 2026 07:25

olavloite reviewed Mar 25, 2026

View reviewed changes

incorporate suggestions

e836a5d

rahul2393 requested a review from olavloite March 25, 2026 16:54

rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 25, 2026

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 25, 2026

olavloite approved these changes Mar 25, 2026

View reviewed changes

use same heuristic for selecting mutation for commit routing which we…

0f39595

… use for beginTransaction RPC

rahul2393 merged commit 7a42211 into main Mar 25, 2026
42 of 47 checks passed

rahul2393 deleted the begin_commit_routing_hint branch March 25, 2026 18:09

Conversation

rahul2393 commented Mar 25, 2026

Summary

What changed

Uh oh!

gemini-code-assist bot commented Mar 25, 2026

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!

gemini-code-assist bot Mar 25, 2026

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!

rahul2393 Mar 25, 2026 • 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rahul2393 Mar 25, 2026 •

edited

Loading