Skip to content

Commit

Permalink
Add worker_key_hash field to WorkerMetrics in the BEP.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 531435217
Change-Id: I9138df8cc660122c4162f584b3d9d1a3b194a20d
  • Loading branch information
wilwell authored and Copybara-Service committed May 12, 2023
1 parent 6c6c196 commit 58eeea1
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 41 deletions.
Expand Up @@ -1065,6 +1065,9 @@ message BuildMetrics {
bool is_sandbox = 5;
// Shows is worker stats measured at the end of invocation.
bool is_measurable = 6;
// Hash value of worker key. Needed to distinguish worker pools with same
// menmonic but with different worker keys.
int64 worker_key_hash = 9;

// Information collected from worker at some point.
message WorkerStats {
Expand Down
Expand Up @@ -103,11 +103,12 @@ public Worker create(WorkerKey key) throws IOException {

String msg =
String.format(
"Created new %s %s %s (id %d), logging to %s",
"Created new %s %s %s (id %d, key hash %d), logging to %s",
key.isSandboxed() ? "sandboxed" : "non-sandboxed",
key.getMnemonic(),
workTypeName,
workerId,
key.hashCode(),
worker.getLogFile());
WorkerLoggingHelper.logMessage(reporter, WorkerLoggingHelper.LogLevel.INFO, msg);
if (eventBus != null) {
Expand Down Expand Up @@ -142,7 +143,8 @@ public void destroyObject(WorkerKey key, PooledObject<Worker> p) {
int workerId = p.getObject().getWorkerId();
String msg =
String.format(
"Destroying %s %s (id %d)", key.getMnemonic(), key.getWorkerTypeName(), workerId);
"Destroying %s %s (id %d, key hash %d)",
key.getMnemonic(), key.getWorkerTypeName(), workerId, key.hashCode());
WorkerLoggingHelper.logMessage(reporter, WorkerLoggingHelper.LogLevel.INFO, msg);
p.getObject().destroy();
if (eventBus != null) {
Expand Down
Expand Up @@ -131,7 +131,7 @@ void killLargeWorkers(ImmutableList<WorkerMetric> workerMetrics, int limitMb) {
if (eventBus != null) {
eventBus.post(
new WorkerEvictedEvent(
l.getWorkerProperties().getWorkerKeylHash(),
l.getWorkerProperties().getWorkerKeyHash(),
l.getWorkerProperties().getMnemonic()));
}
}
Expand Down Expand Up @@ -216,7 +216,7 @@ void evictWorkers(ImmutableList<WorkerMetric> workerMetrics) throws InterruptedE
for (Integer workerId : properties.getWorkerIds()) {
if (evictedWorkers.contains(workerId)) {
eventBus.post(
new WorkerEvictedEvent(properties.getWorkerKeylHash(), properties.getMnemonic()));
new WorkerEvictedEvent(properties.getWorkerKeyHash(), properties.getMnemonic()));
}
}
}
Expand Down
Expand Up @@ -64,7 +64,7 @@ public abstract static class WorkerProperties {

public abstract boolean isSandboxed();

public abstract int getWorkerKeylHash();
public abstract int getWorkerKeyHash();

public static WorkerProperties create(
ImmutableList<Integer> workerIds,
Expand All @@ -89,7 +89,8 @@ public WorkerMetrics toProto() {
.setMnemonic(workerProperties.getMnemonic())
.setIsSandbox(workerProperties.isSandboxed())
.setIsMultiplex(workerProperties.isMultiplex())
.setIsMeasurable(isMeasurable());
.setIsMeasurable(isMeasurable())
.setWorkerKeyHash(workerProperties.getWorkerKeyHash());

if (workerStat != null) {
WorkerStats stats =
Expand Down
Expand Up @@ -71,15 +71,15 @@ public void testRegisterWorker_insertDifferent() throws Exception {
props1.getMnemonic(),
props1.isMultiplex(),
props1.isSandboxed(),
props1.getWorkerKeylHash());
props1.getWorkerKeyHash());
assertThat(spyCollector.getProcessIdToWorkerProperties()).hasSize(1);
spyCollector.registerWorker(
props2.getWorkerIds().get(0),
props2.getProcessId(),
props2.getMnemonic(),
props2.isMultiplex(),
props2.isSandboxed(),
props2.getWorkerKeylHash());
props2.getWorkerKeyHash());
assertThat(spyCollector.getProcessIdToWorkerProperties()).hasSize(2);
assertThat(spyCollector.getProcessIdToWorkerProperties()).isEqualTo(map);
}
Expand Down Expand Up @@ -240,7 +240,7 @@ private static void registerWorker(
props.getMnemonic(),
props.isMultiplex(),
props.isSandboxed(),
props.getWorkerKeylHash());
props.getWorkerKeyHash());
}

private static class ManualClock implements Clock {
Expand Down
8 changes: 4 additions & 4 deletions src/test/shell/bazel/android/desugarer_integration_test.sh
Expand Up @@ -127,8 +127,8 @@ function test_java_8_android_binary_worker_strategy() {
assert_build //java/bazel:bin \
--persistent_android_dex_desugar \
--worker_verbose &> $TEST_log
expect_log "Created new non-sandboxed Desugar worker (id [0-9]\+)"
expect_log "Created new non-sandboxed DexBuilder worker (id [0-9]\+)"
expect_log "Created new non-sandboxed Desugar worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed DexBuilder worker (id [0-9]\+, key hash -\?[0-9]\+)"
}

function test_java_8_android_binary_multiplex_worker_strategy() {
Expand All @@ -140,8 +140,8 @@ function test_java_8_android_binary_multiplex_worker_strategy() {
--experimental_worker_multiplex \
--persistent_multiplex_android_dex_desugar \
--worker_verbose &> $TEST_log
expect_log "Created new non-sandboxed Desugar multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed DexBuilder multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed Desugar multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed DexBuilder multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
}

run_suite "Android desugarer integration tests"
Expand Up @@ -113,11 +113,11 @@ function test_persistent_resource_processor() {

assert_build //java/bazel:bin --persistent_android_resource_processor \
--worker_verbose &> $TEST_log
expect_log "Created new non-sandboxed AndroidResourceParser worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidResourceCompiler worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidCompiledResourceMerger worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidAapt2 worker (id [0-9]\+)"
expect_log "Created new non-sandboxed ManifestMerger worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidResourceParser worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed AndroidResourceCompiler worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed AndroidCompiledResourceMerger worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed AndroidAapt2 worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed ManifestMerger worker (id [0-9]\+, key hash -\?[0-9]\+)"
}

function test_persistent_multiplex_resource_processor() {
Expand All @@ -129,11 +129,11 @@ function test_persistent_multiplex_resource_processor() {
assert_build //java/bazel:bin --experimental_worker_multiplex \
--persistent_multiplex_android_tools \
--worker_verbose &> $TEST_log
expect_log "Created new non-sandboxed AndroidResourceParser multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidResourceCompiler multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidCompiledResourceMerger multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidAapt2 multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed ManifestMerger multiplex-worker (id [0-9]\+)"
expect_log "Created new non-sandboxed AndroidResourceParser multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed AndroidResourceCompiler multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed AndroidCompiledResourceMerger multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed AndroidAapt2 multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Created new non-sandboxed ManifestMerger multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
}

run_suite "Resource processing integration tests"
14 changes: 7 additions & 7 deletions src/test/shell/integration/bazel_worker_multiplexer_test.sh
Expand Up @@ -379,8 +379,8 @@ EOF

bazel build --worker_quit_after_build :hello_world_1 &> $TEST_log \
|| fail "build failed"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+)"
expect_log "Destroying Work multiplex-worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Destroying Work multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Build completed, shutting down worker pool..."
}

Expand All @@ -397,7 +397,7 @@ EOF
bazel build --worker_quit_after_build :hello_world_1 &> $TEST_log \
|| fail "build failed"

expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"

worker_log=$(egrep -o -- 'logging to .*/b(azel|laze)-workers/multiplex-worker-[0-9]-Work.log' "$TEST_log" | sed 's/^logging to //')

Expand Down Expand Up @@ -429,8 +429,8 @@ EOF
bazel build --worker_quit_after_build :hello_world &> $TEST_log \
|| fail "build failed"

expect_not_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+)"
expect_not_log "Destroying Work multiplex-worker (id [0-9]\+)"
expect_not_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_not_log "Destroying Work multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"

# WorkerSpawnStrategy falls back to standalone strategy, so we still expect the output to be generated.
[ -e "$BINS/hello_world.out" ] \
Expand Down Expand Up @@ -469,12 +469,12 @@ EOF
bazel build :hello_clean &> $TEST_log \
|| fail "build failed"
assert_equals "hello clean" "$(cat $BINS/hello_clean.out)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"

bazel clean &> $TEST_log \
|| fail "clean failed"
expect_log "Clean command is running, shutting down worker pool..."
expect_log "Destroying Work multiplex-worker (id [0-9]\+)"
expect_log "Destroying Work multiplex-worker (id [0-9]\+, key hash -\?[0-9]\+)"
}

function test_crashed_worker_causes_log_dump() {
Expand Down
22 changes: 11 additions & 11 deletions src/test/shell/integration/bazel_worker_test.sh
Expand Up @@ -95,7 +95,7 @@ function test_compiles_hello_library_using_persistent_javac() {

bazel build java/main:main &> "$TEST_log" \
|| fail "build failed"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Javac worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Javac worker (id [0-9]\+, key hash -\?[0-9]\+)"
$BINS/java/main/main | grep -q "Hello, Library!;Hello, World!" \
|| fail "comparison failed"
}
Expand All @@ -107,7 +107,7 @@ function test_compiles_hello_library_using_persistent_javac_sibling_layout() {
--experimental_sibling_repository_layout java/main:main \
--worker_max_instances=Javac=1 \
&> "$TEST_log" || fail "build failed"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Javac worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Javac worker (id [0-9]\+, key hash -\?[0-9]\+)"
$BINS/java/main/main | grep -q "Hello, Library!;Hello, World!" \
|| fail "comparison failed"
}
Expand Down Expand Up @@ -382,7 +382,7 @@ EOF
bazel build --worker_verbose :hello_world_2 &> "$TEST_log" \
|| fail "build failed"

expect_log "Work worker (id [0-9]\+) has unexpectedly died with exit code 0."
expect_log "Work worker (id [0-9]\+, key hash -\?[0-9]\+) has unexpectedly died with exit code 0."
}

function test_build_fails_if_worker_dies_during_action() {
Expand Down Expand Up @@ -572,8 +572,8 @@ EOF

bazel build --worker_quit_after_build :hello_world_1 &> "$TEST_log" \
|| fail "build failed"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+)"
expect_log "Destroying Work worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Destroying Work worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_log "Build completed, shutting down worker pool..."
}

