Skip to content

fix(spanner): fix flakiness and race conditions in multiplexed session tests#12949

Merged
sakthivelmanii merged 1 commit intomainfrom
fix-multiplexed-session-test
Apr 29, 2026
Merged

fix(spanner): fix flakiness and race conditions in multiplexed session tests#12949
sakthivelmanii merged 1 commit intomainfrom
fix-multiplexed-session-test

Conversation

@sakthivelmanii
Copy link
Copy Markdown
Contributor

@sakthivelmanii sakthivelmanii commented Apr 29, 2026 •
edited
Loading

No description provided.

@sakthivelmanii sakthivelmanii requested review from a team as code owners April 29, 2026 10:14
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Apr 29, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

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

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 a new helper method, ofMinimumAndRandomTimeAndStickyException, in MockSpannerServiceImpl to simulate RPC delays alongside persistent exceptions. Corresponding tests in MultiplexedSessionDatabaseClientMockServerTest were updated to utilize this method. Feedback suggests replacing the magic number 200 with a named constant for better clarity and considering a more targeted reset of execution times instead of using the broad removeAllExecutionTimes() method.

Comment thread ...r/src/test/java/com/google/cloud/spanner/MultiplexedSessionDatabaseClientMockServerTest.java Outdated
mockSpanner.setCreateSessionExecutionTime(
SimulatedExecutionTime.ofStickyException(Status.DEADLINE_EXCEEDED.asRuntimeException()));
SimulatedExecutionTime.ofMinimumAndRandomTimeAndStickyException(
200, 0, Status.DEADLINE_EXCEEDED.asRuntimeException()));
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

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

medium

The magic number 200 is used here and in other places (e.g., line 342) to represent the simulated execution delay. It would be better to define this as a named constant (e.g., SIMULATED_RPC_DELAY_MS) to improve maintainability and clarity across the test suite.

Comment thread ...r/src/test/java/com/google/cloud/spanner/MultiplexedSessionDatabaseClientMockServerTest.java Outdated
// The CreateSession RPC will be retried, and as the exception is removed by the first call,
// The CreateSession RPC will be retried, and as we manually clear the exception,
// the second attempt will succeed.
mockSpanner.removeAllExecutionTimes();
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

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

medium

The use of removeAllExecutionTimes() is quite broad as it clears all simulated execution times for all RPC methods. While safe in this specific test context, consider using a more targeted reset (e.g., setCreateSessionExecutionTime(null)) if available, to avoid unintended side effects in more complex test scenarios where other RPCs might have configured behaviors.

@sakthivelmanii sakthivelmanii force-pushed the fix-multiplexed-session-test branch 2 times, most recently from e55d353 to 15b11ed Compare April 29, 2026 11:59
olavloite
olavloite approved these changes Apr 29, 2026
View reviewed changes
Comment thread ...nner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/MockSpannerServiceImpl.java
}

