Don't set requestId on non-multiplex requests.

RELNOTES: n/a
PiperOrigin-RevId: 343728512

Author: larsrc-google

src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java

@@ -293,7 +293,8 @@ private WorkRequest createWorkRequest(
       Spawn spawn,
       SpawnExecutionContext context,
       List<String> flagfiles,
-      MetadataProvider inputFileCache)
+      MetadataProvider inputFileCache,
+      WorkerKey key)
       throws IOException {
     WorkRequest.Builder requestBuilder = WorkRequest.newBuilder();
     for (String flagfile : flagfiles) {
@@ -318,7 +319,10 @@ private WorkRequest createWorkRequest(
           .setDigest(digest)
           .build();
     }
-    return requestBuilder.setRequestId(requestIdCounter.getAndIncrement()).build();
+    if (key.getProxied()) {
+      requestBuilder.setRequestId(requestIdCounter.getAndIncrement());
+    }
+    return requestBuilder.build();
   }
 
   /**
@@ -420,7 +424,7 @@ WorkResponse execInWorker(
       try {
         worker = workers.borrowObject(key);
         worker.setReporter(workerOptions.workerVerbose ? reporter : null);
-        request = createWorkRequest(spawn, context, flagFiles, inputFileCache);
+        request = createWorkRequest(spawn, context, flagFiles, inputFileCache, key);
       } catch (IOException e) {
         String message = "IOException while borrowing a worker from the pool:";
         throw createUserExecException(e, message, Code.BORROW_FAILURE);

src/test/java/com/google/devtools/build/lib/worker/ExampleWorker.java

@@ -55,6 +55,9 @@ public class ExampleWorker {
   // Keep state across multiple builds.
   static final LinkedHashMap<String, String> inputs = new LinkedHashMap<>();
 
+  // Contains the request currently being worked on.
+  private static WorkRequest currentRequest;
+
   public static void main(String[] args) throws Exception {
     if (ImmutableSet.copyOf(args).contains("--persistent_worker")) {
       OptionsParser parser =
@@ -92,6 +95,8 @@ private static void runPersistentWorker(ExampleWorkerOptions workerOptions) thro
           break;
         }
 
+        currentRequest = request;
+
         inputs.clear();
         for (Input input : request.getInputsList()) {
           inputs.put(input.getPath(), input.getDigest().toStringUtf8());
@@ -127,6 +132,7 @@ private static void runPersistentWorker(ExampleWorkerOptions workerOptions) thro
         } finally {
           System.setOut(originalStdOut);
           System.setErr(originalStdErr);
+          currentRequest = null;
         }
 
         if (workerOptions.exitDuring > 0 && workUnitCounter > workerOptions.exitDuring) {
@@ -198,6 +204,10 @@ private static void processRequest(List<String> args) throws Exception {
       }
     }
 
+    if (options.printRequests) {
+      outputs.add("REQUEST: " + currentRequest);
+    }
+
     if (options.printEnv) {
       for (Map.Entry<String, String> entry : System.getenv().entrySet()) {
         outputs.add(entry.getKey() + "=" + entry.getValue());

src/test/java/com/google/devtools/build/lib/worker/ExampleWorkerOptions.java

@@ -66,12 +66,19 @@ public static class ExampleWorkOptions extends OptionsBase {
     public boolean writeCounter;
 
     @Option(
-      name = "print_inputs",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.NO_OP},
-      defaultValue = "false",
-      help = "Writes a list of input files and their digests."
-    )
+        name = "print_requests",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.NO_OP},
+        defaultValue = "false",
+        help = "Prints out all requests.")
+    public boolean printRequests;
+
+    @Option(
+        name = "print_inputs",
+        documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+        effectTags = {OptionEffectTag.NO_OP},
+        defaultValue = "false",
+        help = "Writes a list of input files and their digests.")
     public boolean printInputs;
 
     @Option(

src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java

@@ -110,9 +110,8 @@ public void testExecInWorker_happyPath() throws ExecException, InterruptedExcept
     WorkerKey key = createWorkerKey(fs, "mnem", false);
     Path logFile = fs.getPath("/worker.log");
     when(worker.getLogFile()).thenReturn(logFile);
-    when(worker.getResponse(1))
-        .thenReturn(
-            WorkResponse.newBuilder().setExitCode(0).setOutput("out").setRequestId(1).build());
+    when(worker.getResponse(0))
+        .thenReturn(WorkResponse.newBuilder().setExitCode(0).setOutput("out").build());
     WorkResponse response =
         runner.execInWorker(
             spawn,
@@ -126,7 +125,7 @@ public void testExecInWorker_happyPath() throws ExecException, InterruptedExcept
 
     assertThat(response).isNotNull();
     assertThat(response.getExitCode()).isEqualTo(0);
-    assertThat(response.getRequestId()).isEqualTo(1);
+    assertThat(response.getRequestId()).isEqualTo(0);
     assertThat(response.getOutput()).isEqualTo("out");
     assertThat(logFile.exists()).isFalse();
   }
@@ -149,7 +148,7 @@ private void assertRecordedResponsethrowsException(String recordedResponse, Stri
     WorkerKey key = createWorkerKey(fs, "mnem", false);
     Path logFile = fs.getPath("/worker.log");
     when(worker.getLogFile()).thenReturn(logFile);
-    when(worker.getResponse(1)).thenThrow(new IOException("Bad protobuf"));
+    when(worker.getResponse(0)).thenThrow(new IOException("Bad protobuf"));
     when(worker.getRecordingStreamMessage()).thenReturn(recordedResponse);
     String workerLog = "Log from worker\n";
     FileSystemUtils.writeIsoLatin1(logFile, workerLog);

src/test/shell/integration/bazel_worker_test.sh

@@ -206,6 +206,39 @@ EOF
   assert_equals "HELLO WORLD" "$(cat $BINS/hello_world_uppercase.out)"
 }
 
+function test_worker_requests() {
+  prepare_example_worker
+  cat >>BUILD <<EOF
+work(
+  name = "hello_world",
+  worker = ":worker",
+  worker_args = ["--worker_protocol=${WORKER_PROTOCOL}"],
+  args = ["hello world", "--print_requests"],
+)
+
+work(
+  name = "hello_world_uppercase",
+  worker = ":worker",
+  worker_args = ["--worker_protocol=${WORKER_PROTOCOL}"],
+  args = ["--uppercase", "hello world", "--print_requests"],
+)
+EOF
+
+  bazel build  :hello_world &> $TEST_log \
+    || fail "build failed"
+  assert_contains "hello world" "$BINS/hello_world.out"
+  assert_contains "arguments: \"hello world\"" "$BINS/hello_world.out"
+  assert_contains "path:.*hello_world_worker_input" "$BINS/hello_world.out"
+  assert_not_contains "request_id" "$BINS/hello_world.out"
+
+  bazel build  :hello_world_uppercase &> $TEST_log \
+    || fail "build failed"
+  assert_contains "HELLO WORLD" "$BINS/hello_world_uppercase.out"
+  assert_contains "arguments: \"hello world\"" "$BINS/hello_world_uppercase.out"
+  assert_contains "path:.*hello_world_uppercase_worker_input" "$BINS/hello_world_uppercase.out"
+  assert_not_contains "request_id" "$BINS/hello_world_uppercase.out"
+}
+
 function test_shared_worker() {
   prepare_example_worker
   cat >>BUILD <<EOF