Set oldest_content_accepted for remote downloader requests without the checksum
This PR address the #23932 issue with remote downloader.
Closes #23970.
PiperOrigin-RevId: 686180819
Change-Id: Ia36c5793622bd1ac1fdaa9ef1326ddc385a5a01f
diff --git a/src/main/java/com/google/devtools/build/lib/remote/downloader/BUILD b/src/main/java/com/google/devtools/build/lib/remote/downloader/BUILD
index dccb139..16c71bd 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/downloader/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/downloader/BUILD
@@ -16,6 +16,7 @@
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
+ "//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/remote:ReferenceCountedChannel",
"//src/main/java/com/google/devtools/build/lib/remote:Retrier",
@@ -27,6 +28,7 @@
"//third_party:guava",
"//third_party:jsr305",
"//third_party/grpc-java:grpc-jar",
+ "@protobuf//:protobuf_java_util",
"@remoteapis//:build_bazel_remote_asset_v1_remote_asset_java_grpc",
"@remoteapis//:build_bazel_remote_asset_v1_remote_asset_java_proto",
"@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto",
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 8cf643b..9720347 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
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
import com.google.devtools.build.lib.bazel.repository.downloader.Downloader;
import com.google.devtools.build.lib.bazel.repository.downloader.HashOutputStream;
+import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.remote.ReferenceCountedChannel;
@@ -38,6 +39,7 @@
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
import com.google.devtools.build.lib.remote.util.Utils;
import com.google.devtools.build.lib.vfs.Path;
+import com.google.protobuf.util.Timestamps;
import io.grpc.CallCredentials;
import io.grpc.Channel;
import io.grpc.StatusRuntimeException;
@@ -45,6 +47,7 @@
import java.io.OutputStream;
import java.net.URISyntaxException;
import java.net.URL;
+import java.time.Duration;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -235,6 +238,12 @@
.setName(QUALIFIER_CHECKSUM_SRI)
.setValue(checksum.get().toSubresourceIntegrity())
.build());
+ } else {
+ // If no checksum is provided, never accept cached content.
+ // Timestamp is offset by an hour to account for clock skew.
+ requestBuilder.setOldestContentAccepted(
+ Timestamps.fromMillis(
+ BlazeClock.instance().now().plus(Duration.ofHours(1)).toEpochMilli()));
}
if (!Strings.isNullOrEmpty(canonicalId)) {
diff --git a/src/test/java/com/google/devtools/build/lib/remote/downloader/BUILD b/src/test/java/com/google/devtools/build/lib/remote/downloader/BUILD
index 6f45cf4..d5cc6da 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/downloader/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/remote/downloader/BUILD
@@ -24,6 +24,7 @@
"//src/main/java/com/google/devtools/build/lib/authandtls",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
+ "//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/remote",
"//src/main/java/com/google/devtools/build/lib/remote/common",
@@ -44,6 +45,7 @@
"//third_party:truth",
"//third_party/grpc-java:grpc-jar",
"@protobuf//:protobuf_java",
+ "@protobuf//:protobuf_java_util",
"@remoteapis//:build_bazel_remote_asset_v1_remote_asset_java_grpc",
"@remoteapis//:build_bazel_remote_asset_v1_remote_asset_java_proto",
"@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto",
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 1e2ba06..b6ca8c4 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
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
import com.google.devtools.build.lib.bazel.repository.downloader.Downloader;
import com.google.devtools.build.lib.bazel.repository.downloader.UnrecoverableHttpException;
+import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.remote.ChannelConnectionWithServerCapabilitiesFactory;
import com.google.devtools.build.lib.remote.ReferenceCountedChannel;
@@ -54,6 +55,7 @@
import com.google.devtools.build.lib.remote.util.InMemoryCacheClient;
import com.google.devtools.build.lib.remote.util.TestUtils;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
+import com.google.devtools.build.lib.testutil.ManualClock;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -61,6 +63,7 @@
import com.google.devtools.build.lib.vfs.SyscallCache;
import com.google.devtools.common.options.Options;
import com.google.protobuf.ByteString;
+import com.google.protobuf.util.Timestamps;
import io.grpc.CallCredentials;
import io.grpc.ManagedChannel;
import io.grpc.Server;
@@ -73,6 +76,7 @@
import java.io.InputStream;
import java.net.URI;
import java.net.URL;
+import java.time.Duration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -90,6 +94,8 @@
@RunWith(JUnit4.class)
public class GrpcRemoteDownloaderTest {
+ private static final ManualClock clock = new ManualClock();
+
private static final DigestUtil DIGEST_UTIL =
new DigestUtil(SyscallCache.NO_CACHE, DigestHashFunction.SHA256);
@@ -117,6 +123,8 @@
context = RemoteActionExecutionContext.create(metadata);
retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
+
+ BlazeClock.setClock(clock);
}
@After
@@ -212,6 +220,8 @@
.isEqualTo(
FetchBlobRequest.newBuilder()
.setDigestFunction(DIGEST_UTIL.getDigestFunction())
+ .setOldestContentAccepted(
+ Timestamps.fromMillis(clock.advance(Duration.ofHours(1))))
.addUris("http://example.com/content.txt")
.build());
responseObserver.onNext(
@@ -474,4 +484,29 @@
Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID"))
.build());
}
+
+ @Test
+ public void testFetchBlobRequest_withoutChecksum() throws Exception {
+ FetchBlobRequest request =
+ GrpcRemoteDownloader.newFetchBlobRequest(
+ "instance name",
+ false,
+ ImmutableList.of(new URI("http://example.com/").toURL()),
+ Optional.<Checksum>empty(),
+ "canonical ID",
+ DIGEST_UTIL.getDigestFunction(),
+ ImmutableMap.of(),
+ StaticCredentials.EMPTY);
+
+ assertThat(request)
+ .isEqualTo(
+ FetchBlobRequest.newBuilder()
+ .setInstanceName("instance name")
+ .setDigestFunction(DIGEST_UTIL.getDigestFunction())
+ .setOldestContentAccepted(Timestamps.fromMillis(clock.advance(Duration.ofHours(1))))
+ .addUris("http://example.com/")
+ .addQualifiers(
+ Qualifier.newBuilder().setName("bazel.canonical_id").setValue("canonical ID"))
+ .build());
+ }
}
