Skip to content

Commit

Permalink
test: ignore if the mock server failed to return a row (#3335)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
olavloite authored and rahul2393 committed Sep 19, 2024
1 parent b3e2b0f commit fae1585
Show file tree
Hide file tree
Showing 32 changed files with 4,121 additions and 260 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1629,6 +1629,7 @@ public final Transaction beginTransaction(String session, TransactionOptions opt
* SessionName.of("[PROJECT]", "[INSTANCE]", "[DATABASE]", "[SESSION]").toString())
* .setOptions(TransactionOptions.newBuilder().build())
* .setRequestOptions(RequestOptions.newBuilder().build())
* .setMutationKey(Mutation.newBuilder().build())
* .build();
* Transaction response = spannerClient.beginTransaction(request);
* }
Expand Down Expand Up @@ -1662,6 +1663,7 @@ public final Transaction beginTransaction(BeginTransactionRequest request) {
* SessionName.of("[PROJECT]", "[INSTANCE]", "[DATABASE]", "[SESSION]").toString())
* .setOptions(TransactionOptions.newBuilder().build())
* .setRequestOptions(RequestOptions.newBuilder().build())
* .setMutationKey(Mutation.newBuilder().build())
* .build();
* ApiFuture<Transaction> future = spannerClient.beginTransactionCallable().futureCall(request);
* // Do something.
Expand Down Expand Up @@ -1911,6 +1913,7 @@ public final CommitResponse commit(
* .setReturnCommitStats(true)
* .setMaxCommitDelay(Duration.newBuilder().build())
* .setRequestOptions(RequestOptions.newBuilder().build())
* .setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
* .build();
* CommitResponse response = spannerClient.commit(request);
* }
Expand Down Expand Up @@ -1955,6 +1958,7 @@ public final CommitResponse commit(CommitRequest request) {
* .setReturnCommitStats(true)
* .setMaxCommitDelay(Duration.newBuilder().build())
* .setRequestOptions(RequestOptions.newBuilder().build())
* .setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
* .build();
* ApiFuture<CommitResponse> future = spannerClient.commitCallable().futureCall(request);
* // Do something.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@
* <p>The builder of this class is recursive, so contained classes are themselves builders. When
* build() is called, the tree of builders is called to create the complete settings object.
*
* <p>For example, to set the total timeout of createSession to 30 seconds:
* <p>For example, to set the
* [RetrySettings](https://cloud.google.com/java/docs/reference/gax/latest/com.google.api.gax.retrying.RetrySettings)
* of createSession:
*
* <pre>{@code
* // This snippet has been automatically generated and should be regarded as a code template only.
Expand All @@ -92,10 +94,21 @@
* .createSessionSettings()
* .getRetrySettings()
* .toBuilder()
* .setTotalTimeout(Duration.ofSeconds(30))
* .setInitialRetryDelayDuration(Duration.ofSeconds(1))
* .setInitialRpcTimeoutDuration(Duration.ofSeconds(5))
* .setMaxAttempts(5)
* .setMaxRetryDelayDuration(Duration.ofSeconds(30))
* .setMaxRpcTimeoutDuration(Duration.ofSeconds(60))
* .setRetryDelayMultiplier(1.3)
* .setRpcTimeoutMultiplier(1.5)
* .setTotalTimeoutDuration(Duration.ofSeconds(300))
* .build());
* SpannerSettings spannerSettings = spannerSettingsBuilder.build();
* }</pre>
*
* Please refer to the [Client Side Retry
* Guide](https://github.com/googleapis/google-cloud-java/blob/main/docs/client_retries.md) for
* additional support in setting retries.
*/
@Generated("by gapic-generator-java")
public class SpannerSettings extends ClientSettings<SpannerSettings> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@
* <p>The builder of this class is recursive, so contained classes are themselves builders. When
* build() is called, the tree of builders is called to create the complete settings object.
*
* <p>For example, to set the total timeout of createSession to 30 seconds:
* <p>For example, to set the
* [RetrySettings](https://cloud.google.com/java/docs/reference/gax/latest/com.google.api.gax.retrying.RetrySettings)
* of createSession:
*
* <pre>{@code
* // This snippet has been automatically generated and should be regarded as a code template only.
Expand All @@ -110,10 +112,21 @@
* .createSessionSettings()
* .getRetrySettings()
* .toBuilder()
* .setTotalTimeout(Duration.ofSeconds(30))
* .setInitialRetryDelayDuration(Duration.ofSeconds(1))
* .setInitialRpcTimeoutDuration(Duration.ofSeconds(5))
* .setMaxAttempts(5)
* .setMaxRetryDelayDuration(Duration.ofSeconds(30))
* .setMaxRpcTimeoutDuration(Duration.ofSeconds(60))
* .setRetryDelayMultiplier(1.3)
* .setRpcTimeoutMultiplier(1.5)
* .setTotalTimeoutDuration(Duration.ofSeconds(300))
* .build());
* SpannerStubSettings spannerSettings = spannerSettingsBuilder.build();
* }</pre>
*
* Please refer to the [Client Side Retry
* Guide](https://github.com/googleapis/google-cloud-java/blob/main/docs/client_retries.md) for
* additional support in setting retries.
*/
@Generated("by gapic-generator-java")
public class SpannerStubSettings extends StubSettings<SpannerStubSettings> {
Expand Down Expand Up @@ -177,9 +190,7 @@ public String extractNextToken(ListSessionsResponse payload) {

@Override
public Iterable<Session> extractResources(ListSessionsResponse payload) {
return payload.getSessionsList() == null
? ImmutableList.<Session>of()
: payload.getSessionsList();
return payload.getSessionsList();
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,24 @@
"allDeclaredClasses": true,
"allPublicClasses": true
},
{
"name": "com.google.api.PythonSettings$ExperimentalFeatures",
"queryAllDeclaredConstructors": true,
"queryAllPublicConstructors": true,
"queryAllDeclaredMethods": true,
"allPublicMethods": true,
"allDeclaredClasses": true,
"allPublicClasses": true
},
{
"name": "com.google.api.PythonSettings$ExperimentalFeatures$Builder",
"queryAllDeclaredConstructors": true,
"queryAllPublicConstructors": true,
"queryAllDeclaredMethods": true,
"allPublicMethods": true,
"allDeclaredClasses": true,
"allPublicClasses": true
},
{
"name": "com.google.api.ResourceDescriptor",
"queryAllDeclaredConstructors": true,
Expand Down Expand Up @@ -1727,6 +1745,24 @@
"allDeclaredClasses": true,
"allPublicClasses": true
},
{
"name": "com.google.spanner.v1.MultiplexedSessionPrecommitToken",
"queryAllDeclaredConstructors": true,
"queryAllPublicConstructors": true,
"queryAllDeclaredMethods": true,
"allPublicMethods": true,
"allDeclaredClasses": true,
"allPublicClasses": true
},
{
"name": "com.google.spanner.v1.MultiplexedSessionPrecommitToken$Builder",
"queryAllDeclaredConstructors": true,
"queryAllPublicConstructors": true,
"queryAllDeclaredMethods": true,
"allPublicMethods": true,
"allDeclaredClasses": true,
"allPublicClasses": true
},
{
"name": "com.google.spanner.v1.Mutation",
"queryAllDeclaredConstructors": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -59,6 +61,16 @@ Spanner createSpanner() {
.getService();
}

@BeforeClass
public static void setWatchdogTimeout() {
System.setProperty("com.google.cloud.spanner.watchdogTimeoutSeconds", "1");
}

@AfterClass
public static void clearWatchdogTimeout() {
System.clearProperty("com.google.cloud.spanner.watchdogTimeoutSeconds");
}

@After
public void cleanup() {
mockSpanner.unfreeze();
Expand All @@ -75,7 +87,13 @@ public void testBatchClient_closedSpannerWithOpenResultSet_streamsAreCancelled()
client.batchReadOnlyTransaction(TimestampBound.strong());
ResultSet resultSet = transaction.executeQuery(SELECT_RANDOM_STATEMENT)) {
mockSpanner.freezeAfterReturningNumRows(1);
assertTrue(resultSet.next());
// This can sometimes fail, as the mock server may not always actually return the first row.
try {
assertTrue(resultSet.next());
} catch (SpannerException exception) {
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
return;
}
((SpannerImpl) spanner).close(1, TimeUnit.MILLISECONDS);
// This should return an error as the stream is cancelled.
SpannerException exception = assertThrows(SpannerException.class, resultSet::next);
Expand All @@ -93,7 +111,13 @@ public void testNormalDatabaseClient_closedSpannerWithOpenResultSet_sessionsAreD
try (ReadOnlyTransaction transaction = client.readOnlyTransaction(TimestampBound.strong());
ResultSet resultSet = transaction.executeQuery(SELECT_RANDOM_STATEMENT)) {
mockSpanner.freezeAfterReturningNumRows(1);
assertTrue(resultSet.next());
// This can sometimes fail, as the mock server may not always actually return the first row.
try {
assertTrue(resultSet.next());
} catch (SpannerException exception) {
assertEquals(ErrorCode.DEADLINE_EXCEEDED, exception.getErrorCode());
return;
}
List<ExecuteSqlRequest> executeSqlRequests =
mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).stream()
.filter(request -> request.getSql().equals(SELECT_RANDOM_STATEMENT.getSql()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.google.spanner.v1.ExecuteSqlRequest;
import com.google.spanner.v1.KeySet;
import com.google.spanner.v1.ListSessionsResponse;
import com.google.spanner.v1.MultiplexedSessionPrecommitToken;
import com.google.spanner.v1.Mutation;
import com.google.spanner.v1.Partition;
import com.google.spanner.v1.PartitionOptions;
Expand Down Expand Up @@ -587,6 +588,7 @@ public void executeSqlTest() throws Exception {
.setMetadata(ResultSetMetadata.newBuilder().build())
.addAllRows(new ArrayList<ListValue>())
.setStats(ResultSetStats.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockService.addResponse(expectedResponse);

Expand Down Expand Up @@ -673,6 +675,7 @@ public void executeBatchDmlTest() throws Exception {
ExecuteBatchDmlResponse.newBuilder()
.addAllResultSets(new ArrayList<ResultSet>())
.setStatus(Status.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockService.addResponse(expectedResponse);

Expand Down Expand Up @@ -735,6 +738,7 @@ public void readTest() throws Exception {
.setMetadata(ResultSetMetadata.newBuilder().build())
.addAllRows(new ArrayList<ListValue>())
.setStats(ResultSetStats.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockService.addResponse(expectedResponse);

Expand Down Expand Up @@ -821,6 +825,7 @@ public void beginTransactionTest() throws Exception {
Transaction.newBuilder()
.setId(ByteString.EMPTY)
.setReadTimestamp(Timestamp.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockService.addResponse(expectedResponse);

Expand Down Expand Up @@ -868,6 +873,7 @@ public void beginTransactionTest2() throws Exception {
Transaction.newBuilder()
.setId(ByteString.EMPTY)
.setReadTimestamp(Timestamp.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockService.addResponse(expectedResponse);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.google.spanner.v1.KeySet;
import com.google.spanner.v1.ListSessionsRequest;
import com.google.spanner.v1.ListSessionsResponse;
import com.google.spanner.v1.MultiplexedSessionPrecommitToken;
import com.google.spanner.v1.Mutation;
import com.google.spanner.v1.PartialResultSet;
import com.google.spanner.v1.Partition;
Expand Down Expand Up @@ -545,6 +546,7 @@ public void executeSqlTest() throws Exception {
.setMetadata(ResultSetMetadata.newBuilder().build())
.addAllRows(new ArrayList<ListValue>())
.setStats(ResultSetStats.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockSpanner.addResponse(expectedResponse);

Expand Down Expand Up @@ -629,6 +631,7 @@ public void executeStreamingSqlTest() throws Exception {
.setChunkedValue(true)
.setResumeToken(ByteString.EMPTY)
.setStats(ResultSetStats.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockSpanner.addResponse(expectedResponse);
ExecuteSqlRequest request =
Expand Down Expand Up @@ -702,6 +705,7 @@ public void executeBatchDmlTest() throws Exception {
ExecuteBatchDmlResponse.newBuilder()
.addAllResultSets(new ArrayList<ResultSet>())
.setStatus(Status.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockSpanner.addResponse(expectedResponse);

Expand Down Expand Up @@ -762,6 +766,7 @@ public void readTest() throws Exception {
.setMetadata(ResultSetMetadata.newBuilder().build())
.addAllRows(new ArrayList<ListValue>())
.setStats(ResultSetStats.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockSpanner.addResponse(expectedResponse);

Expand Down Expand Up @@ -847,6 +852,7 @@ public void streamingReadTest() throws Exception {
.setChunkedValue(true)
.setResumeToken(ByteString.EMPTY)
.setStats(ResultSetStats.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockSpanner.addResponse(expectedResponse);
ReadRequest request =
Expand Down Expand Up @@ -920,6 +926,7 @@ public void beginTransactionTest() throws Exception {
Transaction.newBuilder()
.setId(ByteString.EMPTY)
.setReadTimestamp(Timestamp.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockSpanner.addResponse(expectedResponse);

Expand Down Expand Up @@ -962,6 +969,7 @@ public void beginTransactionTest2() throws Exception {
Transaction.newBuilder()
.setId(ByteString.EMPTY)
.setReadTimestamp(Timestamp.newBuilder().build())
.setPrecommitToken(MultiplexedSessionPrecommitToken.newBuilder().build())
.build();
mockSpanner.addResponse(expectedResponse);

Expand Down
Loading

0 comments on commit fae1585

Please sign in to comment.