Allow timeout propagation for remote actions

quic-sbjorkle · quic-sbjorkle · commit eea808cf0585 · 2024-11-18T12:11:01.000+01:00
When building using remote execution,
action timeouts sometimes need to be adjusted
per action to ensure reasonable timeouts are set.

This change will allow better control over how
actions behave and encourage users to
optimize their actions since the global default timeout
does not have to be set to the absolute largest
action timeout in the graph.

Change-Id: I0ea63787ff7769e25dd597575cbfd7fd1050797e
diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
@@ -58,6 +58,7 @@ private static boolean legalExecInfoKeys(String tag) {
         || tag.startsWith("disable-")
         || tag.startsWith("cpu:")
         || tag.equals(ExecutionRequirements.LOCAL)
+        || tag.equals(ExecutionRequirements.TIMEOUT)
         || tag.equals(ExecutionRequirements.WORKER_KEY_MNEMONIC)
         || tag.startsWith("resources:");
   }
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
@@ -416,7 +416,7 @@ public void testCreateSpawnActionEnvAndExecInfo() throws Exception {
         "  inputs = ruleContext.files.srcs,",
         "  outputs = ruleContext.files.srcs,",
         "  env = {'a' : 'b'},",
-        "  execution_requirements = {'timeout' : '10', 'block-network' : 'foo'},",
+        "  execution_requirements = {'block-network' : 'foo'},",
         "  mnemonic = 'DummyMnemonic',",
         "  command = 'dummy_command',",
         "  progress_message = 'dummy_message')");
@@ -425,10 +425,31 @@ public void testCreateSpawnActionEnvAndExecInfo() throws Exception {
             Iterables.getOnlyElement(
                 ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
     assertThat(action.getIncompleteEnvironmentForTesting()).containsExactly("a", "b");
-    // We expect "timeout" to be filtered by TargetUtils.
     assertThat(action.getExecutionInfo()).containsExactly("block-network", "foo");
   }
 
+  @Test
+  public void testCreateSpawnActionEnvAndExecInfo_withTimeout() throws Exception {
+    StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");
+    setRuleContext(ruleContext);
+    ev.exec(
+        "ruleContext.actions.run_shell(",
+        "  inputs = ruleContext.files.srcs,",
+        "  outputs = ruleContext.files.srcs,",
+        "  execution_requirements = {",
+        "    'timeout': '42',",
+        "  },",
+        "  mnemonic = 'DummyMnemonic',",
+        "  command = 'dummy_command',",
+        "  progress_message = 'dummy_message')");
+    SpawnAction action =
+        (SpawnAction)
+            Iterables.getOnlyElement(
+                ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
+    assertThat(action.getExecutionInfo())
+        .containsExactly("timeout", "42");
+  }
+
   @Test
   public void testCreateSpawnActionEnvAndExecInfo_withWorkerKeyMnemonic() throws Exception {
     StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");

Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,7 @@ private static boolean legalExecInfoKeys(String tag) {`
`58`	`58`	`\|\| tag.startsWith("disable-")`
`59`	`59`	`\|\| tag.startsWith("cpu:")`
`60`	`60`	`\|\| tag.equals(ExecutionRequirements.LOCAL)`
	`61`	`+ \|\| tag.equals(ExecutionRequirements.TIMEOUT)`
`61`	`62`	`\|\| tag.equals(ExecutionRequirements.WORKER_KEY_MNEMONIC)`
`62`	`63`	`\|\| tag.startsWith("resources:");`
`63`	`64`	`}`