Expand All @@ -591,7 +591,7 @@ EOF
bazel build --worker_quit_after_build :hello_world_1 &> "$TEST_log" \
|| fail "build failed"

expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+, key hash -\?[0-9]\+)"

worker_log=$(egrep -o -- 'logging to .*/b(azel|laze)-workers/worker-[0-9]-Work.log' "$TEST_log" | sed 's/^logging to //')

Expand Down Expand Up @@ -651,8 +651,8 @@ EOF
bazel build --worker_quit_after_build :hello_world &> "$TEST_log" \
|| fail "build failed"

expect_not_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+)"
expect_not_log "Destroying Work worker (id [0-9]\+)"
expect_not_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+, key hash -\?[0-9]\+)"
expect_not_log "Destroying Work worker (id [0-9]\+, key hash -\?[0-9]\+)"

# WorkerSpawnStrategy falls back to standalone strategy, so we still expect the output to be generated.
[ -e "$BINS/hello_world.out" ] \
Expand Down Expand Up @@ -693,12 +693,12 @@ EOF
bazel build :hello_clean &> "$TEST_log" \
|| fail "build failed"
assert_equals "hello clean" "$(cat $BINS/hello_clean.out)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+, key hash -\?[0-9]\+)"

bazel clean &> "$TEST_log" \
|| fail "clean failed"
expect_log "Clean command is running, shutting down worker pool..."
expect_log "Destroying Work worker (id [0-9]\+)"
expect_log "Destroying Work worker (id [0-9]\+, key hash -\?[0-9]\+)"
}

function test_crashed_worker_causes_log_dump() {
Expand Down Expand Up @@ -775,7 +775,7 @@ EOF
--experimental_collect_worker_data_in_profiler \
:hello_world_1 &> "$TEST_log" \
|| fail "build failed"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+)"
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Work worker (id [0-9]\+, key hash -\?[0-9]\+)"
# Now see that we have metrics in the build event log.
mv "${TEST_log}".build.json "${TEST_log}"
expect_log "mnemonic: \"Work\""
Expand Down

0 comments on commit 58eeea1

Please sign in to comment.