Skip to content

Commit

Permalink
Don't set requestId on non-multiplex requests.
Browse files Browse the repository at this point in the history
RELNOTES: n/a
PiperOrigin-RevId: 343728512
  • Loading branch information
larsrc-google authored and copybara-github committed Nov 22, 2020
1 parent 817e220 commit 0881c80
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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();
}

/**
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
}
Expand All @@ -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);
Expand Down
33 changes: 33 additions & 0 deletions src/test/shell/integration/bazel_worker_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0881c80

Please sign in to comment.