Skip to content

Commit 988b56f

Browse files
larsrc-googlelarsrc-google
committed
Revert "Remote: Report checking cache status before the action is scheduled to run remotely."
This reverts commit 97d7b4c.
1 parent 1b19cd3 commit 988b56f

19 files changed

+81
-343
lines changed

‎src/main/java/com/google/devtools/build/lib/actions/CachingActionEvent.java‎

Lines changed: 0 additions & 35 deletions
This file was deleted.

‎src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
import com.google.devtools.build.lib.actions.LostInputsActionExecutionException;
2929
import com.google.devtools.build.lib.actions.LostInputsExecException;
3030
import com.google.devtools.build.lib.actions.MetadataProvider;
31+
import com.google.devtools.build.lib.actions.RunningActionEvent;
3132
import com.google.devtools.build.lib.actions.SandboxedSpawnStrategy;
33+
import com.google.devtools.build.lib.actions.SchedulingActionEvent;
3234
import com.google.devtools.build.lib.actions.Spawn;
3335
import com.google.devtools.build.lib.actions.SpawnExecutedEvent;
3436
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -306,7 +308,7 @@ public SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDir
306308
}
307309

308310
@Override
309-
public void report(ProgressStatus progress) {
311+
public void report(ProgressStatus state, String name) {
310312
ActionExecutionMetadata action = spawn.getResourceOwner();
311313
if (action.getOwner() == null) {
312314
return;
@@ -320,7 +322,17 @@ public void report(ProgressStatus progress) {
320322

321323
// TODO(ulfjack): We should report more details to the UI.
322324
ExtendedEventHandler eventHandler = actionExecutionContext.getEventHandler();
323-
progress.postTo(eventHandler, action);
325+
switch (state) {
326+
case EXECUTING:
327+
case CHECKING_CACHE:
328+
eventHandler.post(new RunningActionEvent(action, name));
329+
break;
330+
case SCHEDULING:
331+
eventHandler.post(new SchedulingActionEvent(action, name));
332+
break;
333+
default:
334+
break;
335+
}
324336
}
325337

326338
@Override

‎src/main/java/com/google/devtools/build/lib/exec/BUILD‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -264,21 +264,14 @@ java_library(
264264

265265
java_library(
266266
name = "spawn_runner",
267-
srcs = [
268-
"SpawnCheckingCacheEvent.java",
269-
"SpawnExecutingEvent.java",
270-
"SpawnRunner.java",
271-
"SpawnSchedulingEvent.java",
272-
],
267+
srcs = ["SpawnRunner.java"],
273268
deps = [
274269
":tree_deleter",
275270
"//src/main/java/com/google/devtools/build/lib/actions",
276271
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
277-
"//src/main/java/com/google/devtools/build/lib/events",
278272
"//src/main/java/com/google/devtools/build/lib/util/io",
279273
"//src/main/java/com/google/devtools/build/lib/vfs",
280274
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
281-
"//third_party:auto_value",
282275
"//third_party:jsr305",
283276
],
284277
)

‎src/main/java/com/google/devtools/build/lib/exec/SpawnCheckingCacheEvent.java‎

Lines changed: 0 additions & 35 deletions
This file was deleted.

‎src/main/java/com/google/devtools/build/lib/exec/SpawnExecutingEvent.java‎

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package com.google.devtools.build.lib.exec;
1515

1616
import com.google.devtools.build.lib.actions.ActionContext;
17-
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
1817
import com.google.devtools.build.lib.actions.ActionInput;
1918
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
2019
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
@@ -25,7 +24,6 @@
2524
import com.google.devtools.build.lib.actions.Spawn;
2625
import com.google.devtools.build.lib.actions.SpawnResult;
2726
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
28-
import com.google.devtools.build.lib.events.ExtendedEventHandler;
2927
import com.google.devtools.build.lib.util.io.FileOutErr;
3028
import com.google.devtools.build.lib.vfs.Path;
3129
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -105,9 +103,25 @@ public interface SpawnRunner {
105103
* <p>{@link SpawnRunner} implementations should post a progress status before any potentially
106104
* long-running operation.
107105
*/
108-
interface ProgressStatus {
109-
/** Post this progress event to the given {@link ExtendedEventHandler}. */
110-
void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action);
106+
enum ProgressStatus {
107+
/** Spawn is waiting for local or remote resources to become available. */
108+
SCHEDULING,
109+
110+
/** The {@link SpawnRunner} is looking for a cache hit. */
111+
CHECKING_CACHE,
112+
113+
/**
114+
* Resources are acquired, and there was probably no cache hit. This MUST be posted before
115+
* attempting to execute the subprocess.
116+
*
117+
* <p>Caching {@link SpawnRunner} implementations should only post this after a failed cache
118+
* lookup, but may post this if cache lookup and execution happen within the same step, e.g. as
119+
* part of a single RPC call with no mechanism to report cache misses.
120+
*/
121+
EXECUTING,
122+
123+
/** Downloading outputs from a remote machine. */
124+
DOWNLOADING
111125
}
112126

113127
/**
@@ -199,7 +213,7 @@ SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDirectory)
199213
throws IOException;
200214

201215
/** Reports a progress update to the Spawn strategy. */
202-
void report(ProgressStatus progress);
216+
void report(ProgressStatus state, String name);
203217

204218
/**
205219
* Returns a {@link MetadataInjector} that allows a caller to inject metadata about spawn

‎src/main/java/com/google/devtools/build/lib/exec/SpawnSchedulingEvent.java‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@
3838
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
3939
import com.google.devtools.build.lib.exec.BinTools;
4040
import com.google.devtools.build.lib.exec.RunfilesTreeUpdater;
41-
import com.google.devtools.build.lib.exec.SpawnExecutingEvent;
4241
import com.google.devtools.build.lib.exec.SpawnRunner;
43-
import com.google.devtools.build.lib.exec.SpawnSchedulingEvent;
4442
import com.google.devtools.build.lib.profiler.Profiler;
4543
import com.google.devtools.build.lib.profiler.ProfilerTask;
4644
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -133,10 +131,10 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
133131
Profiler.instance()
134132
.profile(ProfilerTask.LOCAL_EXECUTION, spawn.getResourceOwner().getMnemonic())) {
135133
ActionExecutionMetadata owner = spawn.getResourceOwner();
136-
context.report(SpawnSchedulingEvent.create(getName()));
134+
context.report(ProgressStatus.SCHEDULING, getName());
137135
try (ResourceHandle handle =
138136
resourceManager.acquireResources(owner, spawn.getLocalResources())) {
139-
context.report(SpawnExecutingEvent.create(getName()));
137+
context.report(ProgressStatus.EXECUTING, getName());
140138
if (!localExecutionOptions.localLockfreeOutput) {
141139
context.lockOutputFiles();
142140
}

‎src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@
3131
import com.google.devtools.build.lib.events.Event;
3232
import com.google.devtools.build.lib.events.Reporter;
3333
import com.google.devtools.build.lib.exec.SpawnCache;
34-
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
35-
import com.google.devtools.build.lib.exec.SpawnExecutingEvent;
34+
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
3635
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
3736
import com.google.devtools.build.lib.profiler.Profiler;
3837
import com.google.devtools.build.lib.profiler.ProfilerTask;
@@ -54,12 +53,6 @@
5453
@ThreadSafe // If the RemoteActionCache implementation is thread-safe.
5554
final class RemoteSpawnCache implements SpawnCache {
5655

57-
private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT =
58-
SpawnCheckingCacheEvent.create("remote-cache");
59-
60-
private static final SpawnExecutingEvent SPAWN_EXECUTING_EVENT =
61-
SpawnExecutingEvent.create("remote-cache");
62-
6356
private final Path execRoot;
6457
private final RemoteOptions options;
6558
private final boolean verboseFailures;
@@ -103,7 +96,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
10396
Profiler prof = Profiler.instance();
10497
if (options.remoteAcceptCached
10598
|| (options.incompatibleRemoteResultsIgnoreDisk && useDiskCache(options))) {
106-
context.report(SPAWN_CHECKING_CACHE_EVENT);
99+
context.report(ProgressStatus.CHECKING_CACHE, "remote-cache");
107100
// Metadata will be available in context.current() until we detach.
108101
// This is done via a thread-local variable.
109102
try {
@@ -157,8 +150,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
157150
}
158151
}
159152

160-
context.report(SPAWN_EXECUTING_EVENT);
161-
162153
context.prefetchInputs();
163154

164155
if (options.remoteUploadLocalResults

‎src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,7 @@
4444
import com.google.devtools.build.lib.exec.AbstractSpawnStrategy;
4545
import com.google.devtools.build.lib.exec.ExecutionOptions;
4646
import com.google.devtools.build.lib.exec.RemoteLocalFallbackRegistry;
47-
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
48-
import com.google.devtools.build.lib.exec.SpawnExecutingEvent;
4947
import com.google.devtools.build.lib.exec.SpawnRunner;
50-
import com.google.devtools.build.lib.exec.SpawnSchedulingEvent;
5148
import com.google.devtools.build.lib.profiler.Profiler;
5249
import com.google.devtools.build.lib.profiler.ProfilerTask;
5350
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -83,15 +80,6 @@
8380
@ThreadSafe
8481
public class RemoteSpawnRunner implements SpawnRunner {
8582

86-
private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT =
87-
SpawnCheckingCacheEvent.create("remote");
88-
89-
private static final SpawnSchedulingEvent SPAWN_SCHEDULING_EVENT =
90-
SpawnSchedulingEvent.create("remote");
91-
92-
private static final SpawnExecutingEvent SPAWN_EXECUTING_EVENT =
93-
SpawnExecutingEvent.create("remote");
94-
9583
private final Path execRoot;
9684
private final RemoteOptions remoteOptions;
9785
private final ExecutionOptions executionOptions;
@@ -154,7 +142,7 @@ public void onNext(Operation o) throws IOException {
154142
}
155143

156144
public void reportExecuting() {
157-
context.report(SPAWN_EXECUTING_EVENT);
145+
context.report(ProgressStatus.EXECUTING, getName());
158146
reportedExecuting = true;
159147
}
160148

@@ -176,6 +164,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
176164
boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCacheableRemotely;
177165
boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCacheableRemotely;
178166

167+
context.report(ProgressStatus.SCHEDULING, getName());
168+
179169
RemoteAction action = remoteExecutionService.buildRemoteAction(spawn, context);
180170
SpawnMetrics.Builder spawnMetrics =
181171
SpawnMetrics.Builder.forRemoteExec()
@@ -188,8 +178,6 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
188178

189179
Profiler prof = Profiler.instance();
190180
try {
191-
context.report(SPAWN_CHECKING_CACHE_EVENT);
192-
193181
// Try to lookup the action in the action cache.
194182
RemoteActionResult cachedResult;
195183
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
@@ -243,8 +231,6 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
243231
.minus(action.getNetworkTime().getDuration().minus(networkTimeStart)));
244232
}
245233

246-
context.report(SPAWN_SCHEDULING_EVENT);
247-
248234
ExecutingStatusReporter reporter = new ExecutingStatusReporter(context);
249235
RemoteActionResult result;
250236
try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) {

0 commit comments

Comments
 (0)