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 #13808. PiperOrigin-RevId: 389785691
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java index 29ee7ac..a1d74b6 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java
@@ -366,6 +366,14 @@ @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));
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index 4999dc7..3faa645 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java
@@ -963,6 +963,28 @@ } @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); Mockito.when(mockBackoff.nextDelayMillis(any(Exception.class))).thenReturn(-1L);