Skip to content

Commit 08e807a

Browse files
committed
feat(gax): retry chunk upload on transient errors
Wraps chunk uploads in a retrying executor to retry transient network and server errors using exponential backoff. Retries individual chunks without restarting the entire upload session.
1 parent 4107698 commit 08e807a

4 files changed

Lines changed: 152 additions & 4 deletions

File tree

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

Lines changed: 35 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,19 @@
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.ofMillis(100))
67+
.setRetryDelayMultiplier(1.3)
68+
.setMaxRetryDelayDuration(Duration.ofMinutes(1))
69+
.setMaxAttempts(5)
70+
.build();
71+
5772
private final ResumableUploadClient<RequestT, ResponseT> client;
5873
private final ResumableUploadCallSettings defaultCallSettings;
5974
private final ClientContext clientContext;
75+
private final UnaryCallable<ChunkUploadRequest, ChunkUploadResponse<ResponseT>>
76+
retryingUploadChunkCallable;
6077

6178
public ResumableUploadCallableImpl(
6279
ResumableUploadClient<RequestT, ResponseT> client,
@@ -66,6 +83,9 @@ public ResumableUploadCallableImpl(
6683
this.defaultCallSettings =
6784
checkNotNull(defaultCallSettings, "defaultCallSettings must not be null");
6885
this.clientContext = checkNotNull(clientContext, "clientContext must not be null");
86+
this.retryingUploadChunkCallable =
87+
createRetryingCallable(
88+
client.uploadChunkCallable(), ResumableUploadCommand.UPLOAD, clientContext);
6989
}
7090

7191
@Override
@@ -88,7 +108,7 @@ public ResumableUploadFuture<ResponseT> futureCall(
88108

89109
return ResumableUploadFutureImpl.create(
90110
startFuture,
91-
client.uploadChunkCallable(),
111+
retryingUploadChunkCallable,
92112
payload,
93113
effectiveSettings,
94114
clientContext.getDefaultCallContext());
@@ -99,4 +119,18 @@ public ResumableUploadFuture<ResponseT> resumeCall(
99119
String sessionUrl, InputStream payload, @Nullable ResumableUploadCallSettings settings) {
100120
throw new UnsupportedOperationException("Session resumption is not yet implemented.");
101121
}
122+
123+
private static <ReqT, RespT> UnaryCallable<ReqT, RespT> createRetryingCallable(
124+
UnaryCallable<ReqT, RespT> callable,
125+
ResumableUploadCommand command,
126+
ClientContext clientContext) {
127+
RetryAlgorithm<RespT> retryAlgorithm =
128+
new RetryAlgorithm<>(
129+
new ResumableUploadResultRetryAlgorithm<>(command),
130+
new ExponentialRetryAlgorithm(RETRY_SETTINGS, clientContext.getClock()));
131+
return new RetryingCallable<>(
132+
clientContext.getDefaultCallContext(),
133+
checkNotNull(callable, "callable must not be null"),
134+
new ScheduledRetryingExecutor<>(retryAlgorithm, clientContext.getExecutor()));
135+
}
102136
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import java.io.InputStream;
4646
import java.util.Arrays;
4747
import java.util.concurrent.CancellationException;
48+
import java.util.concurrent.Executor;
4849
import org.jspecify.annotations.NullMarked;
4950
import org.jspecify.annotations.Nullable;
5051

@@ -59,6 +60,10 @@ final class ResumableUploadChunkCoordinator<ResponseT> {
5960

6061
private static final byte[] EMPTY_PAYLOAD = new byte[0];
6162

63+
// Serializes buffer mutations across multiple threads (i.e. from retry/recovery)
64+
private final Executor chunkExecutor =
65+
MoreExecutors.newSequentialExecutor(MoreExecutors.directExecutor());
66+
6267
private final UnaryCallable<ChunkUploadRequest, ChunkUploadResponse<ResponseT>>
6368
uploadChunkCallable;
6469
private final String uploadUrl;
@@ -93,7 +98,7 @@ ApiFuture<ResponseT> start() {
9398
}
9499
},
95100
MoreExecutors.directExecutor());
96-
transmitChunk(0L);
101+
chunkExecutor.execute(() -> transmitChunk(0L));
97102
return result;
98103
}
99104

@@ -157,9 +162,10 @@ public void onSuccess(ChunkUploadResponse<ResponseT> response) {
157162
result.setException(
158163
new IllegalStateException(
159164
"Upload stream ended and final chunk was transmitted, but server returned"
160-
+ " incomplete status"));
165+
+ " incomplete status for upload URL: "
166+
+ uploadUrl));
161167
} else {
162-
transmitChunk(nextOffset);
168+
chunkExecutor.execute(() -> transmitChunk(nextOffset));
163169
}
164170
}
165171

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
@@ -404,6 +404,102 @@ void testResumeCall_throwsUnsupportedOperationException() {
404404
() -> callable.resumeCall("https://upload.url/session", streamOf("data"), null));
405405
}
406406

