diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index 88b2cc951d9b8b..ba37cbba0c390e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java @@ -192,7 +192,6 @@ public void registerSpawnCache(ModuleActionContextRegistry.Builder registryBuild env.getExecRoot(), checkNotNull(env.getOptions().getOptions(RemoteOptions.class)), checkNotNull(env.getOptions().getOptions(ExecutionOptions.class)).verboseFailures, - env.getReporter(), getRemoteExecutionService()); registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache"); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 195e1b6f61663c..0dfcedb90e743d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -132,10 +132,12 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Set; import java.util.SortedMap; import java.util.TreeSet; import java.util.concurrent.ConcurrentLinkedQueue; @@ -161,6 +163,7 @@ public class RemoteExecutionService { private final ImmutableSet filesToDownload; @Nullable private final Path captureCorruptedOutputsDir; private final Cache merkleTreeCache; + private final Set reportedErrors = new HashSet<>(); private final Scheduler scheduler; @@ -1186,10 +1189,9 @@ private void reportUploadError(Throwable error) { return; } - String errorMessage = - "Writing to Remote Cache: " + grpcAwareErrorMessage(error, verboseFailures); + String errorMessage = "Remote Cache: " + grpcAwareErrorMessage(error, verboseFailures); - reporter.handle(Event.warn(errorMessage)); + report(Event.warn(errorMessage)); } /** @@ -1312,4 +1314,15 @@ public void shutdown() { remoteExecutor.close(); } } + + void report(Event evt) { + + synchronized (this) { + if (reportedErrors.contains(evt.getMessage())) { + return; + } + reportedErrors.add(evt.getMessage()); + reporter.handle(evt); + } + } } 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 e40969b547c36c..3222d6a1ecdc27 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 @@ -31,7 +31,6 @@ import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.exec.SpawnCache; import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; import com.google.devtools.build.lib.exec.SpawnExecutingEvent; @@ -48,10 +47,7 @@ import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; -import java.util.HashSet; import java.util.NoSuchElementException; -import java.util.Set; -import javax.annotation.Nullable; /** A remote {@link SpawnCache} implementation. */ @ThreadSafe // If the RemoteActionCache implementation is thread-safe. @@ -66,20 +62,16 @@ final class RemoteSpawnCache implements SpawnCache { private final Path execRoot; private final RemoteOptions options; private final boolean verboseFailures; - @Nullable private final Reporter cmdlineReporter; - private final Set reportedErrors = new HashSet<>(); private final RemoteExecutionService remoteExecutionService; RemoteSpawnCache( Path execRoot, RemoteOptions options, boolean verboseFailures, - @Nullable Reporter cmdlineReporter, RemoteExecutionService remoteExecutionService) { this.execRoot = execRoot; this.options = options; this.verboseFailures = verboseFailures; - this.cmdlineReporter = cmdlineReporter; this.remoteExecutionService = remoteExecutionService; } @@ -150,13 +142,13 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) errorMessage = Utils.grpcAwareErrorMessage(e); } else { // On --verbose_failures print the whole stack trace - errorMessage = Throwables.getStackTraceAsString(e); + errorMessage = "\n" + Throwables.getStackTraceAsString(e); } if (isNullOrEmpty(errorMessage)) { errorMessage = e.getClass().getSimpleName(); } - errorMessage = "Reading from Remote Cache:\n" + errorMessage; - report(Event.warn(errorMessage)); + errorMessage = "Remote Cache: " + errorMessage; + remoteExecutionService.report(Event.warn(errorMessage)); } } } @@ -193,7 +185,7 @@ public void store(SpawnResult result) throws ExecException, InterruptedException try (SilentCloseable c = prof.profile("RemoteCache.checkForConcurrentModifications")) { checkForConcurrentModifications(); } catch (IOException | ForbiddenActionInputException e) { - report(Event.warn(e.getMessage())); + remoteExecutionService.report(Event.warn(e.getMessage())); return; } } @@ -223,20 +215,6 @@ private void checkForConcurrentModifications() } } - private void report(Event evt) { - if (cmdlineReporter == null) { - return; - } - - synchronized (this) { - if (reportedErrors.contains(evt.getMessage())) { - return; - } - reportedErrors.add(evt.getMessage()); - cmdlineReporter.handle(evt); - } - } - @Override public boolean usefulInDynamicExecution() { return false; 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 edf9133ac918e0..4996893f56e73e 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 @@ -226,7 +226,7 @@ private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { null, ImmutableSet.of(), /* captureCorruptedOutputsDir= */ null)); - return new RemoteSpawnCache(execRoot, options, /* verboseFailures=*/ true, reporter, service); + return new RemoteSpawnCache(execRoot, options, /* verboseFailures=*/ true, service); } @Before diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index dfdf71dfe90182..64a9cec6f74365 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -3266,14 +3266,14 @@ EOF # Check the error message when failed to upload bazel build --remote_cache=http://nonexistent.example.org //a:foo >& $TEST_log || fail "Failed to build" - expect_log "WARNING: Writing to Remote Cache:" + expect_log "WARNING: Remote Cache:" bazel test \ --remote_cache=grpc://localhost:${worker_port} \ --experimental_remote_cache_async \ --flaky_test_attempts=2 \ //a:test >& $TEST_log && fail "expected failure" || true - expect_not_log "WARNING: Writing to Remote Cache:" + expect_not_log "WARNING: Remote Cache:" } function test_download_toplevel_when_turn_remote_cache_off() {