Skip to content

Commit fae1585

Browse files
olavloiterahul2393
olavloite
authored and
rahul2393
committed
test: ignore if the mock server failed to return a row (#3335)
The test uses a trick in the mock server, where the mock server is requested to freeze after returning the first row. However, when the mock server adds the first row to the stream, it is not guaranteed to be readable for the client, which again causes the test to hang on the ResultSet#next() call. The gRPC libraries then execute keep-alive requests to keep the TCP connection alive while waiting for data from the mock server, which will never come. This caused the query to eventually fail with a RESOURCE_EXHAUSTED error. The tests work around this issue by just ignoring the case when the mock server fails to return the first row, as it is something that only very sporadically happens.
1 parent b3e2b0f commit fae1585

32 files changed

+4121
-260
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/v1/SpannerClient.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,6 +1629,7 @@ public final Transaction beginTransaction(String session, TransactionOptions opt
16291629
* SessionName.of("[PROJECT]", "[INSTANCE]", "[DATABASE]", "[SESSION]").toString())
16301630
* .setOptions(TransactionOptions.newBuilder().build())
16311631
* .setRequestOptions(RequestOptions.newBuilder().build())
1632+
* .setMutationKey(Mutation.newBuilder().build())
16321633
* .build();
16331634
* Transaction response = spannerClient.beginTransaction(request);
16341635
* }
@@ -1662,6 +1663,7 @@ public final Transaction beginTransaction(BeginTransactionRequest request) {
16621663
* SessionName.of("[PROJECT]", "[INSTANCE]", "[DATABASE]", "[SESSION]").toString())
16631664
* .setOptions(TransactionOptions.newBuilder().build())
16641665
* .setRequestOptions(RequestOptions.newBuilder().build())
1666+
* .setMutationKey(Mutation.newBuilder().build())
16651667
* .build();
16661668
* ApiFuture<Transaction> future = spannerClient.beginTransactionCallable().futureCall(request);
16671669
* // Do something.
@@ -1911,6 +1913,7 @@ public final CommitResponse commit(
19111913
* .setReturnCommitStats(true)
19121914
* .setMaxCommitDelay(Duration.newBuilder().build())
19131915
* .setRequestOptions(RequestOptions.newBuilder().build())
1916+
* .setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
19141917
* .build();
19151918
* CommitResponse response = spannerClient.commit(request);
19161919
* }
@@ -1955,6 +1958,7 @@ public final CommitResponse commit(CommitRequest request) {
19551958
* .setReturnCommitStats(true)
19561959
* .setMaxCommitDelay(Duration.newBuilder().build())
19571960
* .setRequestOptions(RequestOptions.newBuilder().build())
1961+
* .setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
19581962
* .build();
19591963
* ApiFuture<CommitResponse> future = spannerClient.commitCallable().futureCall(request);
19601964
* // Do something.

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/v1/SpannerSettings.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@
7676
* <p>The builder of this class is recursive, so contained classes are themselves builders. When
7777
* build() is called, the tree of builders is called to create the complete settings object.
7878
*
79-
* <p>For example, to set the total timeout of createSession to 30 seconds:
79+
* <p>For example, to set the
80+
* [RetrySettings](https://cloud.google.com/java/docs/reference/gax/latest/com.google.api.gax.retrying.RetrySettings)
81+
* of createSession:
8082
*
8183
* <pre>{@code
8284
* // This snippet has been automatically generated and should be regarded as a code template only.
@@ -92,10 +94,21 @@
9294
* .createSessionSettings()
9395
* .getRetrySettings()
9496
* .toBuilder()
95-
* .setTotalTimeout(Duration.ofSeconds(30))
97+
* .setInitialRetryDelayDuration(Duration.ofSeconds(1))
98+
* .setInitialRpcTimeoutDuration(Duration.ofSeconds(5))
99+
* .setMaxAttempts(5)
100+
* .setMaxRetryDelayDuration(Duration.ofSeconds(30))
101+
* .setMaxRpcTimeoutDuration(Duration.ofSeconds(60))
102+
* .setRetryDelayMultiplier(1.3)
103+
* .setRpcTimeoutMultiplier(1.5)
104+
* .setTotalTimeoutDuration(Duration.ofSeconds(300))
96105
* .build());
97106
* SpannerSettings spannerSettings = spannerSettingsBuilder.build();
98107
* }</pre>
108+
*
109+
* Please refer to the [Client Side Retry
110+
* Guide](https://github.com/googleapis/google-cloud-java/blob/main/docs/client_retries.md) for
111+
* additional support in setting retries.
99112
*/
100113
@Generated("by gapic-generator-java")
101114
public class SpannerSettings extends ClientSettings<SpannerSettings> {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/v1/stub/SpannerStubSettings.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@
9494
* <p>The builder of this class is recursive, so contained classes are themselves builders. When
9595
* build() is called, the tree of builders is called to create the complete settings object.
9696
*
97-
* <p>For example, to set the total timeout of createSession to 30 seconds:
97+
* <p>For example, to set the
98+
* [RetrySettings](https://cloud.google.com/java/docs/reference/gax/latest/com.google.api.gax.retrying.RetrySettings)
99+
* of createSession:
98100
*
99101
* <pre>{@code
100102
* // This snippet has been automatically generated and should be regarded as a code template only.
@@ -110,10 +112,21 @@
110112
* .createSessionSettings()
111113
* .getRetrySettings()
112114
* .toBuilder()
113-
* .setTotalTimeout(Duration.ofSeconds(30))
115+
* .setInitialRetryDelayDuration(Duration.ofSeconds(1))
116+
* .setInitialRpcTimeoutDuration(Duration.ofSeconds(5))
117+
* .setMaxAttempts(5)
118+
* .setMaxRetryDelayDuration(Duration.ofSeconds(30))
119+
* .setMaxRpcTimeoutDuration(Duration.ofSeconds(60))
120+
* .setRetryDelayMultiplier(1.3)
121+
* .setRpcTimeoutMultiplier(1.5)
122+
* .setTotalTimeoutDuration(Duration.ofSeconds(300))
114123
* .build());
115124
* SpannerStubSettings spannerSettings = spannerSettingsBuilder.build();
116125
* }</pre>
126+
*
127+
* Please refer to the [Client Side Retry
128+
* Guide](https://github.com/googleapis/google-cloud-java/blob/main/docs/client_retries.md) for
129+
* additional support in setting retries.
117130
*/
118131
@Generated("by gapic-generator-java")
119132
public class SpannerStubSettings extends StubSettings<SpannerStubSettings> {
@@ -177,9 +190,7 @@ public String extractNextToken(ListSessionsResponse payload) {
177190

178191
@Override
179192
public Iterable<Session> extractResources(ListSessionsResponse payload) {
180-
return payload.getSessionsList() == null
181-
? ImmutableList.<Session>of()
182-
: payload.getSessionsList();
193+
return payload.getSessionsList();
183194
}
184195
};
185196

‎google-cloud-spanner/src/main/resources/META-INF/native-image/com.google.cloud.spanner.v1/reflect-config.json

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,24 @@
305305
"allDeclaredClasses": true,
306306
"allPublicClasses": true
307307
},
308+
{
309+
"name": "com.google.api.PythonSettings$ExperimentalFeatures",
310+
"queryAllDeclaredConstructors": true,
311+
"queryAllPublicConstructors": true,
312+
"queryAllDeclaredMethods": true,
313+
"allPublicMethods": true,
314+
"allDeclaredClasses": true,
315+
"allPublicClasses": true
316+
},
317+
{
318+
"name": "com.google.api.PythonSettings$ExperimentalFeatures$Builder",
319+
"queryAllDeclaredConstructors": true,
320+
"queryAllPublicConstructors": true,
321+
"queryAllDeclaredMethods": true,
322+
"allPublicMethods": true,
323+
"allDeclaredClasses": true,
324+
"allPublicClasses": true
325+
},
308326
{
309327
"name": "com.google.api.ResourceDescriptor",
310328
"queryAllDeclaredConstructors": true,
@@ -1727,6 +1745,24 @@
17271745
"allDeclaredClasses": true,
17281746
"allPublicClasses": true
17291747
},
1748+
{
1749+
"name": "com.google.spanner.v1.MultiplexedSessionPrecommitToken",
1750+
"queryAllDeclaredConstructors": true,
1751+
"queryAllPublicConstructors": true,
1752+
"queryAllDeclaredMethods": true,
1753+
"allPublicMethods": true,
1754+
"allDeclaredClasses": true,
1755+
"allPublicClasses": true
1756+
},
1757+
{
1758+
"name": "com.google.spanner.v1.MultiplexedSessionPrecommitToken$Builder",
1759+
"queryAllDeclaredConstructors": true,
1760+
"queryAllPublicConstructors": true,
1761+
"queryAllDeclaredMethods": true,
1762+
"allPublicMethods": true,
1763+
"allDeclaredClasses": true,
1764+
"allPublicClasses": true
1765+
},
17301766
{
17311767
"name": "com.google.spanner.v1.Mutation",
17321768
"queryAllDeclaredConstructors": true,

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/CloseSpannerWithOpenResultSetTest.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import java.util.concurrent.TimeUnit;
4040
import java.util.stream.Collectors;
4141
import org.junit.After;
42+
import org.junit.AfterClass;
43+
import org.junit.BeforeClass;
4244
import org.junit.Test;
4345
import org.junit.runner.RunWith;
4446
import org.junit.runners.JUnit4;
@@ -59,6 +61,16 @@ Spanner createSpanner() {
5961
.getService();
6062
}
6163

64+
@BeforeClass
65+
public static void setWatchdogTimeout() {
66+
System.setProperty("com.google.cloud.spanner.watchdogTimeoutSeconds", "1");
67+
}
68+
69+
@AfterClass
70+
public static void clearWatchdogTimeout() {
71+
System.clearProperty("com.google.cloud.spanner.watchdogTimeoutSeconds");
72+
}
73+
6274
@After
6375
public void cleanup() {
6476
mockSpanner.unfreeze();
@@ -75,7 +87,13 @@ public void testBatchClient_closedSpannerWithOpenResultSet_streamsAreCancelled()
7587
client.batchReadOnlyTransaction(TimestampBound.strong());
7688
ResultSet resultSet = transaction.executeQuery(SELECT_RANDOM_STATEMENT)) {
7789
mockSpanner.freezeAfterReturningNumRows(1);
78-
assertTrue(resultSet.next());
90+
// This can sometimes fail, as the mock server may not always actually return the first row.
91+
try {
92+
assertTrue(resultSet.next());
93+
} catch (SpannerException exception) {
94+
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
95+
return;
96+
}
7997
((SpannerImpl) spanner).close(1, TimeUnit.MILLISECONDS);
8098
// This should return an error as the stream is cancelled.
8199
SpannerException exception = assertThrows(SpannerException.class, resultSet::next);
@@ -93,7 +111,13 @@ public void testNormalDatabaseClient_closedSpannerWithOpenResultSet_sessionsAreD
93111
try (ReadOnlyTransaction transaction = client.readOnlyTransaction(TimestampBound.strong());
94112
ResultSet resultSet = transaction.executeQuery(SELECT_RANDOM_STATEMENT)) {
95113
mockSpanner.freezeAfterReturningNumRows(1);
96-
assertTrue(resultSet.next());
114+
// This can sometimes fail, as the mock server may not always actually return the first row.
115+
try {
116+
assertTrue(resultSet.next());
117+
} catch (SpannerException exception) {
118+
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
119+
return;
120+
}
97121
List<ExecuteSqlRequest> executeSqlRequests =
98122
mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream()
99123
.filter(request -> request.getSql().equals(SELECT_RANDOM_STATEMENT.getSql()))

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/v1/SpannerClientHttpJsonTest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import com.google.spanner.v1.ExecuteSqlRequest;
4545
import com.google.spanner.v1.KeySet;
4646
import com.google.spanner.v1.ListSessionsResponse;
47+
import com.google.spanner.v1.MultiplexedSessionPrecommitToken;
4748
import com.google.spanner.v1.Mutation;
4849
import com.google.spanner.v1.Partition;
4950
import com.google.spanner.v1.PartitionOptions;
@@ -587,6 +588,7 @@ public void executeSqlTest() throws Exception {
587588
.setMetadata(ResultSetMetadata.newBuilder().build())
588589
.addAllRows(new ArrayList<ListValue>())
589590
.setStats(ResultSetStats.newBuilder().build())
591+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
590592
.build();
591593
mockService.addResponse(expectedResponse);
592594

@@ -673,6 +675,7 @@ public void executeBatchDmlTest() throws Exception {
673675
ExecuteBatchDmlResponse.newBuilder()
674676
.addAllResultSets(new ArrayList<ResultSet>())
675677
.setStatus(Status.newBuilder().build())
678+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
676679
.build();
677680
mockService.addResponse(expectedResponse);
678681

@@ -735,6 +738,7 @@ public void readTest() throws Exception {
735738
.setMetadata(ResultSetMetadata.newBuilder().build())
736739
.addAllRows(new ArrayList<ListValue>())
737740
.setStats(ResultSetStats.newBuilder().build())
741+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
738742
.build();
739743
mockService.addResponse(expectedResponse);
740744

@@ -821,6 +825,7 @@ public void beginTransactionTest() throws Exception {
821825
Transaction.newBuilder()
822826
.setId(ByteString.EMPTY)
823827
.setReadTimestamp(Timestamp.newBuilder().build())
828+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
824829
.build();
825830
mockService.addResponse(expectedResponse);
826831

@@ -868,6 +873,7 @@ public void beginTransactionTest2() throws Exception {
868873
Transaction.newBuilder()
869874
.setId(ByteString.EMPTY)
870875
.setReadTimestamp(Timestamp.newBuilder().build())
876+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
871877
.build();
872878
mockService.addResponse(expectedResponse);
873879

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/v1/SpannerClientTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import com.google.spanner.v1.KeySet;
5656
import com.google.spanner.v1.ListSessionsRequest;
5757
import com.google.spanner.v1.ListSessionsResponse;
58+
import com.google.spanner.v1.MultiplexedSessionPrecommitToken;
5859
import com.google.spanner.v1.Mutation;
5960
import com.google.spanner.v1.PartialResultSet;
6061
import com.google.spanner.v1.Partition;
@@ -545,6 +546,7 @@ public void executeSqlTest() throws Exception {
545546
.setMetadata(ResultSetMetadata.newBuilder().build())
546547
.addAllRows(new ArrayList<ListValue>())
547548
.setStats(ResultSetStats.newBuilder().build())
549+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
548550
.build();
549551
mockSpanner.addResponse(expectedResponse);
550552

@@ -629,6 +631,7 @@ public void executeStreamingSqlTest() throws Exception {
629631
.setChunkedValue(true)
630632
.setResumeToken(ByteString.EMPTY)
631633
.setStats(ResultSetStats.newBuilder().build())
634+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
632635
.build();
633636
mockSpanner.addResponse(expectedResponse);
634637
ExecuteSqlRequest request =
@@ -702,6 +705,7 @@ public void executeBatchDmlTest() throws Exception {
702705
ExecuteBatchDmlResponse.newBuilder()
703706
.addAllResultSets(new ArrayList<ResultSet>())
704707
.setStatus(Status.newBuilder().build())
708+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
705709
.build();
706710
mockSpanner.addResponse(expectedResponse);
707711

@@ -762,6 +766,7 @@ public void readTest() throws Exception {
762766
.setMetadata(ResultSetMetadata.newBuilder().build())
763767
.addAllRows(new ArrayList<ListValue>())
764768
.setStats(ResultSetStats.newBuilder().build())
769+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
765770
.build();
766771
mockSpanner.addResponse(expectedResponse);
767772

@@ -847,6 +852,7 @@ public void streamingReadTest() throws Exception {
847852
.setChunkedValue(true)
848853
.setResumeToken(ByteString.EMPTY)
849854
.setStats(ResultSetStats.newBuilder().build())
855+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
850856
.build();
851857
mockSpanner.addResponse(expectedResponse);
852858
ReadRequest request =
@@ -920,6 +926,7 @@ public void beginTransactionTest() throws Exception {
920926
Transaction.newBuilder()
921927
.setId(ByteString.EMPTY)
922928
.setReadTimestamp(Timestamp.newBuilder().build())
929+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
923930
.build();
924931
mockSpanner.addResponse(expectedResponse);
925932

@@ -962,6 +969,7 @@ public void beginTransactionTest2() throws Exception {
962969
Transaction.newBuilder()
963970
.setId(ByteString.EMPTY)
964971
.setReadTimestamp(Timestamp.newBuilder().build())
972+
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
965973
.build();
966974
mockSpanner.addResponse(expectedResponse);
967975

0 commit comments

Comments
 (0)