Skip to content

fix(worker): emit activation status events - #57

Draft
DavidBellamy wants to merge 1 commit into
prodfrom
fix/56-worker-capacity-status-events
Draft

fix(worker): emit activation status events#57
DavidBellamy wants to merge 1 commit into
prodfrom
fix/56-worker-capacity-status-events

Conversation

@DavidBellamy

Copy link
Copy Markdown

Description

Problem

Worker registration activation changes each shared worker object from Pending to Ready directly. That bypasses the registry event stream used by aggregate worker capacity and partition admission. When two registration workflows overlap, the capacity subscribers can snapshot after only the first worker is ready and remain permanently stale even though /workers later reports both workers healthy and ready.

This was reproduced with two workers reporting 38 slots each: depending on workflow scheduling, the gateway reported either the correct aggregate 76 or a stale 38. The stale value persisted through both dynamic IGW registration and regular static worker startup.

Solution

Activate registered workers through WorkerRegistry::transition_status rather than mutating the worker object directly. The registry transition emits StatusChanged { Pending, Ready }, which deterministically wakes both capacity consumers.

Activation remains idempotent for an already-ready worker and fails closed if a worker disappears between workflow registration and activation.

Closes #56.

Changes

  • Route workflow activation through the registry lifecycle transition API.
  • Distinguish an already-ready worker from a worker removed during activation.
  • Add a lifecycle-event regression test for pending and already-ready workers.
  • Add a deterministic aggregate-capacity regression that starts from a stale legacy fallback and requires two 38-slot activations to produce 76.

Test Plan

Local verification on exact prod base f6163ad9:

  • cargo test --offline -p smg --lib workflow::steps::shared::activate::tests (2 passed)
  • cargo test --offline -p smg --lib (1,471 passed, 5 ignored)
  • cargo clippy --offline -p smg --lib (passes; one unchanged warning in crates/protocols/src/chat.rs:642)
  • cargo +nightly fmt --all -- --check
  • git diff --check

The deterministic capacity test registers two pending workers reporting 38 slots, waits until the capacity tracker has consumed their registration events while they are still pending, activates them, and then requires CapacitySource::WorkerReported with aggregate capacity 76.

Repository-wide clippy with -D warnings remains blocked on the unchanged base by the existing #[allow(deprecated)] warning in crates/protocols/src/chat.rs:642; this one-file diff introduces no clippy warning.

Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes (blocked by the unchanged warning documented above)
  • (Optional) Documentation updated
  • (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Signed-off-by: David <12414531+DavidBellamy@users.noreply.github.com>
@DavidBellamy

Copy link
Copy Markdown
Author

M1 runtime validation from the corrected owner-pressure A/B (job 1191549)

This PR was common to both arms. Two independently registered Kimi K3 workers each reported 38 running-request slots. In both the control and treatment gateway processes, runtime telemetry advanced deterministically from one worker / 38 slots to two workers / 76 aggregate slots and remained complete for all 228 workload decisions.

The treatment additionally carried #55; the control carried only this PR. Both runtime-loaded compiled-binary hashes and embedded provenance were checked before traffic, so this validation does not rely on the earlier stale build artifact.

Both arms completed 228/228 turns with zero terminal failures, 429 retries, retractions, or CUDA OOMs. This is positive runtime evidence for the activation-event fix, but the PR remains draft pending code-owner review. Production Comet was not changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant