Skip to content

Commit

Permalink
remote: Do not retry bytestream read requests if the file is complete.
Browse files Browse the repository at this point in the history
Previously, an error at the end of the Read stream would result in a pointless retry request for 0 bytes of data.

Closes #13808.

PiperOrigin-RevId: 389785691
  • Loading branch information
benjaminp authored and copybara-github committed Aug 10, 2021
1 parent ead4495 commit 78b89a0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,14 @@ public void onNext(ReadResponse readResponse) {

@Override
public void onError(Throwable t) {
if (offset.get() == digest.getSizeBytes()) {
// If the file was fully downloaded, it doesn't matter if there was an error at
// the end of the stream.
logger.atInfo().withCause(t).log(
"ignoring error because file was fully received");
onCompleted();
return;
}
Status status = Status.fromThrowable(t);
if (status.getCode() == Status.Code.NOT_FOUND) {
future.setException(new CacheNotFoundException(digest));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,28 @@ public void read(ReadRequest request, StreamObserver<ReadResponse> responseObser
Mockito.verify(mockBackoff, Mockito.never()).nextDelayMillis(any(Exception.class));
}

@Test
public void downloadBlobDoesNotRetryZeroLengthRequests()
throws IOException, InterruptedException {
Backoff mockBackoff = Mockito.mock(Backoff.class);
final GrpcCacheClient client =
newClient(Options.getDefaults(RemoteOptions.class), () -> mockBackoff);
final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg");
serviceRegistry.addService(
new ByteStreamImplBase() {
@Override
public void read(ReadRequest request, StreamObserver<ReadResponse> responseObserver) {
assertThat(request.getResourceName()).contains(digest.getHash());
assertThat(request.getReadOffset()).isEqualTo(0);
ByteString data = ByteString.copyFromUtf8("abcdefg");
responseObserver.onNext(ReadResponse.newBuilder().setData(data).build());
responseObserver.onError(Status.INTERNAL.asException());
}
});
assertThat(new String(downloadBlob(context, client, digest), UTF_8)).isEqualTo("abcdefg");
Mockito.verify(mockBackoff, Mockito.never()).nextDelayMillis(any(Exception.class));
}

@Test
public void downloadBlobPassesThroughDeadlineExceededWithoutProgress() throws IOException {
Backoff mockBackoff = Mockito.mock(Backoff.class);
Expand Down

0 comments on commit 78b89a0

Please sign in to comment.