Skip to content
This repository was archived by the owner on Apr 7, 2026. It is now read-only.

Commit d54f252

Browse files
olavloiteolavloite
authored
fix: retries of updates in the Connection API ignored analyze mode (#2010)
* fix: retries of updates in the Connection API ignored analyze mode Retries of read/write transactions in the Connection API did not respect the analyze mode of update statements. This would cause updates to be executed using AnalyzeMode.NORMAL during retries, regardless of what was used during the initial attempt. Fixes #2009 * fix: remove null check and force not-null
1 parent 33aa21c commit d54f252

3 files changed

Lines changed: 55 additions & 8 deletions

File tree

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ReadWriteTransaction.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ private ApiFuture<ResultSetStats> internalExecuteUpdateAsync(
444444
options);
445445
}
446446
createAndAddRetriableUpdate(
447-
update, updateCount.getRowCountExact(), options);
447+
update, analyzeMode, updateCount.getRowCountExact(), options);
448448
return updateCount;
449449
} catch (AbortedException e) {
450450
throw e;
@@ -712,9 +712,9 @@ private void createAndAddFailedQuery(
712712
}
713713

714714
private void createAndAddRetriableUpdate(
715-
ParsedStatement update, long updateCount, UpdateOption... options) {
715+
ParsedStatement update, AnalyzeMode analyzeMode, long updateCount, UpdateOption... options) {
716716
if (retryAbortsInternally) {
717-
addRetryStatement(new RetriableUpdate(this, update, updateCount, options));
717+
addRetryStatement(new RetriableUpdate(this, update, analyzeMode, updateCount, options));
718718
}
719719
}
720720

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/RetriableUpdate.java‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,19 @@
3131
final class RetriableUpdate implements RetriableStatement {
3232
private final ReadWriteTransaction transaction;
3333
private final ParsedStatement statement;
34+
private final AnalyzeMode analyzeMode;
3435
private final long updateCount;
3536
private final UpdateOption[] options;
3637

3738
RetriableUpdate(
3839
ReadWriteTransaction transaction,
3940
ParsedStatement statement,
41+
AnalyzeMode analyzeMode,
4042
long updateCount,
4143
UpdateOption... options) {
42-
Preconditions.checkNotNull(transaction);
43-
Preconditions.checkNotNull(statement);
44-
this.transaction = transaction;
45-
this.statement = statement;
44+
this.transaction = Preconditions.checkNotNull(transaction);
45+
this.statement = Preconditions.checkNotNull(statement);
46+
this.analyzeMode = Preconditions.checkNotNull(analyzeMode);
4647
this.updateCount = updateCount;
4748
this.options = options;
4849
}
@@ -54,7 +55,15 @@ public void retry(AbortedException aborted) throws AbortedException {
5455
transaction
5556
.getStatementExecutor()
5657
.invokeInterceptors(statement, StatementExecutionStep.RETRY_STATEMENT, transaction);
57-
newCount = transaction.getReadContext().executeUpdate(statement.getStatement(), options);
58+
if (analyzeMode == AnalyzeMode.NONE) {
59+
newCount = transaction.getReadContext().executeUpdate(statement.getStatement(), options);
60+
} else {
61+
newCount =
62+
transaction
63+
.getReadContext()
64+
.analyzeUpdate(statement.getStatement(), analyzeMode.getQueryAnalyzeMode())
65+
.getRowCountExact();
66+
}
5867
} catch (AbortedException e) {
5968
// Just re-throw the AbortedException and let the retry logic determine whether another try
6069
// should be executed or not.

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AbortedTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import com.google.cloud.Timestamp;
2727
import com.google.cloud.spanner.ErrorCode;
2828
import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult;
29+
import com.google.cloud.spanner.ReadContext.QueryAnalyzeMode;
2930
import com.google.cloud.spanner.ResultSet;
3031
import com.google.cloud.spanner.SpannerException;
3132
import com.google.cloud.spanner.Statement;
@@ -37,9 +38,11 @@
3738
import com.google.spanner.v1.CommitRequest;
3839
import com.google.spanner.v1.ExecuteBatchDmlRequest;
3940
import com.google.spanner.v1.ExecuteSqlRequest;
41+
import com.google.spanner.v1.ExecuteSqlRequest.QueryMode;
4042
import io.grpc.Status;
4143
import io.grpc.StatusRuntimeException;
4244
import java.util.Collections;
45+
import java.util.List;
4346
import org.junit.Test;
4447
import org.junit.runner.RunWith;
4548
import org.junit.runners.JUnit4;
@@ -299,6 +302,41 @@ public void testRetryUsesTags() {
299302
assertEquals(2L, commitRequestCount);
300303
}
301304

305+
@Test
306+
public void testRetryUsesAnalyzeModeForUpdate() {
307+
mockSpanner.putStatementResult(
308+
StatementResult.query(SELECT_COUNT_STATEMENT, SELECT_COUNT_RESULTSET_BEFORE_INSERT));
309+
mockSpanner.putStatementResult(StatementResult.update(INSERT_STATEMENT, 0));
310+
try (ITConnection connection = createConnection()) {
311+
assertEquals(
312+
0L, connection.analyzeUpdate(INSERT_STATEMENT, QueryAnalyzeMode.PLAN).getRowCountExact());
313+
314+
mockSpanner.abortNextStatement();
315+
connection.executeQuery(SELECT_COUNT_STATEMENT);
316+
317+
mockSpanner.putStatementResult(StatementResult.update(INSERT_STATEMENT, 1));
318+
assertEquals(1L, connection.executeUpdate(INSERT_STATEMENT));
319+
320+
connection.commit();
321+
}
322+
// 5 requests because:
323+
// 1. Analyze INSERT
324+
// 2. Execute SELECT COUNT(*) (Aborted)
325+
// 3. Analyze INSERT (retry)
326+
// 4. Execute SELECT COUNT(*) (retry)
327+
// 5. Execute INSERT
328+
assertEquals(5, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class));
329+
List<ExecuteSqlRequest> requests = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class);
330+
assertEquals(QueryMode.PLAN, requests.get(0).getQueryMode());
331+
assertEquals(QueryMode.NORMAL, requests.get(1).getQueryMode());
332+
333+
// This used NORMAL because of https://github.com/googleapis/java-spanner/issues/2009.
334+
assertEquals(QueryMode.PLAN, requests.get(2).getQueryMode());
335+
336+
assertEquals(QueryMode.NORMAL, requests.get(3).getQueryMode());
337+
assertEquals(QueryMode.NORMAL, requests.get(4).getQueryMode());
338+
}
339+
302340
ITConnection createConnection(TransactionRetryListener listener) {
303341
ITConnection connection =
304342
super.createConnection(ImmutableList.of(), ImmutableList.of(listener));

0 commit comments

Comments
 (0)