Properly attaching context for remote uploads in RemoteSpawnCache.
Fixes #3930. Also added tests.
TESTED=added tests
RELNOTES: Fixing regression to --experimental_remote_spawn_cache
PiperOrigin-RevId: 172852740
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index 3a8d0a3..c6c7664 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -126,47 +126,50 @@
// There's a cache miss. Fall back to local execution.
}
}
- if (options.remoteUploadLocalResults) {
- return new CacheHandle() {
- @Override
- public boolean hasResult() {
- return false;
- }
-
- @Override
- public SpawnResult getResult() {
- throw new NoSuchElementException();
- }
-
- @Override
- public boolean willStore() {
- return true;
- }
-
- @Override
- public void store(SpawnResult result, Collection<Path> files)
- throws InterruptedException, IOException {
- boolean uploadAction = Status.SUCCESS.equals(result.status()) && result.exitCode() == 0;
- try {
- remoteCache.upload(actionKey, execRoot, files, policy.getFileOutErr(), uploadAction);
- } catch (IOException e) {
- if (verboseFailures) {
- report(Event.debug("Upload to remote cache failed: " + e.getMessage()));
- } else {
- reportOnce(Event.warn("Some artifacts failed be uploaded to the remote cache."));
- }
- }
- }
-
- @Override
- public void close() {}
- };
- } else {
- return SpawnCache.NO_RESULT_NO_STORE;
- }
} finally {
withMetadata.detach(previous);
}
+ if (options.remoteUploadLocalResults) {
+ return new CacheHandle() {
+ @Override
+ public boolean hasResult() {
+ return false;
+ }
+
+ @Override
+ public SpawnResult getResult() {
+ throw new NoSuchElementException();
+ }
+
+ @Override
+ public boolean willStore() {
+ return true;
+ }
+
+ @Override
+ public void store(SpawnResult result, Collection<Path> files)
+ throws InterruptedException, IOException {
+ boolean uploadAction = Status.SUCCESS.equals(result.status()) && result.exitCode() == 0;
+ Context previous = withMetadata.attach();
+ try {
+ remoteCache.upload(actionKey, execRoot, files, policy.getFileOutErr(), uploadAction);
+ } catch (IOException e) {
+ if (verboseFailures) {
+ report(Event.debug("Upload to remote cache failed: " + e.getMessage()));
+ } else {
+ reportOnce(Event.warn("Some artifacts failed be uploaded to the remote cache."));
+ }
+ } finally {
+ withMetadata.detach(previous);
+ }
+ }
+
+ @Override
+ public void close() {}
+ };
+ } else {
+ return SpawnCache.NO_RESULT_NO_STORE;
+ }
}
private void reportOnce(Event evt) {
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
index 215f2b5..3295df6 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
@@ -53,6 +53,7 @@
import com.google.devtools.common.options.Options;
import com.google.devtools.remoteexecution.v1test.ActionResult;
import com.google.devtools.remoteexecution.v1test.Command;
+import com.google.devtools.remoteexecution.v1test.RequestMetadata;
import java.io.IOException;
import java.time.Duration;
import java.util.Collection;
@@ -62,7 +63,10 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.Mock;
+import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
/** Tests for {@link RemoteSpawnCache}. */
@RunWith(JUnit4.class)
@@ -176,7 +180,29 @@
@Test
public void cacheHit() throws Exception {
ActionResult actionResult = ActionResult.getDefaultInstance();
- when(remoteCache.getCachedActionResult(any(ActionKey.class))).thenReturn(actionResult);
+ when(remoteCache.getCachedActionResult(any(ActionKey.class)))
+ .thenAnswer(
+ new Answer<ActionResult>() {
+ @Override
+ public ActionResult answer(InvocationOnMock invocation) {
+ RequestMetadata meta = TracingMetadataUtils.fromCurrentContext();
+ assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id");
+ assertThat(meta.getToolInvocationId()).isEqualTo("command-id");
+ return actionResult;
+ }
+ });
+ Mockito.doAnswer(
+ new Answer<Void>() {
+ @Override
+ public Void answer(InvocationOnMock invocation) {
+ RequestMetadata meta = TracingMetadataUtils.fromCurrentContext();
+ assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id");
+ assertThat(meta.getToolInvocationId()).isEqualTo("command-id");
+ return null;
+ }
+ })
+ .when(remoteCache)
+ .download(actionResult, execRoot, outErr);
CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy);
assertThat(entry.hasResult()).isTrue();
@@ -209,6 +235,18 @@
assertThat(entry.hasResult()).isFalse();
SpawnResult result = new SpawnResult.Builder().setExitCode(0).setStatus(Status.SUCCESS).build();
ImmutableList<Path> outputFiles = ImmutableList.of(fs.getPath("/random/file"));
+ Mockito.doAnswer(
+ new Answer<Void>() {
+ @Override
+ public Void answer(InvocationOnMock invocation) {
+ RequestMetadata meta = TracingMetadataUtils.fromCurrentContext();
+ assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id");
+ assertThat(meta.getToolInvocationId()).isEqualTo("command-id");
+ return null;
+ }
+ })
+ .when(remoteCache)
+ .upload(any(ActionKey.class), any(Path.class), eq(outputFiles), eq(outErr), eq(true));
entry.store(result, outputFiles);
verify(remoteCache)
.upload(any(ActionKey.class), any(Path.class), eq(outputFiles), eq(outErr), eq(true));
