Skip to content

Commit a3144a0

Browse files
mairbekvkedia
mairbek
authored and
vkedia
committed
Treat RESOURCE_EXHAUSTED with retry information as aborted. (#2645)
* Treat RESOURCE_EXHAUSTED with retry information as aborted. * Updated according to comments * Updated according to comments
1 parent e4e6a0c commit a3144a0

6 files changed

Lines changed: 81 additions & 30 deletions

File tree

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbortedException.java‎

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

1717
package com.google.cloud.spanner;
1818

19-
import com.google.protobuf.util.Durations;
20-
import com.google.rpc.RetryInfo;
21-
import io.grpc.Metadata;
22-
import io.grpc.Status;
23-
import io.grpc.protobuf.ProtoUtils;
2419
import javax.annotation.Nullable;
2520

2621
/**
@@ -29,8 +24,6 @@
2924
* other types of errors, most typically by retrying the transaction.
3025
*/
3126
public class AbortedException extends SpannerException {
32-
private static final Metadata.Key<RetryInfo> KEY_RETRY_INFO =
33-
ProtoUtils.keyForProto(RetryInfo.getDefaultInstance());
3427

3528
/**
3629
* Abort is not retryable per se: the operation request needs to change (generally to reflect a
@@ -43,21 +36,4 @@ public class AbortedException extends SpannerException {
4336
DoNotConstructDirectly token, @Nullable String message, @Nullable Throwable cause) {
4437
super(token, ErrorCode.ABORTED, IS_RETRYABLE, message, cause);
4538
}
46-
47-
/**
48-
* Return the retry delay for transaction in milliseconds. Return -1 if this does not specify any
49-
* retry delay. In that case, clients should fall back to a locally computed retry delay.
50-
*/
51-
public long getRetryDelayInMillis() {
52-
if (this.getCause() != null) {
53-
Metadata trailers = Status.trailersFromThrowable(this.getCause());
54-
if (trailers != null && trailers.containsKey(KEY_RETRY_INFO)) {
55-
RetryInfo retryInfo = trailers.get(KEY_RETRY_INFO);
56-
if (retryInfo.hasRetryDelay()) {
57-
return Durations.toMillis(retryInfo.getRetryDelay());
58-
}
59-
}
60-
}
61-
return -1L;
62-
}
6339
}

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerException.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,18 @@
1818

1919
import com.google.cloud.grpc.BaseGrpcServiceException;
2020
import com.google.common.base.Preconditions;
21+
import com.google.protobuf.util.Durations;
22+
import com.google.rpc.RetryInfo;
23+
import io.grpc.Metadata;
24+
import io.grpc.Status;
25+
import io.grpc.protobuf.ProtoUtils;
2126
import javax.annotation.Nullable;
2227

2328
/** Base exception type for all exceptions produced by the Cloud Spanner service. */
2429
public class SpannerException extends BaseGrpcServiceException {
2530
private static final long serialVersionUID = 20150916L;
31+
private static final Metadata.Key<RetryInfo> KEY_RETRY_INFO =
32+
ProtoUtils.keyForProto(RetryInfo.getDefaultInstance());
2633

2734
private final ErrorCode code;
2835

@@ -48,4 +55,26 @@ public ErrorCode getErrorCode() {
4855
enum DoNotConstructDirectly {
4956
ALLOWED
5057
}
58+
59+
/**
60+
* Return the retry delay for operation in milliseconds. Return -1 if this does not specify any
61+
* retry delay.
62+
*/
63+
public long getRetryDelayInMillis() {
64+
return extractRetryDelay(this.getCause());
65+
}
66+
67+
static long extractRetryDelay(Throwable cause) {
68+
if (cause != null) {
69+
Metadata trailers = Status.trailersFromThrowable(cause);
70+
if (trailers != null && trailers.containsKey(KEY_RETRY_INFO)) {
71+
RetryInfo retryInfo = trailers.get(KEY_RETRY_INFO);
72+
if (retryInfo.hasRetryDelay()) {
73+
return Durations.toMillis(retryInfo.getRetryDelay());
74+
}
75+
}
76+
}
77+
return -1L;
78+
}
79+
5180
}

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ private static boolean isRetryable(ErrorCode code, @Nullable Throwable cause) {
126126
return hasCauseMatching(cause, Matchers.isRetryableInternalError);
127127
case UNAVAILABLE:
128128
return true;
129+
case RESOURCE_EXHAUSTED:
130+
return SpannerException.extractRetryDelay(cause) > 0;
129131
default:
130132
return false;
131133
}

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,12 @@ static <T> T runWithRetries(Callable<T> callable) {
231231
throw e;
232232
}
233233
logger.log(Level.FINE, "Retryable exception, will sleep and retry", e);
234-
backoffSleep(context, backOff);
234+
long delay = e.getRetryDelayInMillis();
235+
if (delay != -1) {
236+
backoffSleep(context, delay);
237+
} else {
238+
backoffSleep(context, backOff);
239+
}
235240
} catch (Exception e) {
236241
Throwables.throwIfUnchecked(e);
237242
throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e);
@@ -2396,7 +2401,12 @@ protected PartialResultSet computeNext() {
23962401
assert buffer.isEmpty() || buffer.getLast().getResumeToken().equals(resumeToken);
23972402
stream = null;
23982403
try (Scope s = tracer.withSpan(span)) {
2399-
backoffSleep(context, backOff);
2404+
long delay = e.getRetryDelayInMillis();
2405+
if (delay != -1) {
2406+
backoffSleep(context, delay);
2407+
} else {
2408+
backoffSleep(context, backOff);
2409+
}
24002410
}
24012411
continue;
24022412
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,34 @@ public void connectionClosedIsRetryable() {
5151
assertThat(e.isRetryable()).isTrue();
5252
}
5353

54+
@Test
55+
public void resourceExhausted() {
56+
Status status =
57+
Status.fromCodeValue(Status.Code.RESOURCE_EXHAUSTED.value())
58+
.withDescription("Memory pushback");
59+
SpannerException e =
60+
SpannerExceptionFactory.newSpannerException(new StatusRuntimeException(status));
61+
assertThat(e.isRetryable()).isFalse();
62+
}
63+
64+
@Test
65+
public void resourceExhaustedWithBackoff() {
66+
Status status =
67+
Status.fromCodeValue(Status.Code.RESOURCE_EXHAUSTED.value())
68+
.withDescription("Memory pushback");
69+
Metadata trailers = new Metadata();
70+
Metadata.Key<RetryInfo> key = ProtoUtils.keyForProto(RetryInfo.getDefaultInstance());
71+
RetryInfo retryInfo =
72+
RetryInfo.newBuilder()
73+
.setRetryDelay(Duration.newBuilder().setNanos(1000000).setSeconds(1L))
74+
.build();
75+
trailers.put(key, retryInfo);
76+
SpannerException e =
77+
SpannerExceptionFactory.newSpannerException(new StatusRuntimeException(status, trailers));
78+
assertThat(e.isRetryable()).isTrue();
79+
assertThat(e.getRetryDelayInMillis()).isEqualTo(1001);
80+
}
81+
5482
@Test
5583
public void abortWithRetryInfo() {
5684
Metadata.Key<RetryInfo> key = ProtoUtils.keyForProto(RetryInfo.getDefaultInstance());

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void setUp() throws Exception {
6464

6565
@Test
6666
public void runAbort() {
67-
runTransaction(createRetryException());
67+
runTransaction(createRetryException(Status.Code.ABORTED));
6868
ArgumentCaptor<Long> backoffMillis = ArgumentCaptor.forClass(Long.class);
6969
verify(sleeper, times(1)).backoffSleep(Mockito.<Context>any(), backoffMillis.capture());
7070
assertThat(backoffMillis.getValue()).isEqualTo(1001L);
@@ -81,7 +81,7 @@ public void runAbortNoRetryInfo() {
8181
@Test
8282
public void commitAbort() {
8383
final SpannerException error =
84-
SpannerExceptionFactory.newSpannerException(createRetryException());
84+
SpannerExceptionFactory.newSpannerException(createRetryException(Status.Code.ABORTED));
8585
when(rpc.commit(Mockito.<CommitRequest>any(), Mockito.<Map<Option, ?>>any()))
8686
.thenThrow(error)
8787
.thenReturn(
@@ -106,9 +106,15 @@ public Void run(TransactionContext transaction) throws Exception {
106106
assertThat(backoffMillis.getValue()).isEqualTo(1001L);
107107
}
108108

109-
private StatusRuntimeException createRetryException() {
109+
@Test(expected = SpannerException.class)
110+
public void runResourceExhaustedNoRetry() throws Exception {
111+
runTransaction(
112+
new StatusRuntimeException(Status.fromCodeValue(Status.Code.RESOURCE_EXHAUSTED.value())));
113+
}
114+
115+
private StatusRuntimeException createRetryException(Status.Code code) {
110116
Metadata.Key<RetryInfo> key = ProtoUtils.keyForProto(RetryInfo.getDefaultInstance());
111-
Status status = Status.fromCodeValue(Status.Code.ABORTED.value());
117+
Status status = Status.fromCodeValue(code.value());
112118
Metadata trailers = new Metadata();
113119
RetryInfo retryInfo =
114120
RetryInfo.newBuilder()

0 commit comments

Comments
 (0)