Skip to content

Commit 4506f6f

Browse files
committed
fix: retry specific internal errors
Some specific internal errors should be retrid. Instead of adding INTERNAL as a standard retryable error code, we use an interceptor to catch and translate those specific errors. See also b/375684610
1 parent 7cc4e86 commit 4506f6f

File tree

3 files changed

+101
-5
lines changed

3 files changed

+101
-5
lines changed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/IsRetryableInternalError.java

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
import com.google.common.base.Predicate;
2121
import com.google.common.collect.ImmutableList;
2222
import io.grpc.Status;
23+
import io.grpc.Status.Code;
2324
import io.grpc.StatusRuntimeException;
2425

2526
public class IsRetryableInternalError implements Predicate<Throwable> {
27+
public static final IsRetryableInternalError INSTANCE = new IsRetryableInternalError();
2628

2729
private static final ImmutableList<String> RETRYABLE_ERROR_MESSAGES =
2830
ImmutableList.of(
@@ -32,13 +34,19 @@ public class IsRetryableInternalError implements Predicate<Throwable> {
3234
"stream terminated by RST_STREAM",
3335
"Authentication backend internal server error. Please retry.");
3436

37+
public boolean isRetryableInternalError(Status status) {
38+
return status.getCode() == Code.INTERNAL
39+
&& status.getDescription() != null
40+
&& isRetryableErrorMessage(status.getDescription());
41+
}
42+
3543
@Override
3644
public boolean apply(Throwable cause) {
37-
if (isInternalError(cause)) {
38-
return RETRYABLE_ERROR_MESSAGES.stream()
39-
.anyMatch(message -> cause.getMessage().contains(message));
40-
}
41-
return false;
45+
return isInternalError(cause) && isRetryableErrorMessage(cause.getMessage());
46+
}
47+
48+
private boolean isRetryableErrorMessage(String errorMessage) {
49+
return RETRYABLE_ERROR_MESSAGES.stream().anyMatch(errorMessage::contains);
4250
}
4351

4452
private boolean isInternalError(Throwable cause) {

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerErrorInterceptor.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.cloud.spanner.spi.v1;
1818

19+
import com.google.cloud.spanner.IsRetryableInternalError;
1920
import com.google.rpc.BadRequest;
2021
import com.google.rpc.Help;
2122
import com.google.rpc.LocalizedMessage;
@@ -32,6 +33,7 @@
3233
import io.grpc.Metadata;
3334
import io.grpc.MethodDescriptor;
3435
import io.grpc.Status;
36+
import io.grpc.Status.Code;
3537
import io.grpc.protobuf.ProtoUtils;
3638
import java.util.logging.Level;
3739
import java.util.logging.Logger;
@@ -69,6 +71,12 @@ public void start(Listener<RespT> responseListener, Metadata headers) {
6971
@Override
7072
public void onClose(Status status, Metadata trailers) {
7173
try {
74+
// Translate INTERNAL errors that should be retried to a retryable error code.
75+
if (status.getCode() == Code.INTERNAL
76+
&& IsRetryableInternalError.INSTANCE.isRetryableInternalError(status)) {
77+
status =
78+
Status.fromCode(Code.UNAVAILABLE).withDescription(status.getDescription());
79+
}
7280
if (trailers.containsKey(LOCALIZED_MESSAGE_KEY)) {
7381
status =
7482
Status.fromCodeValue(status.getCode().value())
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/*
2+
* Copyright 2024 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.cloud.spanner;
18+
19+
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertFalse;
21+
import static org.junit.Assert.assertTrue;
22+
23+
import com.google.cloud.NoCredentials;
24+
import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime;
25+
import com.google.cloud.spanner.connection.AbstractMockServerTest;
26+
import com.google.spanner.v1.BatchCreateSessionsRequest;
27+
import com.google.spanner.v1.ExecuteSqlRequest;
28+
import io.grpc.ManagedChannelBuilder;
29+
import io.grpc.Status;
30+
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
import org.junit.runners.JUnit4;
33+
34+
@RunWith(JUnit4.class)
35+
public class RetryableInternalErrorTest extends AbstractMockServerTest {
36+
@Test
37+
public void testTranslateInternalException() {
38+
try (Spanner spanner =
39+
SpannerOptions.newBuilder()
40+
.setProjectId("my-project")
41+
.setHost(String.format("http://localhost:%d", getPort()))
42+
.setChannelConfigurator(ManagedChannelBuilder::usePlaintext)
43+
.setCredentials(NoCredentials.getInstance())
44+
.setSessionPoolOption(
45+
SessionPoolOptions.newBuilder().setMinSessions(1).setMaxSessions(1).build())
46+
.build()
47+
.getService()) {
48+
DatabaseClient client = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d"));
49+
mockSpanner.setBatchCreateSessionsExecutionTime(
50+
SimulatedExecutionTime.ofException(
51+
Status.INTERNAL
52+
.withDescription("Authentication backend internal server error. Please retry.")
53+
.asRuntimeException()));
54+
mockSpanner.setExecuteStreamingSqlExecutionTime(
55+
SimulatedExecutionTime.ofException(
56+
Status.INTERNAL
57+
.withDescription("Authentication backend internal server error. Please retry.")
58+
.asRuntimeException()));
59+
mockSpanner.setExecuteSqlExecutionTime(
60+
SimulatedExecutionTime.ofException(
61+
Status.INTERNAL
62+
.withDescription("Authentication backend internal server error. Please retry.")
63+
.asRuntimeException()));
64+
65+
try (ResultSet resultSet = client.singleUse().executeQuery(SELECT1_STATEMENT)) {
66+
assertTrue(resultSet.next());
67+
assertFalse(resultSet.next());
68+
}
69+
assertEquals(
70+
Long.valueOf(1L),
71+
client
72+
.readWriteTransaction()
73+
.run(transaction -> transaction.executeUpdate(INSERT_STATEMENT)));
74+
75+
// Verify that all the RPCs were retried.
76+
assertEquals(2, mockSpanner.countRequestsOfType(BatchCreateSessionsRequest.class));
77+
assertEquals(4, mockSpanner.countRequestsOfType(ExecuteSqlRequest.class));
78+
}
79+
}
80+
}

0 commit comments

Comments
 (0)