Skip to content

Commit 97d7b4c

Browse files
coeuvrecoeuvre
committed
Remote: Report checking cache status before the action is scheduled to run remotely.
Add a Caching state to ActionState. This state indicates the action is checking the cache and should be happened before Scheduling state. Change ProgressStatus from enum to interface so that we can pass more data to the event handler (which is required to report upload/download details later). Fixes #13531. Closes #13555. PiperOrigin-RevId: 378800212
1 parent 00efa84 commit 97d7b4c

19 files changed

+343
-81
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.actions;
15+
16+
import static com.google.common.base.Preconditions.checkNotNull;
17+
18+
import com.google.auto.value.AutoValue;
19+
import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
20+
21+
/** Notifies that an in-flight action is checking the cache. */
22+
@AutoValue
23+
public abstract class CachingActionEvent implements ProgressLike {
24+
25+
public static CachingActionEvent create(ActionExecutionMetadata action, String strategy) {
26+
return new AutoValue_CachingActionEvent(
27+
action, checkNotNull(strategy, "Strategy names are not optional"));
28+
}
29+
30+
/** Gets the metadata associated with the action. */
31+
public abstract ActionExecutionMetadata action();
32+
33+
/** Gets the name of the strategy on which the action is caching. */
34+
public abstract String strategy();
35+
}

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@
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;
3231
import com.google.devtools.build.lib.actions.SandboxedSpawnStrategy;
33-
import com.google.devtools.build.lib.actions.SchedulingActionEvent;
3432
import com.google.devtools.build.lib.actions.Spawn;
3533
import com.google.devtools.build.lib.actions.SpawnExecutedEvent;
3634
import com.google.devtools.build.lib.actions.SpawnResult;
@@ -308,7 +306,7 @@ public SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDir
308306
}
309307

310308
@Override
311-
public void report(ProgressStatus state, String name) {
309+
public void report(ProgressStatus progress) {
312310
ActionExecutionMetadata action = spawn.getResourceOwner();
313311
if (action.getOwner() == null) {
314312
return;
@@ -322,17 +320,7 @@ public void report(ProgressStatus state, String name) {
322320

323321
// TODO(ulfjack): We should report more details to the UI.
324322
ExtendedEventHandler eventHandler = actionExecutionContext.getEventHandler();
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-
}
323+
progress.postTo(eventHandler, action);
336324
}
337325

338326
@Override

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

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

265265
java_library(
266266
name = "spawn_runner",
267-
srcs = ["SpawnRunner.java"],
267+
srcs = [
268+
"SpawnCheckingCacheEvent.java",
269+
"SpawnExecutingEvent.java",
270+
"SpawnRunner.java",
271+
"SpawnSchedulingEvent.java",
272+
],
268273
deps = [
269274
":tree_deleter",
270275
"//src/main/java/com/google/devtools/build/lib/actions",
271276
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
277+
"//src/main/java/com/google/devtools/build/lib/events",
272278
"//src/main/java/com/google/devtools/build/lib/util/io",
273279
"//src/main/java/com/google/devtools/build/lib/vfs",
274280
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
281+
"//third_party:auto_value",
275282
"//third_party:jsr305",
276283
],
277284
)

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.exec;
15+
16+
import com.google.auto.value.AutoValue;
17+
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
18+
import com.google.devtools.build.lib.actions.CachingActionEvent;
19+
import com.google.devtools.build.lib.events.ExtendedEventHandler;
20+
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
21+
22+
/** Notifies that {@link SpawnRunner} is looking for a cache hit. */
23+
@AutoValue
24+
public abstract class SpawnCheckingCacheEvent implements ProgressStatus {
25+
public static SpawnCheckingCacheEvent create(String name) {
26+
return new AutoValue_SpawnCheckingCacheEvent(name);
27+
}
28+
29+
public abstract String name();
30+
31+
@Override
32+
public void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action) {
33+
eventHandler.post(CachingActionEvent.create(action, name()));
34+
}
35+
}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.exec;
15+
16+
import com.google.auto.value.AutoValue;
17+
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
18+
import com.google.devtools.build.lib.actions.RunningActionEvent;
19+
import com.google.devtools.build.lib.events.ExtendedEventHandler;
20+
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
21+
22+
/**
23+
* Notifies that {@link SpawnRunner} failed to find a cache hit and acquired the resources to
24+
* execute. This MUST be posted before attempting to execute the subprocess.
25+
*
26+
* <p>Caching {@link SpawnRunner} implementations should only post this after a failed cache lookup,
27+
* but may post this if cache lookup and execution happen within the same step, e.g. as part of a
28+
* single RPC call with no mechanism to report cache misses.
29+
*/
30+
@AutoValue
31+
public abstract class SpawnExecutingEvent implements ProgressStatus {
32+
public static SpawnExecutingEvent create(String name) {
33+
return new AutoValue_SpawnExecutingEvent(name);
34+
}
35+
36+
public abstract String name();
37+
38+
@Override
39+
public void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action) {
40+
eventHandler.post(new RunningActionEvent(action, name()));
41+
}
42+
}

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

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
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;
1718
import com.google.devtools.build.lib.actions.ActionInput;
1819
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
1920
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
@@ -24,6 +25,7 @@
2425
import com.google.devtools.build.lib.actions.Spawn;
2526
import com.google.devtools.build.lib.actions.SpawnResult;
2627
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
28+
import com.google.devtools.build.lib.events.ExtendedEventHandler;
2729
import com.google.devtools.build.lib.util.io.FileOutErr;
2830
import com.google.devtools.build.lib.vfs.Path;
2931
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -103,25 +105,9 @@ public interface SpawnRunner {
103105
* <p>{@link SpawnRunner} implementations should post a progress status before any potentially
104106
* long-running operation.
105107
*/
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
108+
interface ProgressStatus {
109+
/** Post this progress event to the given {@link ExtendedEventHandler}. */
110+
void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action);
125111
}
126112

