Skip to content

fix(credit): wait for worker registration instead of raising in StickyCreditRouter - #23

Open
rebel-jinmoo wants to merge 1 commit into
SemiAnalysisAI:cquil11/aiperf-agentx-v1.0from
rebel-jinmoo:fix/credit-router-worker-registration-race
Open

fix(credit): wait for worker registration instead of raising in StickyCreditRouter#23
rebel-jinmoo wants to merge 1 commit into
SemiAnalysisAI:cquil11/aiperf-agentx-v1.0from
rebel-jinmoo:fix/credit-router-worker-registration-race

Conversation

@rebel-jinmoo

Copy link
Copy Markdown

Phase-start credit dispatch can outrun worker registration. When it does, the run hangs forever and no request ever reaches the server.

The race

  1. Workers announce readiness asynchronously over ZMQ. Measured skew between the first credit dispatch and the first WorkerReady is 0.4–0.9s, widening with concurrency (more workers to register).
  2. AgenticReplayTiming._execute_warmup dispatches the phase-start burst with sequential awaits.
  3. CreditIssuer._issue_credit_internal calls increment_sent() before send_credit(), so a failed send is still counted as sent.
  4. StickyCreditRouter.send_credit raises RuntimeError("No workers available for routing") when _workers is empty. That aborts the sequential loop — the remaining credits are never attempted.
  5. The exception is swallowed at the task boundary, and in-flight credits have no timeout, so the run stalls permanently.

Instrumented evidence:

[DIAG] send_credit id=0 workers=0
dispatch 15:10:02.528   vs   first WorkerReady 15:10:02.957   (429 ms later)

Symptom

sent=1 returned=0 in_flight=1 errors=0, with zero server-side evidence — gateway/router counters 0, engine num_requests_running 0, KV-transfer counters 0, and a fresh curl to the same endpoint returns 200 while the run is stuck. Because the request never left the client, there is nothing to find on the server, which makes this very hard to diagnose. With the default 6-hour --request-timeout-seconds it is effectively a deadlock.

Fix

When no worker has registered yet, wait for registration (60s cap, 50ms poll) instead of raising. The raise is kept as the timeout path, so a genuinely worker-less run still fails loudly.

Verification

1P1D vLLM PD-disaggregated stack, MiniMax-M2.5, 145K context, --scenario inferencex-agentx-mvp --public-dataset semianalysis_cc_traces_weka_062126_256k, on 208125ac:

concurrency stack before after
8 warm hang, sent=1 returned=0 (2/2 runs) pass — 82 warmup requests, 0 errors, entered profiling
16 warm hang, sent=1/13 19/19 credits routed, in_flight=11
16 fresh stack pass — 110 credits routed, 90 warmup requests
8 fresh stack hang pass

The race triggers on every fresh stack (logged wait 0.55–0.85s) and the patch absorbs it each time, so it is structural rather than incidental.

Two related defects found while debugging (not fixed here)

Happy to send separate PRs if you want them addressed:

  1. ReplayBarrierCoordinator._releases_paused is never reset to False outside __init__. pause_releases() is called from _finish_accelerated_warmup, and complete() early-returns while paused — so deferred releases can be stranded once the barrier is active.
  2. ROUTER_MANDATORY is never set on the credit ROUTER socket, so unroutable messages are dropped silently instead of surfacing an error. Setting it would have turned this hang into an immediate, obvious failure.

…yCreditRouter

Phase-start credit dispatch can outrun worker registration, and when it does
the whole run hangs forever with no request ever reaching the server.

Sequence:

1. Workers announce readiness asynchronously over ZMQ. Measured skew between
   the first credit dispatch and the first `WorkerReady` is 0.4-0.9s, widening
   with concurrency (more workers to register).
2. `AgenticReplayTiming._execute_warmup` dispatches the phase-start burst with
   sequential `await`s.
3. `CreditIssuer._issue_credit_internal` calls `increment_sent()` *before*
   `send_credit()`, so a failed send is still counted as sent.
4. `StickyCreditRouter.send_credit` raises
   `RuntimeError("No workers available for routing")` when `_workers` is empty,
   which aborts the sequential loop — the remaining credits are never attempted.
5. The exception is swallowed at the task boundary and in-flight credits have no
   timeout, so the run stalls permanently.

Observed symptom: `sent=1 returned=0 in_flight=1 errors=0` with zero server-side
evidence (gateway/router counters 0, engine running 0, a fresh curl to the same
endpoint returns 200). With the default 6h `--request-timeout-seconds` this is
effectively a deadlock.

Fix: when no worker has registered yet, wait for registration (60s cap, 50ms
poll) instead of raising. The raise is kept as the timeout path.

Verified on a 1P1D vLLM PD-disaggregated stack (MiniMax-M2.5, 145K context,
`--scenario inferencex-agentx-mvp`, `--public-dataset
semianalysis_cc_traces_weka_062126_256k`):

| concurrency | stack       | before                        | after                          |
|-------------|-------------|-------------------------------|--------------------------------|
| 8           | warm        | hang, `sent=1 returned=0` 2/2 | pass, 82 warmup reqs, 0 errors |
| 16          | warm        | hang, `sent=1/13`             | 19/19 credits routed           |
| 16          | fresh stack | -                             | pass, 110 routed, 90 warmup    |
| 8           | fresh stack | hang                          | pass                           |

The race triggers on every fresh stack (logged wait 0.55-0.85s), so it is
structural rather than incidental.
@github-actions

Copy link
Copy Markdown

Try out this PR

Quick install:

pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@ad434d116e3d3435de28afd1c10303c308be94fe

Recommended with virtual environment (using uv):

uv venv --python 3.12 && source .venv/bin/activate
uv pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@ad434d116e3d3435de28afd1c10303c308be94fe

Last updated for commit: ad434d1Browse code

@rebel-jinmoo

Copy link
Copy Markdown
Author

The failing Validate PR title and add label check looks like a workflow permissions issue rather than a problem with this PR.

From the job log:

##[group]GITHUB_TOKEN Permissions
Contents: read
Metadata: read
Packages: read
##[endgroup]
...
##[error]Resource not accessible by integration

The title itself validates — fix is in the configured task_types, and fix(credit): ... matches the conventional-commits format. What fails is the add_label: 'true' step in .github/workflows/lint-pr-title.yaml, which needs write access to the PR. The job has no permissions:` block, so the token falls back to read-only and the label API call is rejected.

Adding this to the conventional-commits job should fix it for any PR (this would affect all external contributions, not just this one):

jobs:
  conventional-commits:
    permissions:
      pull-requests: write

Separately: only two checks have run on this PR (Validate PR title and add label, Comment with install instructions). The test workflows (pre-commit, run-unit-tests, run-integration-tests) have not been triggered — presumably awaiting maintainer approval for a first-time contributor. Could you kick those off? Happy to address anything they surface.

For context on the change itself: we hit this in a multi-turn agentic replay where the phase-start credit burst outran worker registration (0.4–0.9s skew). send_credit raising aborted the sequential warmup dispatch loop, leaving credits in-flight with no timeout, so the run hung indefinitely with zero server-side evidence. Waiting for registration instead of raising resolved it.

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