Skip to content

Commit da177d5

Browse files
authored
chore: unflake SavepointMockServerTest (#3237)
* chore: unflake SavepointMockServerTest * fix: use separate tag to verify no keep-alive requests
1 parent d2d942e commit da177d5

File tree

2 files changed

+17
-34
lines changed

2 files changed

+17
-34
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,10 @@ public void run() {
461461
CallType.SYNC,
462462
SELECT1_STATEMENT,
463463
AnalyzeMode.NONE,
464-
Options.tag("connection.transaction-keep-alive"));
464+
Options.tag(
465+
System.getProperty(
466+
"spanner.connection.keep_alive_query_tag",
467+
"connection.transaction-keep-alive")));
465468
future.addListener(
466469
ReadWriteTransaction.this::maybeScheduleKeepAlivePing, MoreExecutors.directExecutor());
467470
}

google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/SavepointMockServerTest.java

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -682,77 +682,57 @@ public void testRollbackToSavepointWithoutInternalRetriesInReadOnlyTransaction()
682682

683683
@Test
684684
public void testKeepAlive() throws InterruptedException, TimeoutException {
685+
String keepAliveTag = "test_keep_alive_tag";
685686
System.setProperty("spanner.connection.keep_alive_interval_millis", "1");
687+
System.setProperty("spanner.connection.keep_alive_query_tag", keepAliveTag);
686688
try (Connection connection = createConnection()) {
687689
connection.setSavepointSupport(SavepointSupport.ENABLED);
688690
connection.setKeepTransactionAlive(true);
689691
// Start a transaction by executing a statement.
690692
connection.execute(INSERT_STATEMENT);
691693
// Verify that we get a keep-alive request.
692-
verifyHasKeepAliveRequest();
694+
verifyHasKeepAliveRequest(keepAliveTag);
693695
// Set a savepoint, execute another statement, and rollback to the savepoint.
694696
// The keep-alive should not be sent after the transaction has been rolled back to the
695697
// savepoint.
696698
connection.savepoint("s1");
697699
connection.execute(INSERT_STATEMENT);
698700
connection.rollbackToSavepoint("s1");
699701
mockSpanner.waitForRequestsToContain(RollbackRequest.class, 1000L);
700-
// Wait for up to 2 milliseconds to make sure that any keep-alive requests that were in flight
701-
// have finished.
702-
try {
703-
mockSpanner.waitForRequestsToContain(
704-
r -> {
705-
if (!(r instanceof ExecuteSqlRequest)) {
706-
return false;
707-
}
708-
ExecuteSqlRequest request = (ExecuteSqlRequest) r;
709-
return request.getSql().equals("SELECT 1")
710-
&& request
711-
.getRequestOptions()
712-
.getRequestTag()
713-
.equals("connection.transaction-keep-alive");
714-
},
715-
2L);
716-
} catch (TimeoutException ignore) {
717-
}
702+
String keepAliveTagAfterRollback = "test_keep_alive_tag_after_rollback";
703+
System.setProperty("spanner.connection.keep_alive_query_tag", keepAliveTagAfterRollback);
718704

719-
// Verify that we don't get any keep-alive requests from this point.
720-
mockSpanner.clearRequests();
705+
// Verify that we don't get any new keep-alive requests from this point.
721706
Thread.sleep(2L);
722-
assertEquals(0, countKeepAliveRequest());
707+
assertEquals(0, countKeepAliveRequest(keepAliveTagAfterRollback));
723708
// Resume the transaction and verify that we get a keep-alive again.
724709
connection.execute(INSERT_STATEMENT);
725-
verifyHasKeepAliveRequest();
710+
verifyHasKeepAliveRequest(keepAliveTagAfterRollback);
726711
} finally {
727712
System.clearProperty("spanner.connection.keep_alive_interval_millis");
713+
System.clearProperty("spanner.connection.keep_alive_query_tag");
728714
}
729715
}
730716

731-
private void verifyHasKeepAliveRequest() throws InterruptedException, TimeoutException {
717+
private void verifyHasKeepAliveRequest(String tag) throws InterruptedException, TimeoutException {
732718
mockSpanner.waitForRequestsToContain(
733719
r -> {
734720
if (!(r instanceof ExecuteSqlRequest)) {
735721
return false;
736722
}
737723
ExecuteSqlRequest request = (ExecuteSqlRequest) r;
738724
return request.getSql().equals("SELECT 1")
739-
&& request
740-
.getRequestOptions()
741-
.getRequestTag()
742-
.equals("connection.transaction-keep-alive");
725+
&& request.getRequestOptions().getRequestTag().equals(tag);
743726
},
744727
1000L);
745728
}
746729

747-
private long countKeepAliveRequest() {
730+
private long countKeepAliveRequest(String tag) {
748731
return mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream()
749732
.filter(
750733
request ->
751734
request.getSql().equals("SELECT 1")
752-
&& request
753-
.getRequestOptions()
754-
.getRequestTag()
755-
.equals("connection.transaction-keep-alive"))
735+
&& request.getRequestOptions().getRequestTag().equals(tag))
756736
.count();
757737
}
758738
}

0 commit comments

Comments
 (0)