Skip to content

feat(spanner): implement transaction routing logic based on database metadata isolation levels and lock modes#13800

Open
shobhitsg wants to merge 1 commit into
googleapis:mainfrom
shobhitsg:lryw_client
Open

feat(spanner): implement transaction routing logic based on database metadata isolation levels and lock modes#13800
shobhitsg wants to merge 1 commit into
googleapis:mainfrom
shobhitsg:lryw_client

Conversation

@shobhitsg

@shobhitsg shobhitsg commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

When a multiplexed session is created or replaced, the client library now automatically executes an asynchronous gRPC query (SELECT ... FROM INFORMATION_SCHEMA.DATABASE_OPTIONS) to cache database isolation levels and default read lock modes for Local Read-Your-Writes (LRYW) routing. Because this query executes via the standard query pipeline (singleUse().executeQuery(...)), it emits normal telemetry:

  • Trace Spans: An additional Spanner.ExecuteStreamingSql and CloudSpannerOperation.ExecuteStreamingQuery span.
  • Stream Annotations: A "Starting/Resuming stream" event during session setup.
  • Server Request Counts: An additional ExecuteSqlRequest received by mock endpoints.

Tests that strictly assert exact RPC invocation counts, channel allocations, and OpenTelemetry/OpenCensus span arrays (particularly under Java 8 where reflection-heavy tracer tests run) required updates to account for this automated warmup query.

Alternative Approaches

  1. Add a Test-Only Option Flag to Production Code (e.g., disableMetadataIntrospection)
    • Idea: Add a boolean toggle in SessionPoolOptions or a system property switch to suppress the metadata query during unit tests so test files wouldn't need updating one by one.
    • Why Rejected: It couples production classes to test logic and introduces a risky backdoor if a customer accidentally activates that flag in production.
  2. Warm Up & Reset Trace Exporters in @Before setUp()
    • Idea: Invoke client warmup in setUp() and call spanExporter.reset() right before individual test methods execute to erase startup query spans.
    • Why Rejected: Tests in SpanTest and OpenTelemetrySpanTest explicitly assert that the test's initial transaction generates the CreateMultiplexedSession span and annotation. Erasing spans in setup destroyed this required session-creation span, breaking assertions.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces dynamic database-level metadata detection (dialect, default transaction isolation level, and default read lock mode) queried from INFORMATION_SCHEMA.DATABASE_OPTIONS to replace client-side hardcoded defaults. The feedback identifies several critical issues: a correctness bug in canEnableLRYW that breaks Leader-Routed Read-Your-Writes for repeatable read transactions with unspecified lock modes, a race condition in metadataSupplier when retrieving the session reference, and a concurrency race condition in AbstractLazyInitializer between reset() and get(). Additionally, it is recommended to add a unit test to prevent regressions for repeatable reads with null lock modes.

@shobhitsg
shobhitsg force-pushed the lryw_client branch 4 times, most recently from 374c33b to 9b23481 Compare July 20, 2026 06:54
@shobhitsg
shobhitsg marked this pull request as ready for review July 20, 2026 09:37
@shobhitsg
shobhitsg requested review from a team as code owners July 20, 2026 09:37
@shobhitsg
shobhitsg force-pushed the lryw_client branch 3 times, most recently from 228b637 to d7c22d3 Compare July 20, 2026 17:54
@sakthivelmanii sakthivelmanii added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 21, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants