Adding a property name to the SpawnRunner. Most runners already had it, I just add it to the interface, and include it in the SpawnResult. This will be used to categorize/aggregate spawns executed by various runners. Also, minor refinement to the cacheHit property of the SpawnResult with remote execution. RELNOTES: None TESTED=presubmit, next cl PiperOrigin-RevId: 186637978
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java index 0c5feaa..4cfa9aa 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java
@@ -129,6 +129,12 @@ @Nullable String getExecutorHostName(); /** + * The name of the SpawnRunner that executed the spawn. It should always be defined, unless + * isCacheHit is true, in which case the spawn was not actually run. + */ + String getRunnerName(); + + /** * Returns the wall time taken by the {@link Spawn}'s execution. * * @return the measurement, or empty in case of execution errors or when the measurement is not @@ -206,6 +212,7 @@ private final int exitCode; private final Status status; private final String executorHostName; + private final String runnerName; private final Optional<Duration> wallTime; private final Optional<Duration> userTime; private final Optional<Duration> systemTime; @@ -219,6 +226,7 @@ this.exitCode = builder.exitCode; this.status = Preconditions.checkNotNull(builder.status); this.executorHostName = builder.executorHostName; + this.runnerName = builder.runnerName; this.wallTime = builder.wallTime; this.userTime = builder.userTime; this.systemTime = builder.systemTime; @@ -258,6 +266,11 @@ } @Override + public String getRunnerName() { + return runnerName; + } + + @Override public Optional<Duration> getWallTime() { return wallTime; } @@ -335,6 +348,7 @@ private int exitCode; private Status status; private String executorHostName; + private String runnerName = ""; private Optional<Duration> wallTime = Optional.empty(); private Optional<Duration> userTime = Optional.empty(); private Optional<Duration> systemTime = Optional.empty(); @@ -371,6 +385,11 @@ return this; } + public Builder setRunnerName(String runnerName) { + this.runnerName = runnerName; + return this; + } + public Builder setWallTime(Duration wallTime) { this.wallTime = Optional.of(wallTime); return this;
diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index bf6339d..36346a5 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
@@ -203,4 +203,7 @@ Spawn spawn, SpawnExecutionPolicy policy) throws InterruptedException, IOException, ExecException; + + /* Name of the SpawnRunner. */ + String getName(); }
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java index c695fd6..b10a62f 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java
@@ -114,14 +114,19 @@ } @Override + public String getName() { + return "local"; + } + + @Override public SpawnResult exec( Spawn spawn, SpawnExecutionPolicy policy) throws IOException, InterruptedException { ActionExecutionMetadata owner = spawn.getResourceOwner(); - policy.report(ProgressStatus.SCHEDULING, "local"); + policy.report(ProgressStatus.SCHEDULING, getName()); try (ResourceHandle handle = resourceManager.acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, "local"); + policy.report(ProgressStatus.EXECUTING, getName()); policy.lockOutputFiles(); return new SubprocessHandler(spawn, policy).run(); } @@ -227,6 +232,7 @@ ("Action type " + actionType + " is not allowed to run locally due to regex filter: " + localExecutionOptions.allowedLocalAction + "\n").getBytes(UTF_8)); return new SpawnResult.Builder() + .setRunnerName(getName()) .setStatus(Status.EXECUTION_DENIED) .setExitCode(LOCAL_EXEC_ERROR) .setExecutorHostname(hostName) @@ -310,6 +316,7 @@ .write(("Action failed to execute: " + msg + "\n").getBytes(UTF_8)); outErr.getErrorStream().flush(); return new SpawnResult.Builder() + .setRunnerName(getName()) .setStatus(Status.EXECUTION_FAILED) .setExitCode(LOCAL_EXEC_ERROR) .setExecutorHostname(hostName) @@ -331,6 +338,7 @@ : (exitCode == 0 ? Status.SUCCESS : Status.NON_ZERO_EXIT); SpawnResult.Builder spawnResultBuilder = new SpawnResult.Builder() + .setRunnerName(getName()) .setStatus(status) .setExitCode(exitCode) .setExecutorHostname(hostName)
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 0d0ae9a..a061bed 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -110,13 +110,18 @@ } @Override + public String getName() { + return "remote"; + } + + @Override public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) throws ExecException, InterruptedException, IOException { if (!Spawns.mayBeExecutedRemotely(spawn) || remoteCache == null) { return fallbackRunner.exec(spawn, policy); } - policy.report(ProgressStatus.EXECUTING, "remote"); + policy.report(ProgressStatus.EXECUTING, getName()); // Temporary hack: the TreeNodeRepository should be created and maintained upstream! ActionInputFileCache inputFileCache = policy.getActionInputFileCache(); TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil); @@ -182,6 +187,7 @@ } final ActionResult result; + boolean remoteCacheHit = false; try { ExecuteRequest.Builder request = ExecuteRequest.newBuilder() @@ -190,12 +196,16 @@ .setSkipCacheLookup(!acceptCachedResult); ExecuteResponse reply = remoteExecutor.executeRemotely(request.build()); result = reply.getResult(); + remoteCacheHit = reply.getCachedResult(); } catch (IOException e) { return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); } try { - return downloadRemoteResults(result, policy.getFileOutErr()).build(); + return downloadRemoteResults(result, policy.getFileOutErr()) + .setRunnerName(remoteCacheHit ? "" : getName()) + .setCacheHit(remoteCacheHit) + .build(); } catch (IOException e) { return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); } @@ -238,6 +248,7 @@ out.write(msg.getBytes(StandardCharsets.UTF_8)); } return new SpawnResult.Builder() + .setRunnerName(getName()) .setStatus(Status.TIMEOUT) .setExitCode(POSIX_TIMEOUT_EXIT_CODE) .build(); @@ -255,6 +266,7 @@ throw new SpawnExecException( Throwables.getStackTraceAsString(exception), new SpawnResult.Builder() + .setRunnerName(getName()) .setStatus(status) .setExitCode(ExitCode.REMOTE_ERROR.getNumericExitCode()) .build(),
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index 6985392..b7263b8 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java
@@ -165,6 +165,7 @@ outErr.getErrorStream().write(("Action failed to execute: " + msg + "\n").getBytes(UTF_8)); outErr.getErrorStream().flush(); return new SpawnResult.Builder() + .setRunnerName(getName()) .setStatus(Status.EXECUTION_FAILED) .setExitCode(LOCAL_EXEC_ERROR) .setFailureMessage(failureMessage) @@ -185,6 +186,7 @@ SpawnResult.Builder spawnResultBuilder = new SpawnResult.Builder() + .setRunnerName(getName()) .setStatus(status) .setExitCode(exitCode) .setWallTime(wallTime) @@ -292,6 +294,4 @@ protected SandboxOptions getSandboxOptions() { return sandboxOptions; } - - protected abstract String getName(); }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java index 9d924e9..1c23623 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedSpawnRunner.java
@@ -331,7 +331,7 @@ } @Override - protected String getName() { + public String getName() { return "darwin-sandbox"; } }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java index bd21c1b..7db25c8 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -173,7 +173,7 @@ } @Override - protected String getName() { + public String getName() { return "linux-sandbox"; }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java index 60bd17a..979878e 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedSpawnRunner.java
@@ -135,7 +135,7 @@ } @Override - protected String getName() { + public String getName() { return "processwrapper-sandbox"; } }
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java index 3e4af21..d125d7a 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
@@ -125,6 +125,11 @@ } @Override + public String getName() { + return "sandbox-fallback"; + } + + @Override public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) throws InterruptedException, IOException, ExecException { if (!Spawns.mayBeSandboxed(spawn)) {
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index 31e68be..f720d8c 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java
@@ -91,6 +91,11 @@ } @Override + public String getName() { + return "worker"; + } + + @Override public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) throws ExecException, IOException, InterruptedException { if (!spawn.getExecutionInfo().containsKey(ExecutionRequirements.SUPPORTS_WORKERS) @@ -103,11 +108,11 @@ return fallbackRunner.exec(spawn, policy); } - policy.report(ProgressStatus.SCHEDULING, "worker"); + policy.report(ProgressStatus.SCHEDULING, getName()); ActionExecutionMetadata owner = spawn.getResourceOwner(); try (ResourceHandle handle = ResourceManager.instance().acquireResources(owner, spawn.getLocalResources())) { - policy.report(ProgressStatus.EXECUTING, "worker"); + policy.report(ProgressStatus.EXECUTING, getName()); return actuallyExec(spawn, policy); } } @@ -161,6 +166,7 @@ int exitCode = response.getExitCode(); return new SpawnResult.Builder() + .setRunnerName(getName()) .setExitCode(exitCode) .setStatus(exitCode == 0 ? SpawnResult.Status.SUCCESS : SpawnResult.Status.NON_ZERO_EXIT) .setWallTime(wallTime)