From e675cd70cb2dcab041bfc52e63bc273d7456255e Mon Sep 17 00:00:00 2001 From: Zack Winter Date: Fri, 10 Apr 2026 00:01:48 +0000 Subject: [PATCH 1/2] DEVPROD-31493 Add Bazel Flag to Reverse Remote Asset API Download Fallback Order --- .../downloader/GrpcRemoteDownloader.java | 38 +++- .../downloader/GrpcRemoteDownloaderTest.java | 170 +++++++++++++++++- 2 files changed, 204 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java b/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java index d94030930fb9b6..e3adb6f4a620ae 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java @@ -82,6 +82,12 @@ public class GrpcRemoteDownloader implements AutoCloseable, Downloader { private static final String QUALIFIER_CHECKSUM_SRI = "checksum.sri"; private static final String QUALIFIER_CANONICAL_ID = "bazel.canonical_id"; + // When this environment variable is set to "1" in the client environment, + // the download order is reversed: the local (HTTP) downloader is tried first, + // and the remote asset API is only used as a fallback. + @VisibleForTesting + static final String REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV = "REVERSE_REMOTE_API_ATTEMPT_ORDER"; + // The `:` character is not permitted in an HTTP header name. So, we use it to // delimit the qualifier prefix which denotes an HTTP header qualifer from the // header name itself. @@ -131,6 +137,32 @@ public void download( Map clientEnv, Optional type) throws IOException, InterruptedException { + boolean localFirst = + fallbackDownloader != null + && "1".equals(clientEnv.get(REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV)); + + if (localFirst) { + try { + fallbackDownloader.download( + urls, + headers, + credentials, + checksum, + canonicalId, + destination, + eventHandler, + clientEnv, + type); + return; + } catch (IOException e) { + eventHandler.handle( + Event.warn( + "Local download failed: " + + e.getMessage() + + ", trying remote downloader")); + } + } + RequestMetadata metadata = TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "remote_downloader", null); RemoteActionExecutionContext remoteActionExecutionContext = @@ -159,11 +191,11 @@ public void download( }); } catch (StatusRuntimeException | IOException e) { - if (fallbackDownloader == null) { + if (localFirst || fallbackDownloader == null) { if (e instanceof StatusRuntimeException) { throw new IOException(e); } - throw e; + throw (IOException) e; } Optional url = urls.stream() @@ -181,7 +213,7 @@ public void download( eventHandler.handle( Event.warn("Remote Cache: " + Utils.grpcAwareErrorMessage(e, verboseFailures))); } - + fallbackDownloader.download( urls, headers, diff --git a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java index 60fed77876607e..f88b2e06aad52f 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java @@ -79,6 +79,7 @@ import java.util.Optional; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; @@ -178,11 +179,19 @@ public int maxConcurrency() { private static byte[] downloadBlob( GrpcRemoteDownloader downloader, URL url, Optional checksum) throws IOException, InterruptedException { + return downloadBlob(downloader, url, checksum, ImmutableMap.of()); + } + + private static byte[] downloadBlob( + GrpcRemoteDownloader downloader, + URL url, + Optional checksum, + Map clientEnv) + throws IOException, InterruptedException { final List urls = ImmutableList.of(url); final String canonicalId = ""; final ExtendedEventHandler eventHandler = mock(ExtendedEventHandler.class); - final Map clientEnv = ImmutableMap.of(); Scratch scratch = new Scratch(); final Path destination = scratch.resolve("output file path"); @@ -419,4 +428,163 @@ public void testFetchBlobRequest_withoutChecksum() throws Exception { Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID")) .build()); } + + @Test + public void testLocalFirstEnvVar_localSucceeds_remoteNotCalled() throws Exception { + // Remote service that should NOT be called. + final AtomicBoolean remoteCalled = new AtomicBoolean(false); + serviceRegistry.addService( + new FetchImplBase() { + @Override + public void fetchBlob( + FetchBlobRequest request, StreamObserver responseObserver) { + remoteCalled.set(true); + responseObserver.onError(new IOException("should not be called")); + } + }); + + final byte[] content = "local content".getBytes(UTF_8); + final RemoteCacheClient cacheClient = new InMemoryCacheClient(); + Downloader fallbackDownloader = mock(Downloader.class); + doAnswer( + invocation -> { + Path output = invocation.getArgument(5); + FileSystemUtils.writeContent(output, content); + return null; + }) + .when(fallbackDownloader) + .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); + final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); + + Map clientEnv = + ImmutableMap.of(GrpcRemoteDownloader.REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV, "1"); + + final byte[] downloaded = + downloadBlob( + downloader, + new URL("http://example.com/content.txt"), + Optional.empty(), + clientEnv); + + assertThat(downloaded).isEqualTo(content); + assertThat(remoteCalled.get()).isFalse(); + } + + @Test + public void testLocalFirstEnvVar_localFails_remoteFallback() throws Exception { + final byte[] content = "remote content".getBytes(UTF_8); + final Digest contentDigest = DIGEST_UTIL.compute(content); + + serviceRegistry.addService( + new FetchImplBase() { + @Override + public void fetchBlob( + FetchBlobRequest request, StreamObserver responseObserver) { + responseObserver.onNext( + FetchBlobResponse.newBuilder().setBlobDigest(contentDigest).build()); + responseObserver.onCompleted(); + } + }); + + final RemoteCacheClient cacheClient = new InMemoryCacheClient(); + getFromFuture(cacheClient.uploadBlob(context, contentDigest, ByteString.copyFrom(content))); + + Downloader fallbackDownloader = mock(Downloader.class); + doAnswer( + invocation -> { + throw new IOException("local download failed"); + }) + .when(fallbackDownloader) + .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); + final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); + + Map clientEnv = + ImmutableMap.of(GrpcRemoteDownloader.REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV, "1"); + + final byte[] downloaded = + downloadBlob( + downloader, + new URL("http://example.com/content.txt"), + Optional.empty(), + clientEnv); + + assertThat(downloaded).isEqualTo(content); + } + + @Test + public void testLocalFirstEnvVar_bothFail_throwsException() throws Exception { + serviceRegistry.addService( + new FetchImplBase() { + @Override + public void fetchBlob( + FetchBlobRequest request, StreamObserver responseObserver) { + responseObserver.onError(new IOException("remote failed")); + } + }); + + final RemoteCacheClient cacheClient = new InMemoryCacheClient(); + Downloader fallbackDownloader = mock(Downloader.class); + doAnswer( + invocation -> { + throw new IOException("local download failed"); + }) + .when(fallbackDownloader) + .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); + final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); + + Map clientEnv = + ImmutableMap.of(GrpcRemoteDownloader.REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV, "1"); + + assertThrows( + IOException.class, + () -> + downloadBlob( + downloader, + new URL("http://example.com/content.txt"), + Optional.empty(), + clientEnv)); + } + + @Test + public void testLocalFirstEnvVar_notSet_remoteCalledFirst() throws Exception { + final byte[] content = "remote content".getBytes(UTF_8); + final Digest contentDigest = DIGEST_UTIL.compute(content); + + serviceRegistry.addService( + new FetchImplBase() { + @Override + public void fetchBlob( + FetchBlobRequest request, StreamObserver responseObserver) { + responseObserver.onNext( + FetchBlobResponse.newBuilder().setBlobDigest(contentDigest).build()); + responseObserver.onCompleted(); + } + }); + + final RemoteCacheClient cacheClient = new InMemoryCacheClient(); + getFromFuture(cacheClient.uploadBlob(context, contentDigest, ByteString.copyFrom(content))); + + // Fallback that should NOT be called since remote succeeds. + final AtomicBoolean localCalled = new AtomicBoolean(false); + Downloader fallbackDownloader = mock(Downloader.class); + doAnswer( + invocation -> { + localCalled.set(true); + return null; + }) + .when(fallbackDownloader) + .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); + final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); + + // Empty clientEnv — env var not set. + final byte[] downloaded = + downloadBlob( + downloader, + new URL("http://example.com/content.txt"), + Optional.empty(), + ImmutableMap.of()); + + assertThat(downloaded).isEqualTo(content); + assertThat(localCalled.get()).isFalse(); + } } From 9d77f0382e72037d29f68049ad1c226c9a245182 Mon Sep 17 00:00:00 2001 From: Zack Winter Date: Fri, 10 Apr 2026 00:50:05 +0000 Subject: [PATCH 2/2] remove-test --- .../downloader/GrpcRemoteDownloaderTest.java | 170 +----------------- 1 file changed, 1 insertion(+), 169 deletions(-) diff --git a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java index f88b2e06aad52f..60fed77876607e 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java @@ -79,7 +79,6 @@ import java.util.Optional; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; @@ -179,19 +178,11 @@ public int maxConcurrency() { private static byte[] downloadBlob( GrpcRemoteDownloader downloader, URL url, Optional checksum) throws IOException, InterruptedException { - return downloadBlob(downloader, url, checksum, ImmutableMap.of()); - } - - private static byte[] downloadBlob( - GrpcRemoteDownloader downloader, - URL url, - Optional checksum, - Map clientEnv) - throws IOException, InterruptedException { final List urls = ImmutableList.of(url); final String canonicalId = ""; final ExtendedEventHandler eventHandler = mock(ExtendedEventHandler.class); + final Map clientEnv = ImmutableMap.of(); Scratch scratch = new Scratch(); final Path destination = scratch.resolve("output file path"); @@ -428,163 +419,4 @@ public void testFetchBlobRequest_withoutChecksum() throws Exception { Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID")) .build()); } - - @Test - public void testLocalFirstEnvVar_localSucceeds_remoteNotCalled() throws Exception { - // Remote service that should NOT be called. - final AtomicBoolean remoteCalled = new AtomicBoolean(false); - serviceRegistry.addService( - new FetchImplBase() { - @Override - public void fetchBlob( - FetchBlobRequest request, StreamObserver responseObserver) { - remoteCalled.set(true); - responseObserver.onError(new IOException("should not be called")); - } - }); - - final byte[] content = "local content".getBytes(UTF_8); - final RemoteCacheClient cacheClient = new InMemoryCacheClient(); - Downloader fallbackDownloader = mock(Downloader.class); - doAnswer( - invocation -> { - Path output = invocation.getArgument(5); - FileSystemUtils.writeContent(output, content); - return null; - }) - .when(fallbackDownloader) - .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); - final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); - - Map clientEnv = - ImmutableMap.of(GrpcRemoteDownloader.REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV, "1"); - - final byte[] downloaded = - downloadBlob( - downloader, - new URL("http://example.com/content.txt"), - Optional.empty(), - clientEnv); - - assertThat(downloaded).isEqualTo(content); - assertThat(remoteCalled.get()).isFalse(); - } - - @Test - public void testLocalFirstEnvVar_localFails_remoteFallback() throws Exception { - final byte[] content = "remote content".getBytes(UTF_8); - final Digest contentDigest = DIGEST_UTIL.compute(content); - - serviceRegistry.addService( - new FetchImplBase() { - @Override - public void fetchBlob( - FetchBlobRequest request, StreamObserver responseObserver) { - responseObserver.onNext( - FetchBlobResponse.newBuilder().setBlobDigest(contentDigest).build()); - responseObserver.onCompleted(); - } - }); - - final RemoteCacheClient cacheClient = new InMemoryCacheClient(); - getFromFuture(cacheClient.uploadBlob(context, contentDigest, ByteString.copyFrom(content))); - - Downloader fallbackDownloader = mock(Downloader.class); - doAnswer( - invocation -> { - throw new IOException("local download failed"); - }) - .when(fallbackDownloader) - .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); - final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); - - Map clientEnv = - ImmutableMap.of(GrpcRemoteDownloader.REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV, "1"); - - final byte[] downloaded = - downloadBlob( - downloader, - new URL("http://example.com/content.txt"), - Optional.empty(), - clientEnv); - - assertThat(downloaded).isEqualTo(content); - } - - @Test - public void testLocalFirstEnvVar_bothFail_throwsException() throws Exception { - serviceRegistry.addService( - new FetchImplBase() { - @Override - public void fetchBlob( - FetchBlobRequest request, StreamObserver responseObserver) { - responseObserver.onError(new IOException("remote failed")); - } - }); - - final RemoteCacheClient cacheClient = new InMemoryCacheClient(); - Downloader fallbackDownloader = mock(Downloader.class); - doAnswer( - invocation -> { - throw new IOException("local download failed"); - }) - .when(fallbackDownloader) - .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); - final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); - - Map clientEnv = - ImmutableMap.of(GrpcRemoteDownloader.REVERSE_REMOTE_API_ATTEMPT_ORDER_ENV, "1"); - - assertThrows( - IOException.class, - () -> - downloadBlob( - downloader, - new URL("http://example.com/content.txt"), - Optional.empty(), - clientEnv)); - } - - @Test - public void testLocalFirstEnvVar_notSet_remoteCalledFirst() throws Exception { - final byte[] content = "remote content".getBytes(UTF_8); - final Digest contentDigest = DIGEST_UTIL.compute(content); - - serviceRegistry.addService( - new FetchImplBase() { - @Override - public void fetchBlob( - FetchBlobRequest request, StreamObserver responseObserver) { - responseObserver.onNext( - FetchBlobResponse.newBuilder().setBlobDigest(contentDigest).build()); - responseObserver.onCompleted(); - } - }); - - final RemoteCacheClient cacheClient = new InMemoryCacheClient(); - getFromFuture(cacheClient.uploadBlob(context, contentDigest, ByteString.copyFrom(content))); - - // Fallback that should NOT be called since remote succeeds. - final AtomicBoolean localCalled = new AtomicBoolean(false); - Downloader fallbackDownloader = mock(Downloader.class); - doAnswer( - invocation -> { - localCalled.set(true); - return null; - }) - .when(fallbackDownloader) - .download(any(), any(), any(), any(), any(), any(), any(), any(), any()); - final GrpcRemoteDownloader downloader = newDownloader(cacheClient, fallbackDownloader); - - // Empty clientEnv — env var not set. - final byte[] downloaded = - downloadBlob( - downloader, - new URL("http://example.com/content.txt"), - Optional.empty(), - ImmutableMap.of()); - - assertThat(downloaded).isEqualTo(content); - assertThat(localCalled.get()).isFalse(); - } }