Avoid too verbose warnings in terminal when cache issues
Deduplicate warnings in terminal. This was working in earlier bazel
versions both for read and write, but become broken when write was
moved to RemoteExecutionService.java by the "Remote: Async upload"
set of commits, completed by commit 581c81a854.
Use same phrase "Remote Cache" for both read and write, for
deduplication to work better.
Avoid printing short warnings on multiple lines for reads, as it
already was for writes.
Closes #14442.
PiperOrigin-RevId: 419442535
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 88b2cc9..ba37cbb 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 @@
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 195e1b6..0dfcedb 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 @@
private final ImmutableSet<PathFragment> filesToDownload;
@Nullable private final Path captureCorruptedOutputsDir;
private final Cache<Object, MerkleTree> merkleTreeCache;
+ private final Set<String> reportedErrors = new HashSet<>();
private final Scheduler scheduler;
@@ -1186,10 +1189,9 @@
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 @@
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 e40969b..3222d6a 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 @@
private final Path execRoot;
private final RemoteOptions options;
private final boolean verboseFailures;
- @Nullable private final Reporter cmdlineReporter;
- private final Set<String> 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 @@
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 @@
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 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 edf9133..4996893 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 @@
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 dfdf71d..64a9cec 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 @@
# 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() {