407+
@Test
408+
void testChunkRetry_transientFailureThenSuccess_retriesAndSucceeds() throws Exception {
409+
stubStartSession("https://upload.url/chunk-retry-ok");
410+
TrackableStream stream = new TrackableStream("01234567"); // exactly 1 chunk of 8 bytes
411+
when(mockChunkCallable.futureCall(any(ChunkUploadRequest.class), any()))
412+
.thenReturn(
413+
ApiFutures.immediateFailedFuture(createApiException(503, StatusCode.Code.UNAVAILABLE)))
414+
.thenReturn(
415+
ApiFutures.immediateFuture(
416+
ChunkUploadResponse.create(ResumableUploadStatus.FINAL, "chunk-done")));
417+
418+
ResumableUploadFuture<String> future = callable.futureCall("resource-path", stream, null);
419+
420+
assertThat(future.get()).isEqualTo("chunk-done");
421+
assertThat(future.isDone()).isTrue();
422+
assertThat(stream.totalBytesRead).isEqualTo(8);
423+
assertThat(stream.closed).isTrue();
424+
425+
ArgumentCaptor<ChunkUploadRequest> captor = ArgumentCaptor.forClass(ChunkUploadRequest.class);
426+
verify(mockChunkCallable, times(2)).futureCall(captor.capture(), any());
427+
List<ChunkUploadRequest> requests = captor.getAllValues();
428+
assertThat(requests.get(0).getOffset()).isEqualTo(0);
429+
assertThat(requests.get(0).getPayload()).isEqualTo("01234567".getBytes(StandardCharsets.UTF_8));
430+
assertThat(requests.get(1).getOffset()).isEqualTo(0);
431+
assertThat(requests.get(1).getPayload()).isEqualTo("01234567".getBytes(StandardCharsets.UTF_8));
432+
}
433+
434+
@Test
435+
void testChunkRetry_transientFailureExhaustion_surfacesLastError() {
436+
stubStartSession("https://upload.url/chunk-exhaustion");
437+
when(mockChunkCallable.futureCall(any(ChunkUploadRequest.class), any()))
438+
.thenReturn(
439+
ApiFutures.immediateFailedFuture(createApiException(503, StatusCode.Code.UNAVAILABLE)));
440+
441+
ResumableUploadFuture<String> future =
442+
callable.futureCall("resource-path", streamOf("hello"), null);
443+
444+
ExecutionException exception = assertThrows(ExecutionException.class, future::get);
445+
assertThat(exception.getCause()).isInstanceOf(ApiException.class);
446+
assertThat(((ApiException) exception.getCause()).getStatusCode().getTransportCode())
447+
.isEqualTo(503);
448+
449+
// Default chunk retry settings has maxAttempts = 5
450+
verify(mockChunkCallable, times(5)).futureCall(any(), any());
451+
}
452+
453+
@Test
454+
void testChunkRetry_cancellationDuringBackoff_deschedulesPendingAttempt() {
455+
stubStartSession("https://upload.url/cancel-backoff");
456+
SettableApiFuture<ChunkUploadResponse<String>> chunkAttempt0Future = SettableApiFuture.create();
457+
when(mockChunkCallable.futureCall(any(ChunkUploadRequest.class), any()))
458+
.thenReturn(chunkAttempt0Future)
459+
.thenReturn(
460+
ApiFutures.immediateFuture(
461+
ChunkUploadResponse.create(ResumableUploadStatus.FINAL, "should-not-reach")));
462+
463+
ResumableUploadFuture<String> sessionFuture =
464+
callable.futureCall("resource-path", streamOf("hello"), null);
465+
466+
// Fail attempt 0 with 503 to schedule backoff
467+
chunkAttempt0Future.setException(createApiException(503, StatusCode.Code.UNAVAILABLE));
468+
469+
// Cancel while backoff is pending
470+
assertThat(sessionFuture.cancel(true)).isTrue();
471+
assertThat(sessionFuture.isCancelled()).isTrue();
472+
assertThrows(CancellationException.class, sessionFuture::get);
473+
474+
// Only attempt 0 occurred; attempt 1 was de-scheduled
475+
verify(mockChunkCallable, times(1)).futureCall(any(), any());
476+
}
477+
478+
private static class HttpStatusStatusCode implements StatusCode {
479+
private final int httpStatus;
480+
private final StatusCode.Code code;
481+
482+
HttpStatusStatusCode(int httpStatus, StatusCode.Code code) {
483+
this.httpStatus = httpStatus;
484+
this.code = code;
485+
}
486+
487+
@Override
488+
public StatusCode.Code getCode() {
489+
return code;
490+
}
491+
492+
@Override
493+
public Integer getTransportCode() {
494+
return httpStatus;
495+
}
496+
}
497+
498+
private static ApiException createApiException(int httpStatus, StatusCode.Code code) {
499+
return ApiExceptionFactory.createException(
500+
"HTTP " + httpStatus, null, new HttpStatusStatusCode(httpStatus, code), false);
501+
}
502+
407503
private void stubStartSession(String uploadUrl) {
408504
when(mockStartCallable.futureCall(any(), any()))
409505
.thenReturn(
@@ -424,11 +520,21 @@ private static void assertChunk(
424520

425521
private static class TrackableStream extends ByteArrayInputStream {
426522
boolean closed = false;
523+
int totalBytesRead = 0;
427524

428525
TrackableStream(String content) {
429526
super(content.getBytes(StandardCharsets.UTF_8));
430527
}
431528

529+
@Override
530+
public int read(byte[] b, int off, int len) {
531+
int read = super.read(b, off, len);
532+
if (read > 0) {
533+
totalBytesRead += read;
534+
}
535+
return read;
536+
}
537+
432538
@Override
433539
public void close() throws IOException {
434540
closed = true;

0 commit comments

Comments
 (0)