Skip to content

test(spanner): deflake testCreateSessionDeadlineExceededWithNoSessionCreateWaitTime - #14285

Open
sakthivelmanii wants to merge 2 commits into
mainfrom
deflake-multiplexed-session-mock-test
Open

test(spanner): deflake testCreateSessionDeadlineExceededWithNoSessionCreateWaitTime#14285
sakthivelmanii wants to merge 2 commits into
mainfrom
deflake-multiplexed-session-mock-test

Conversation

@sakthivelmanii

Copy link
Copy Markdown
Contributor

Freeze MockSpanner before initializing DatabaseClient to deterministically bind the transaction to the initial failing CreateSession attempt, and enclose ReadContext in try-with-resources to ensure proper transaction cleanup.

…CreateWaitTime

Freeze MockSpanner before initializing DatabaseClient to deterministically bind the transaction to the initial failing CreateSession attempt, and enclose ReadContext in try-with-resources to ensure proper transaction cleanup.
@sakthivelmanii
sakthivelmanii requested review from a team as code owners September 7, 2026 15:32

@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 updates the MultiplexedSessionDatabaseClientMockServerTest to wait for the initial CreateSession request to reach the mock server and freeze before acquiring the ReadContext. However, a review comment points out a potential race condition where calling mockSpanner.unfreeze() before executing the query could cause the client to clear the failed session and initiate a new successful request. The reviewer suggests calling unfreeze() after executing the query but before calling resultSet.next() to guarantee the query binds to the initial failing attempt.

Comment on lines +127 to 136
try (ReadContext readContext = client.singleUse()) {
mockSpanner.unfreeze();

// The first attempt should lead to a DEADLINE_EXCEEDED error being propagated from the
// CreateSession attempt.
try (ResultSet resultSet = readContext.executeQuery(STATEMENT)) {
SpannerException exception = assertThrows(SpannerException.class, resultSet::next);
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
}
}

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.

medium

Calling mockSpanner.unfreeze() before readContext.executeQuery(STATEMENT) introduces a potential race condition. If the mock server processes the unfrozen CreateSession request and the client receives the DEADLINE_EXCEEDED failure before executeQuery is executed, the client might clear the failed session attempt and initiate a new CreateSession request for the query. Since the simulated exception is not sticky, this new request would succeed, causing the test to fail.

To guarantee that the query binds to the initial failing attempt, we should call readContext.executeQuery(STATEMENT) while the server is still frozen, and then call mockSpanner.unfreeze() before calling resultSet.next() (which is where the blocking and exception propagation actually happen).

    try (ReadContext readContext = client.singleUse()) {
      // The first attempt should lead to a DEADLINE_EXCEEDED error being propagated from the
      // CreateSession attempt.
      try (ResultSet resultSet = readContext.executeQuery(STATEMENT)) {
        mockSpanner.unfreeze();
        SpannerException exception = assertThrows(SpannerException.class, resultSet::next);
        assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
      }
    }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

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.

1 participant