127113
/**
@@ -213,7 +199,7 @@ SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDirectory)
213199
throws IOException;
214200

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

218204
/**
219205
* 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: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.exec;
15+
16+
import com.google.auto.value.AutoValue;
17+
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
18+
import com.google.devtools.build.lib.actions.SchedulingActionEvent;
19+
import com.google.devtools.build.lib.events.ExtendedEventHandler;
20+
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
21+
22+
/**
23+
* Notifies that {@link SpawnRunner} is waiting for local or remote resources to become available.
24+
*/
25+
@AutoValue
26+
public abstract class SpawnSchedulingEvent implements ProgressStatus {
27+
public static SpawnSchedulingEvent create(String name) {
28+
return new AutoValue_SpawnSchedulingEvent(name);
29+
}
30+
31+
public abstract String name();
32+
33+
@Override
34+
public void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action) {
35+
eventHandler.post(new SchedulingActionEvent(action, name()));
36+
}
37+
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@
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;
4142
import com.google.devtools.build.lib.exec.SpawnRunner;
43+
import com.google.devtools.build.lib.exec.SpawnSchedulingEvent;
4244
import com.google.devtools.build.lib.profiler.Profiler;
4345
import com.google.devtools.build.lib.profiler.ProfilerTask;
4446
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -131,10 +133,10 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
131133
Profiler.instance()
132134
.profile(ProfilerTask.LOCAL_EXECUTION, spawn.getResourceOwner().getMnemonic())) {
133135
ActionExecutionMetadata owner = spawn.getResourceOwner();
134-
context.report(ProgressStatus.SCHEDULING, getName());
136+
context.report(SpawnSchedulingEvent.create(getName()));
135137
try (ResourceHandle handle =
136138
resourceManager.acquireResources(owner, spawn.getLocalResources())) {
137-
context.report(ProgressStatus.EXECUTING, getName());
139+
context.report(SpawnExecutingEvent.create(getName()));
138140
if (!localExecutionOptions.localLockfreeOutput) {
139141
context.lockOutputFiles();
140142
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
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.SpawnRunner.ProgressStatus;
34+
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
35+
import com.google.devtools.build.lib.exec.SpawnExecutingEvent;
3536
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
3637
import com.google.devtools.build.lib.profiler.Profiler;
3738
import com.google.devtools.build.lib.profiler.ProfilerTask;
@@ -53,6 +54,12 @@
5354
@ThreadSafe // If the RemoteActionCache implementation is thread-safe.
5455
final class RemoteSpawnCache implements SpawnCache {
5556

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+
5663
private final Path execRoot;
5764
private final RemoteOptions options;
5865
private final boolean verboseFailures;
@@ -96,7 +103,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
96103
Profiler prof = Profiler.instance();
97104
if (options.remoteAcceptCached
98105
|| (options.incompatibleRemoteResultsIgnoreDisk && useDiskCache(options))) {
99-
context.report(ProgressStatus.CHECKING_CACHE, "remote-cache");
106+
context.report(SPAWN_CHECKING_CACHE_EVENT);
100107
// Metadata will be available in context.current() until we detach.
101108
// This is done via a thread-local variable.
102109
try {
@@ -150,6 +157,8 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
150157
}
151158
}
152159

160+
context.report(SPAWN_EXECUTING_EVENT);
161+
153162
context.prefetchInputs();
154163

155164
if (options.remoteUploadLocalResults

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@
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;
4749
import com.google.devtools.build.lib.exec.SpawnRunner;
50+
import com.google.devtools.build.lib.exec.SpawnSchedulingEvent;
4851
import com.google.devtools.build.lib.profiler.Profiler;
4952
import com.google.devtools.build.lib.profiler.ProfilerTask;
5053
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -80,6 +83,15 @@
8083
@ThreadSafe
8184
public class RemoteSpawnRunner implements SpawnRunner {
8285

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+
8395
private final Path execRoot;
8496
private final RemoteOptions remoteOptions;
8597
private final ExecutionOptions executionOptions;
@@ -142,7 +154,7 @@ public void onNext(Operation o) throws IOException {
142154
}
143155

144156
public void reportExecuting() {
145-
context.report(ProgressStatus.EXECUTING, getName());
157+
context.report(SPAWN_EXECUTING_EVENT);
146158
reportedExecuting = true;
147159
}
148160

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

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

179189
Profiler prof = Profiler.instance();
180190
try {
191+
context.report(SPAWN_CHECKING_CACHE_EVENT);
192+
181193
// Try to lookup the action in the action cache.
182194
RemoteActionResult cachedResult;
183195
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
@@ -231,6 +243,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
231243
.minus(action.getNetworkTime().getDuration().minus(networkTimeStart)));
232244
}
233245

246+
context.report(SPAWN_SCHEDULING_EVENT);
247+
234248
ExecutingStatusReporter reporter = new ExecutingStatusReporter(context);
235249
RemoteActionResult result;
236250
try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) {

0 commit comments

Comments
 (0)