diff options
5 files changed, 100 insertions, 2 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java index 06aabda492..3dabf0a677 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java @@ -73,8 +73,7 @@ class GrpcRemoteExecutor { if (e.getStatus().getCode() == Code.DEADLINE_EXCEEDED) { // This was caused by the command itself exceeding the timeout, // therefore it is not retriable. - // TODO(olaola): this should propagate a timeout SpawnResult instead of raising. - throw new IOException("Remote execution time out"); + throw new TimeoutException(); } throw e; } 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 26fba9d25c..69b751cbda 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 @@ -45,6 +45,8 @@ import com.google.devtools.remoteexecution.v1test.ExecuteResponse; import com.google.devtools.remoteexecution.v1test.Platform; import io.grpc.Status.Code; import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.ArrayList; import java.util.Collection; @@ -207,9 +209,25 @@ class RemoteSpawnRunner implements SpawnRunner { if (options.remoteLocalFallback) { return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey); } + if (cause instanceof TimeoutException) { + return handleTimeout(policy.getFileOutErr()); + } throw new EnvironmentalExecException(errorMessage(cause), cause, true); } + private SpawnResult handleTimeout(FileOutErr outErr) throws IOException { + // TODO(buchgr): Once the remote execution protocol allows it, also provide stdout/stderr + // from the action that timed out. + try (OutputStream out = outErr.getOutputStream()) { + // This is a hack to ensure that the test.log file gets created, as else the action + // will complain that one of its outputs has not been created. + String msg = "Log output for timeouts is not yet supported in remote execution."; + out.write(msg.getBytes(StandardCharsets.UTF_8)); + out.flush(); + } + return new SpawnResult.Builder().setStatus(Status.TIMEOUT).build(); + } + private String errorMessage(IOException e) { String message = ""; if (e instanceof RetryException diff --git a/src/main/java/com/google/devtools/build/lib/remote/TimeoutException.java b/src/main/java/com/google/devtools/build/lib/remote/TimeoutException.java new file mode 100644 index 0000000000..f1ba33a8dc --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/TimeoutException.java @@ -0,0 +1,22 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote; + +import java.io.IOException; + +/** + * Exception to signal that a remote execution has timed out. + */ +class TimeoutException extends IOException { +} diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index b8e4f21ec0..ea1ac727df 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -383,6 +383,40 @@ public class RemoteSpawnRunnerTest { verify(executor).executeRemotely(any(ExecuteRequest.class)); } + @Test + public void testRemoteExecutionTimeout() throws Exception { + // If remote execution times out the SpawnResult status should be TIMEOUT. + + RemoteOptions options = Options.getDefaults(RemoteOptions.class); + options.remoteLocalFallback = false; + + RemoteSpawnRunner runner = + new RemoteSpawnRunner(execRoot, options, localRunner, true, /*cmdlineReporter=*/null, + cache, executor); + + ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); + when(cache.getCachedActionResult(any(ActionKey.class))).thenReturn(null); + when(executor.executeRemotely(any(ExecuteRequest.class))).thenThrow(new TimeoutException()); + + Spawn spawn = + new SimpleSpawn( + new FakeOwner("foo", "bar"), + /*arguments=*/ ImmutableList.of(), + /*environment=*/ ImmutableMap.of(), + /*executionInfo=*/ ImmutableMap.of(), + /*inputs=*/ ImmutableList.of(), + /*outputs=*/ ImmutableList.<ActionInput>of(), + ResourceSet.ZERO); + + SpawnExecutionPolicy policy = new FakeSpawnExecutionPolicy(spawn); + + SpawnResult res = runner.exec(spawn, policy); + assertThat(res.status()).isEqualTo(Status.TIMEOUT); + + verify(executor).executeRemotely(any(ExecuteRequest.class)); + verify(cache, never()).download(eq(cachedResult), eq(execRoot), any(FileOutErr.class)); + } + // TODO(buchgr): Extract a common class to be used for testing. class FakeSpawnExecutionPolicy implements SpawnExecutionPolicy { diff --git a/src/test/shell/bazel/remote_execution_test.sh b/src/test/shell/bazel/remote_execution_test.sh index eaa097e46a..53463777b1 100755 --- a/src/test/shell/bazel/remote_execution_test.sh +++ b/src/test/shell/bazel/remote_execution_test.sh @@ -355,6 +355,31 @@ EOF || fail "Failed to run //a:test with remote execution" } +function test_timeout() { + mkdir -p a + cat > a/BUILD <<'EOF' +sh_test( + name = "sleep", + timeout = "short", + srcs = ["sleep.sh"], +) +EOF + + cat > a/sleep.sh <<'EOF' +#!/bin/bash +sleep 2 +EOF + chmod +x a/sleep.sh + bazel --host_jvm_args=-Dbazel.DigestFunction=SHA1 test \ + --spawn_strategy=remote \ + --remote_executor=localhost:${worker_port} \ + --test_output=errors \ + --test_timeout=1,1,1,1 \ + //a:sleep >& $TEST_log \ + && fail "Test failure (timeout) expected" || true + expect_log "TIMEOUT" +} + # TODO(alpha): Add a test that fails remote execution when remote worker # supports sandbox. |