Skip to content

Commit e72365e

Browse files
authored
feat(gax): retry chunk upload on transient errors (#14422)
Wraps chunk uploads in a `RetryingCallable` to retry transient errors. Note: the retry settings used here are intentionally different from those that will be used for starting the upload; our requirement for this milestone is that "the retry policy [specified by the user] is only applicable to the initial request."
1 parent 5bd4520 commit e72365e

3 files changed

Lines changed: 147 additions & 1 deletion

File tree

‎sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/rpc/ResumableUploadCallableImpl.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,16 @@
3535
import com.google.api.core.ApiFutures;
3636
import com.google.api.core.BetaApi;
3737
import com.google.api.core.InternalApi;
38+
import com.google.api.gax.resumable.ChunkUploadRequest;
39+
import com.google.api.gax.resumable.ChunkUploadResponse;
3840
import com.google.api.gax.resumable.ResumableUploadClient;
3941
import com.google.api.gax.resumable.ResumableUploadSession;
42+
import com.google.api.gax.retrying.ExponentialRetryAlgorithm;
43+
import com.google.api.gax.retrying.RetryAlgorithm;
44+
import com.google.api.gax.retrying.RetrySettings;
45+
import com.google.api.gax.retrying.ScheduledRetryingExecutor;
4046
import java.io.InputStream;
47+
import java.time.Duration;
4148
import org.jspecify.annotations.NullMarked;
4249
import org.jspecify.annotations.Nullable;
4350

@@ -54,9 +61,23 @@
5461
public class ResumableUploadCallableImpl<RequestT, ResponseT>
5562
extends ResumableUploadCallable<RequestT, ResponseT> {
5663

64+
private static final RetrySettings RETRY_SETTINGS =
65+
RetrySettings.newBuilder()
66+
.setInitialRetryDelayDuration(Duration.ofSeconds(1))
67+
.setRetryDelayMultiplier(2.0)
68+
.setMaxRetryDelayDuration(Duration.ofSeconds(60))
69+
.setMaxAttempts(5)
70+
.setInitialRpcTimeoutDuration(Duration.ofMinutes(1))
71+
.setRpcTimeoutMultiplier(1.0)
72+
.setMaxRpcTimeoutDuration(Duration.ofMinutes(1))
73+
.setTotalTimeoutDuration(Duration.ofMinutes(2))
74+
.build();
75+
5776
private final ResumableUploadClient<RequestT, ResponseT> client;
5877
private final ResumableUploadCallSettings defaultCallSettings;
5978
private final ClientContext clientContext;
79+
private final UnaryCallable<ChunkUploadRequest, ChunkUploadResponse<ResponseT>>
80+
retryingUploadChunkCallable;
6081

6182
public ResumableUploadCallableImpl(
6283
ResumableUploadClient<RequestT, ResponseT> client,
@@ -66,6 +87,9 @@ public ResumableUploadCallableImpl(
6687
this.defaultCallSettings =
6788
checkNotNull(defaultCallSettings, "defaultCallSettings must not be null");
6889
this.clientContext = checkNotNull(clientContext, "clientContext must not be null");
90+
this.retryingUploadChunkCallable =
91+
createRetryingCallable(
92+
client.uploadChunkCallable(), ResumableUploadCommand.UPLOAD, clientContext);
6993
}
7094

7195
@Override
@@ -88,7 +112,7 @@ public ResumableUploadFuture<ResponseT> futureCall(
88112

89113
return ResumableUploadFutureImpl.create(
90114
startFuture,
91-
client.uploadChunkCallable(),
115+
retryingUploadChunkCallable,
92116
payload,
93117
effectiveSettings,
94118
clientContext.getDefaultCallContext());
@@ -99,4 +123,18 @@ public ResumableUploadFuture<ResponseT> resumeCall(
99123
String sessionUrl, InputStream payload, @Nullable ResumableUploadCallSettings settings) {
100124
throw new UnsupportedOperationException("Session resumption is not yet implemented.");
101125
}
126+
127+
private static <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> createRetryingCallable(
128+
UnaryCallable<RequestT, ResponseT> callable,
129+
ResumableUploadCommand command,
130+
ClientContext clientContext) {
131+
RetryAlgorithm<ResponseT> retryAlgorithm =
132+
new RetryAlgorithm<>(
133+
new ResumableUploadResultRetryAlgorithm<>(command),
134+
new ExponentialRetryAlgorithm(RETRY_SETTINGS, clientContext.getClock()));
135+
return new RetryingCallable<>(
136+
clientContext.getDefaultCallContext(),
137+
checkNotNull(callable, "callable must not be null"),
138+
new ScheduledRetryingExecutor<>(retryAlgorithm, clientContext.getExecutor()));
139+
}
102140
}

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/rpc/CallableTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@ void testWatched_usesJavaTimeMethods() {
214214
void testResumableUploadCallable() {
215215
ResumableUploadClient<String, String> uploadClient =
216216
mock(ResumableUploadClient.class, Mockito.withSettings().withoutAnnotations());
217+
when(uploadClient.uploadChunkCallable())
218+
.thenReturn(mock(UnaryCallable.class, Mockito.withSettings().withoutAnnotations()));
217219
ResumableUploadCallSettings settings =
218220
ResumableUploadCallSettings.newBuilder().setChunkSize(1024).build();
219221

‎sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/rpc/ResumableUploadCallableImplTest.java‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,102 @@ void testResumeCall_throwsUnsupportedOperationException() {
457457
() -> callable.resumeCall("https://upload.url/session", streamOf("data"), null));
458458
}
459459

460+
@Test
461+
void testChunkRetry_transientFailureThenSuccess_retriesAndSucceeds() throws Exception {
462+
stubStartSession("https://upload.url/chunk-retry-ok");
463+
TrackableStream stream = new TrackableStream("01234567"); // exactly 1 chunk of 8 bytes
464+
when(mockChunkCallable.futureCall(any(ChunkUploadRequest.class), any()))
465+
.thenReturn(
466+
ApiFutures.immediateFailedFuture(createApiException(503, StatusCode.Code.UNAVAILABLE)))
467+
.thenReturn(
468+
ApiFutures.immediateFuture(
469+
ChunkUploadResponse.create(ResumableUploadStatus.FINAL, "chunk-done")));
470+
471+
ResumableUploadFuture<String> future = callable.futureCall("resource-path", stream, null);
472+
473+
assertThat(future.get()).isEqualTo("chunk-done");
474+
assertThat(future.isDone()).isTrue();
475+
assertThat(stream.totalBytesRead).isEqualTo(8);
476+
assertThat(stream.closed).isTrue();
477+
478+
ArgumentCaptor<ChunkUploadRequest> captor = ArgumentCaptor.forClass(ChunkUploadRequest.class);
479+
verify(mockChunkCallable, times(2)).futureCall(captor.capture(), any());
480+
List<ChunkUploadRequest> requests = captor.getAllValues();
481+
assertThat(requests.get(0).getOffset()).isEqualTo(0);
482+
assertThat(requests.get(0).getPayload()).isEqualTo("01234567".getBytes(StandardCharsets.UTF_8));
483+
assertThat(requests.get(1).getOffset()).isEqualTo(0);
484+
assertThat(requests.get(1).getPayload()).isEqualTo("01234567".getBytes(StandardCharsets.UTF_8));
485+
}
486+
487+
@Test
488+
void testChunkRetry_transientFailureExhaustion_surfacesLastError() {
489+
stubStartSession("https://upload.url/chunk-exhaustion");
490+
when(mockChunkCallable.futureCall(any(ChunkUploadRequest.class), any()))
491+
.thenReturn(
492+
ApiFutures.immediateFailedFuture(createApiException(503, StatusCode.Code.UNAVAILABLE)));
493+
494+
ResumableUploadFuture<String> future =
495+
callable.futureCall("resource-path", streamOf("hello"), null);
496+
497+
ExecutionException exception = assertThrows(ExecutionException.class, future::get);
498+
assertThat(exception.getCause()).isInstanceOf(ApiException.class);
499+
assertThat(((ApiException) exception.getCause()).getStatusCode().getTransportCode())
500+
.isEqualTo(503);
501+
502+
// Default chunk retry settings has maxAttempts = 5
503+
verify(mockChunkCallable, times(5)).futureCall(any(), any());
504+
}
505+
506+
@Test
507+
void testChunkRetry_cancellationDuringBackoff_deschedulesPendingAttempt() {
508+
stubStartSession("https://upload.url/cancel-backoff");
509+
SettableApiFuture<ChunkUploadResponse<String>> chunkAttempt0Future = SettableApiFuture.create();
510+
when(mockChunkCallable.futureCall(any(ChunkUploadRequest.class), any()))
511+
.thenReturn(chunkAttempt0Future)
512+
.thenReturn(
513+
ApiFutures.immediateFuture(
514+
ChunkUploadResponse.create(ResumableUploadStatus.FINAL, "should-not-reach")));
515+
516+
ResumableUploadFuture<String> sessionFuture =
517+
callable.futureCall("resource-path", streamOf("hello"), null);
518+
519+
// Fail attempt 0 with 503 to schedule backoff
520+
chunkAttempt0Future.setException(createApiException(503, StatusCode.Code.UNAVAILABLE));
521+
522+
// Cancel while backoff is pending
523+
assertThat(sessionFuture.cancel(true)).isTrue();
524+
assertThat(sessionFuture.isCancelled()).isTrue();
525+
assertThrows(CancellationException.class, sessionFuture::get);
526+
527+
// Only attempt 0 occurred; attempt 1 was de-scheduled
528+
verify(mockChunkCallable, times(1)).futureCall(any(), any());
529+
}
530+
531+
private static class HttpStatusStatusCode implements StatusCode {
532+
private final int httpStatus;
533+
private final StatusCode.Code code;
534+
535+
HttpStatusStatusCode(int httpStatus, StatusCode.Code code) {
536+
this.httpStatus = httpStatus;
537+
this.code = code;
538+
}
539+
540+
@Override
541+
public StatusCode.Code getCode() {
542+
return code;
543+
}
544+
545+
@Override
546+
public Integer getTransportCode() {
547+
return httpStatus;
548+
}
549+
}
550+
551+
private static ApiException createApiException(int httpStatus, StatusCode.Code code) {
552+
return ApiExceptionFactory.createException(
553+
"HTTP " + httpStatus, null, new HttpStatusStatusCode(httpStatus, code), false);
554+
}
555+
460556
private void stubStartSession(String uploadUrl) {
461557
when(mockStartCallable.futureCall(any(), any()))
462558
.thenReturn(
@@ -477,11 +573,21 @@ private static void assertChunk(
477573

478574
private static class TrackableStream extends ByteArrayInputStream {
479575
boolean closed = false;
576+
int totalBytesRead = 0;
480577

481578
TrackableStream(String content) {
482579
super(content.getBytes(StandardCharsets.UTF_8));
483580
}
484581

582+
@Override
583+
public int read(byte[] b, int off, int len) {
584+
int read = super.read(b, off, len);
585+
if (read > 0) {
586+
totalBytesRead += read;
587+
}
588+
return read;
589+
}
590+
485591
@Override
486592
public void close() throws IOException {
487593
closed = true;

0 commit comments

Comments
 (0)