Skip to content

Commit 78b89a0

Browse files
benjaminpcopybara-github
authored andcommitted
remote: Do not retry bytestream read requests if the file is complete.
Previously, an error at the end of the Read stream would result in a pointless retry request for 0 bytes of data. Closes bazelbuild#13808. PiperOrigin-RevId: 389785691
1 parent ead4495 commit 78b89a0

File tree

2 files changed

+30
-0
lines changed

2 files changed

+30
-0
lines changed

src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java

+8
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,14 @@ public void onNext(ReadResponse readResponse) {
366366

367367
@Override
368368
public void onError(Throwable t) {
369+
if (offset.get() == digest.getSizeBytes()) {
370+
// If the file was fully downloaded, it doesn't matter if there was an error at
371+
// the end of the stream.
372+
logger.atInfo().withCause(t).log(
373+
"ignoring error because file was fully received");
374+
onCompleted();
375+
return;
376+
}
369377
Status status = Status.fromThrowable(t);
370378
if (status.getCode() == Status.Code.NOT_FOUND) {
371379
future.setException(new CacheNotFoundException(digest));

src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java

+22
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,28 @@ public void read(ReadRequest request, StreamObserver<ReadResponse> responseObser
962962
Mockito.verify(mockBackoff, Mockito.never()).nextDelayMillis(any(Exception.class));
963963
}
964964

965+
@Test
966+
public void downloadBlobDoesNotRetryZeroLengthRequests()
967+
throws IOException, InterruptedException {
968+
Backoff mockBackoff = Mockito.mock(Backoff.class);
969+
final GrpcCacheClient client =
970+
newClient(Options.getDefaults(RemoteOptions.class), () -> mockBackoff);
971+
final Digest digest = DIGEST_UTIL.computeAsUtf8("abcdefg");
972+
serviceRegistry.addService(
973+
new ByteStreamImplBase() {
974+
@Override
975+
public void read(ReadRequest request, StreamObserver<ReadResponse> responseObserver) {
976+
assertThat(request.getResourceName()).contains(digest.getHash());
977+
assertThat(request.getReadOffset()).isEqualTo(0);
978+
ByteString data = ByteString.copyFromUtf8("abcdefg");
979+
responseObserver.onNext(ReadResponse.newBuilder().setData(data).build());
980+
responseObserver.onError(Status.INTERNAL.asException());
981+
}
982+
});
983+
assertThat(new String(downloadBlob(context, client, digest), UTF_8)).isEqualTo("abcdefg");
984+
Mockito.verify(mockBackoff, Mockito.never()).nextDelayMillis(any(Exception.class));
985+
}
986+
965987
@Test
966988
public void downloadBlobPassesThroughDeadlineExceededWithoutProgress() throws IOException {
967989
Backoff mockBackoff = Mockito.mock(Backoff.class);

0 commit comments

Comments
 (0)