test(spanner): deflake testCreateSessionDeadlineExceededWithNoSessionCreateWaitTime - #14285
test(spanner): deflake testCreateSessionDeadlineExceededWithNoSessionCreateWaitTime#14285sakthivelmanii wants to merge 2 commits into
Conversation
…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.
There was a problem hiding this comment.
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.
| 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()); | ||
| } | ||
| } |
There was a problem hiding this comment.
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());
}
}
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.