private void maybeFreezeAndRecordRequest(AbstractMessage request) {
synchronized (lock) {
Copy link
Copy Markdown
Contributor

@olavloite olavloite Apr 29, 2026

Choose a reason for hiding this comment

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

(Probably does not matter much, but still): Do you really need to take the lock here?

@sakthivelmanii sakthivelmanii force-pushed the fix-multiplexed-session-test branch from 15b11ed to acc5b8c Compare April 29, 2026 14:09
@sakthivelmanii sakthivelmanii merged commit 3e02f18 into main Apr 29, 2026
130 of 131 checks passed
@sakthivelmanii sakthivelmanii deleted the fix-multiplexed-session-test branch April 29, 2026 16:19
This was referenced Apr 30, 2026
Closed
Closed
Merged
jinseopkim0 added a commit that referenced this pull request May 5, 2026
@release-please @jinseopkim0
chore(main): release 1.86.0 (#13015)
4614cae 
🤖 I have created a release *beep* *boop*
---


<details><summary>1.86.0</summary>

##
[1.86.0](v1.85.0...v1.86.0)
(2026-05-05)


### Features

* [aiplatform] Add asyncQueryReasoningEngine to aiplatform v1 API
([a1ab487](a1ab487))
* [aiplatform] Add asyncQueryReasoningEngine to aiplatform v1beta1
([a1ab487](a1ab487))
* [aiplatform] add OnlineEvaluator API and update Evaluation API
([a1ab487](a1ab487))
* [aiplatform] Model Registry CopyModel BYOSA
([531942b](531942b))
* [aiplatform] new field CopyModelRequest.custome_service_account
([531942b](531942b))
* [aiplatform] Support VeoLoraTuningSpec in the tuning jobs
([a1ab487](a1ab487))
* [analytics-admin] add UserProvidedDataSettings resource and
([a1ab487](a1ab487))
* [bigqueryreservation] add principal field to BigQuery Reservation
([531942b](531942b))
* [ces] Add ability to specify mocked tool responses in ExecuteTool
([a1ab487](a1ab487))
* [chat] Addition of ChatService.FindGroupChats
([a1ab487](a1ab487))
* [compute] Update Compute Engine v1 API to revision 20260331
([d3b76d9](d3b76d9))
* [health] new module for health
([#12993](#12993))
([d568a8c](d568a8c))
* [iam] new iam v3beta client for AccessPolicies, this is step 4&5
([a1ab487](a1ab487))
* [mapmanagement] new module for mapmanagement
([#12874](#12874))
([0720279](0720279))
* [modelarmor] add streaming methods StreamSanitizeUserPrompt and
([a1ab487](a1ab487))
* [netapp] add ScaleType for Storage Pools and LargeCapacityConfig
([a1ab487](a1ab487))
* [redis-cluster] [Memorystore for Redis Cluster] Updating new node
([d3b76d9](d3b76d9))
* [redis-cluster][Memorystore for Redis Cluster] Updating new node
([d3b76d9](d3b76d9))
* [shopping-merchant-products] a new optional field `video_links` is
([a1ab487](a1ab487))
* [shopping-merchant-reports] add `store_type` to
([d3b76d9](d3b76d9))
* [valkey] [Memorystore for Valkey] Updating new node types added
([d3b76d9](d3b76d9))
* [valkey] [Memorystore for Valkey] Updating new node types added
([d3b76d9](d3b76d9))
* [vectorsearch] Added CMEK support
([531942b](531942b))
* **bqjdbc:** Bypass dry-run job for read-only tokens.
([#12961](#12961))
([cd57169](cd57169))
* **bqjdbc:** Integrate the PerConnectionFileHandler with
BigQueryJdbcRootLogger
([#12933](#12933))
([2e56184](2e56184))
* **bqjdbc:** Per connection logs - Add BigQueryJdbcMdc
([#12833](#12833))
([f562667](f562667))
* **bqjdbc:** Per connection logs - Add PerConnectionFileHandler
([#12899](#12899))
([5846197](5846197))
* **datastore:** Bump to v3 major version
([#12989](#12989))
([df20994](df20994))
* **datastore:** Enable Otel metrics for custom Otel
([#12969](#12969))
([1721e7c](1721e7c))
* **Datastore:** Introduce Client Side Metrics
([#12718](#12718))
([552a34d](552a34d))
* **datastore:** Remove deprecated classes and methods and bump
ObsoleteApi to Deprecated
([#12971](#12971))
([0bca75c](0bca75c))
* **Datastore:** Swap the default transport from HttpJson to gRPC
([#12977](#12977))
([24fb234](24fb234))
* **datastore:** Update default channel pool configs to handle initial
bursts and scalability
([#12883](#12883))
([26fe0f9](26fe0f9))
* **gax-grpc:** add configurable resize delta and warning for repeated
resizing
([#12838](#12838))
([2caf026](2caf026))
* **spanner:** add connection properties for min/max RPCs for DCP
([#12951](#12951))
([dc1216e](dc1216e))
* **spanner:** add connection property for enabling/disabling grpc-gcp
([#12898](#12898))
([1e633d7](1e633d7))
* **spanner:** add shared endpoint cooldowns for location-aware
rerouting
([#12845](#12845))
([f5f273b](f5f273b))
* **spanner:** Cleanup GcpFallbackChannel creation and enable by default
([#12707](#12707))
([5251219](5251219))
* **storage:** Implement deleteSourceObjects for Compose Operation
([#12873](#12873))
([2cecd0c](2cecd0c))


### Bug Fixes

* **bq:** do not call 'getQueryResults' for stateful queries via 'query'
api
([#12999](#12999))
([44e9789](44e9789))
* **bqjdbc:** add default OAuth client id/secret
([#12946](#12946))
([9b5c4fa](9b5c4fa))
* **bqjdbc:** add Google Driver scope to all credential types
([#12847](#12847))
([5c890f8](5c890f8))
* **bqjdbc:** enhance logging with caller inference and explicit
exception tracking
([#12903](#12903))
([ce4969b](ce4969b))
* **bqjdbc:** Log exception messages - part 2
([#12907](#12907))
([5215b11](5215b11))
* **bqjdbc:** Log exception messages - part 3
([#12920](#12920))
([45b572f](45b572f))
* **bqjdbc:** metadata methods & getSqlTypeName for struct
([#12947](#12947))
([37555fb](37555fb))
* **bqjdbc:** optimize formatter in BigQueryJdbcRootLogger
([#12877](#12877))
([4233faf](4233faf))
* **bqjdbc:** Revert DatabaseMetaData field to be non-static in
BigQueryConnection
([#12778](#12778))
([ac69c8d](ac69c8d))
* **bqjdbc:** support Service Account Impersonation in ADC
([#12967](#12967))
([8ce183c](8ce183c))
* bump vectorsearch to 0.13.0 for partial release
([#12805](#12805))
([5208fc9](5208fc9))
* **ci:** update java-showcase path in excluded_modules
([#12984](#12984))
([3456ee9](3456ee9))
* correct build directory paths in graalvm cloudbuild.yaml
([#12794](#12794))
([8e6ba36](8e6ba36))
* **datastore:** Create a plaintext gRPC transport channel when using
the Emulator
([#12721](#12721))
([4bed8fd](4bed8fd))
* **datastore:** Update initial ChannelPool configs according to
Datastore best practice guide
([#12919](#12919))
([851fb89](851fb89))
* **deps:** update the Java code generator (gapic-generator-java) to
([531942b](531942b))
* do not generate Version.java files by default
([#12955](#12955))
([43a888a](43a888a))
* **gax:** record fractional latency metrics
([#12979](#12979))
([d690333](d690333))
* **gax:** Remove strict validation that resize delta must be less than
max channel count
([#12863](#12863))
([26b06f9](26b06f9))
* **generator:** use json_name when available
([#12940](#12940))
([622955b](622955b))
* manual preservation of Version.java files
([#12862](#12862))
([3d65c78](3d65c78))
* remove google/rpc exclusion to generate HttpResponse
([#12992](#12992))
([24b1719](24b1719))
* **spanner:** fix flakiness and race conditions in multiplexed session
tests
([#12949](#12949))
([3e02f18](3e02f18))
* **spanner:** fix flakiness in testCreateSessionDeadlineExceeded
([#12944](#12944))
([93f21ed](93f21ed))
* Update renovate config check template to use npx
([#12865](#12865))
([b974740](b974740))
* use org.junit.jupiter.api.Assertions.assertThrow
([#12882](#12882))
([bf243a6](bf243a6))


### Performance Improvements

* **spanner:** use StringBuilder for generating RequestId
([#12809](#12809))
([5c821a3](5c821a3))


### Dependencies

* upgrade opentelemetry
([#12953](#12953))
([522e05b](522e05b))


### Documentation

* [cloudsecuritycompliance] Updated docs for the APIs
([a1ab487](a1ab487))
* [kms] Update the comment for duration value
([a1ab487](a1ab487))
* [saasservicemgmt] rebrand from "SaaS Runtime" to "App Lifecycle
([a1ab487](a1ab487))
* Add a guide on configuring ChannelPools for gRPC
([#12905](#12905))
([ea081fe](ea081fe))
* update bazel targets in DEVELOPMENT.md to point to sdk-platform-java
([#12844](#12844))
([4568284](4568284))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Jin Seop Kim <jinseop@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@olavloite olavloite olavloite approved these changes

@rahul2393 rahul2393 rahul2393 approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sakthivelmanii @olavloite @rahul2393