Skip to content

Commit 0881c80

Browse files
larsrc-googlecopybara-github
larsrc-google
authored and
copybara-github
committed
Don't set requestId on non-multiplex requests.
RELNOTES: n/a PiperOrigin-RevId: 343728512
1 parent 817e220 commit 0881c80

5 files changed

Lines changed: 67 additions & 14 deletions

File tree

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,8 @@ private WorkRequest createWorkRequest(
293293
Spawn spawn,
294294
SpawnExecutionContext context,
295295
List<String> flagfiles,
296-
MetadataProvider inputFileCache)
296+
MetadataProvider inputFileCache,
297+
WorkerKey key)
297298
throws IOException {
298299
WorkRequest.Builder requestBuilder = WorkRequest.newBuilder();
299300
for (String flagfile : flagfiles) {
@@ -318,7 +319,10 @@ private WorkRequest createWorkRequest(
318319
.setDigest(digest)
319320
.build();
320321
}
321-
return requestBuilder.setRequestId(requestIdCounter.getAndIncrement()).build();
322+
if (key.getProxied()) {
323+
requestBuilder.setRequestId(requestIdCounter.getAndIncrement());
324+
}
325+
return requestBuilder.build();
322326
}
323327

324328
/**
@@ -420,7 +424,7 @@ WorkResponse execInWorker(
420424
try {
421425
worker = workers.borrowObject(key);
422426
worker.setReporter(workerOptions.workerVerbose ? reporter : null);
423-
request = createWorkRequest(spawn, context, flagFiles, inputFileCache);
427+
request = createWorkRequest(spawn, context, flagFiles, inputFileCache, key);
424428
} catch (IOException e) {
425429
String message = "IOException while borrowing a worker from the pool:";
426430
throw createUserExecException(e, message, Code.BORROW_FAILURE);

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ public class ExampleWorker {
5555
// Keep state across multiple builds.
5656
static final LinkedHashMap<String, String> inputs = new LinkedHashMap<>();
5757

58+
// Contains the request currently being worked on.
59+
private static WorkRequest currentRequest;
60+
5861
public static void main(String[] args) throws Exception {
5962
if (ImmutableSet.copyOf(args).contains("--persistent_worker")) {
6063
OptionsParser parser =
@@ -92,6 +95,8 @@ private static void runPersistentWorker(ExampleWorkerOptions workerOptions) thro
9295
break;
9396
}
9497

98+
currentRequest = request;
99+
95100
inputs.clear();
96101
for (Input input : request.getInputsList()) {
97102
inputs.put(input.getPath(), input.getDigest().toStringUtf8());
@@ -127,6 +132,7 @@ private static void runPersistentWorker(ExampleWorkerOptions workerOptions) thro
127132
} finally {
128133
System.setOut(originalStdOut);
129134
System.setErr(originalStdErr);
135+
currentRequest = null;
130136
}
131137

132138
if (workerOptions.exitDuring > 0 && workUnitCounter > workerOptions.exitDuring) {
@@ -198,6 +204,10 @@ private static void processRequest(List<String> args) throws Exception {
198204
}
199205
}
200206

207+
if (options.printRequests) {
208+
outputs.add("REQUEST: " + currentRequest);
209+
}
210+
201211
if (options.printEnv) {
202212
for (Map.Entry<String, String> entry : System.getenv().entrySet()) {
203213
outputs.add(entry.getKey() + "=" + entry.getValue());

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,19 @@ public static class ExampleWorkOptions extends OptionsBase {
6666
public boolean writeCounter;
6767

6868
@Option(
69-
name = "print_inputs",
70-
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
71-
effectTags = {OptionEffectTag.NO_OP},
72-
defaultValue = "false",
73-
help = "Writes a list of input files and their digests."
74-
)
69+
name = "print_requests",
70+
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
71+
effectTags = {OptionEffectTag.NO_OP},
72+
defaultValue = "false",
73+
help = "Prints out all requests.")
74+
public boolean printRequests;
75+
76+
@Option(
77+
name = "print_inputs",
78+
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
79+
effectTags = {OptionEffectTag.NO_OP},
80+
defaultValue = "false",
81+
help = "Writes a list of input files and their digests.")
7582
public boolean printInputs;
7683

7784
@Option(

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,8 @@ public void testExecInWorker_happyPath() throws ExecException, InterruptedExcept
110110
WorkerKey key = createWorkerKey(fs, "mnem", false);
111111
Path logFile = fs.getPath("/worker.log");
112112
when(worker.getLogFile()).thenReturn(logFile);
113-
when(worker.getResponse(1))
114-
.thenReturn(
115-
WorkResponse.newBuilder().setExitCode(0).setOutput("out").setRequestId(1).build());
113+
when(worker.getResponse(0))
114+
.thenReturn(WorkResponse.newBuilder().setExitCode(0).setOutput("out").build());
116115
WorkResponse response =
117116
runner.execInWorker(
118117
spawn,
@@ -126,7 +125,7 @@ public void testExecInWorker_happyPath() throws ExecException, InterruptedExcept
126125

127126
assertThat(response).isNotNull();
128127
assertThat(response.getExitCode()).isEqualTo(0);
129-
assertThat(response.getRequestId()).isEqualTo(1);
128+
assertThat(response.getRequestId()).isEqualTo(0);
130129
assertThat(response.getOutput()).isEqualTo("out");
131130
assertThat(logFile.exists()).isFalse();
132131
}
@@ -149,7 +148,7 @@ private void assertRecordedResponsethrowsException(String recordedResponse, Stri
149148
WorkerKey key = createWorkerKey(fs, "mnem", false);
150149
Path logFile = fs.getPath("/worker.log");
151150
when(worker.getLogFile()).thenReturn(logFile);
152-
when(worker.getResponse(1)).thenThrow(new IOException("Bad protobuf"));
151+
when(worker.getResponse(0)).thenThrow(new IOException("Bad protobuf"));
153152
when(worker.getRecordingStreamMessage()).thenReturn(recordedResponse);
154153
String workerLog = "Log from worker\n";
155154
FileSystemUtils.writeIsoLatin1(logFile, workerLog);

‎src/test/shell/integration/bazel_worker_test.sh‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,39 @@ EOF
206206
assert_equals "HELLO WORLD" "$(cat $BINS/hello_world_uppercase.out)"
207207
}
208208

209+
function test_worker_requests() {
210+
prepare_example_worker
211+
cat >>BUILD <<EOF
212+
work(
213+
name = "hello_world",
214+
worker = ":worker",
215+
worker_args = ["--worker_protocol=${WORKER_PROTOCOL}"],
216+
args = ["hello world", "--print_requests"],
217+
)
218+
219+
work(
220+
name = "hello_world_uppercase",
221+
worker = ":worker",
222+
worker_args = ["--worker_protocol=${WORKER_PROTOCOL}"],
223+
args = ["--uppercase", "hello world", "--print_requests"],
224+
)
225+
EOF
226+
227+
bazel build :hello_world &> $TEST_log \
228+
|| fail "build failed"
229+
assert_contains "hello world" "$BINS/hello_world.out"
230+
assert_contains "arguments: \"hello world\"" "$BINS/hello_world.out"
231+
assert_contains "path:.*hello_world_worker_input" "$BINS/hello_world.out"
232+
assert_not_contains "request_id" "$BINS/hello_world.out"
233+
234+
bazel build :hello_world_uppercase &> $TEST_log \
235+
|| fail "build failed"
236+
assert_contains "HELLO WORLD" "$BINS/hello_world_uppercase.out"
237+
assert_contains "arguments: \"hello world\"" "$BINS/hello_world_uppercase.out"
238+
assert_contains "path:.*hello_world_uppercase_worker_input" "$BINS/hello_world_uppercase.out"
239+
assert_not_contains "request_id" "$BINS/hello_world_uppercase.out"
240+
}
241+
209242
function test_shared_worker() {
210243
prepare_example_worker
211244
cat >>BUILD <<EOF

0 commit comments

Comments
 (0)