Skip to content

test(frontend): pin zmq event plane in realtime Omni mock worker - #16215

Open
glamr-agent wants to merge 4 commits into
ai-dynamo:mainfrom
glamr-agent:dyn-4614-flaky-websocket-audio-round-trip-timeout-38f845e15049
Open

glamr-agent wants to merge 4 commits into
ai-dynamo:mainfrom
glamr-agent:dyn-4614-flaky-websocket-audio-round-trip-timeout-38f845e15049

Conversation

@glamr-agent

@glamr-agent glamr-agent commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Closes DYN-4614.

Summary

tests/frontend/realtime_omni_mock_worker.py built DistributedRuntime without event_plane. When NATS_SERVER is set and no event plane is passed, the runtime connects to NATS and retries that first connect for up to 120 s before the worker registers its model. If the NATS server is slow or unreachable, test_websocket_audio_round_trip fails at its 60 s worker health check. The worker now passes event_plane="zmq", as realtime_echo_worker.py already does.

Details:

  • tests/frontend/realtime_omni_mock_worker.py: pass event_plane="zmq", with a short comment on why.
  • tests/frontend/test_realtime_omni_bridge.py: docstring only. It now says where each process sets the zmq event plane.

No production code, timeouts, or retries change.

Validation

With NATS_SERVER=nats://127.0.0.1:9, the test fails at setup on main (TIMEOUT: Check URL ... failed after 60.2s) and passes with this change. Reverting only the worker line fails again. With the change, 20/20 sequential runs passed.

Limitations: the output of the original CI failure was not available, so this change does not prove that NATS caused that incident. Heavy CPU starvation can still fail the 60 s health check on both main and this branch.

Where should the reviewer start?

tests/frontend/realtime_omni_mock_worker.py, the DistributedRuntime(...) call in main().

Related Issues

Summary by CodeRabbit

  • Tests
    • Realtime frontend integration tests now consistently use the ZMQ event transport for both processes. This avoids differences caused by ambient connection settings and makes test startup more predictable, including in environments where a NATS server is configured. The change affects test setup only; no end-user-facing behavior or product features are changed.

The mock worker built DistributedRuntime without event_plane, so an ambient
NATS_SERVER forced a NATS connection whose startup retry (up to 120s) blocks
registration past the 60s worker health check. Pass event_plane="zmq" like
realtime_echo_worker.py, and correct the fixture docstring.

Refs DYN-4614

Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
Signed-off-by: svc-glamr@nvidia.com <svc-glamr@nvidia.com>
@glamr-agent
glamr-agent requested a review from a team as a code owner October 12, 2026 20:21
@copy-pr-bot

copy-pr-bot Bot commented Oct 12, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@glamr-agent
glamr-agent temporarily deployed to external_collaborator October 12, 2026 20:21 — with GitHub Actions Inactive
@glamr-agent
glamr-agent temporarily deployed to external_collaborator October 12, 2026 20:21 — with GitHub Actions Inactive
@github-actions github-actions Bot added the test label Oct 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor

👋 Hi glamr-agent! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

@glamr-agent

Copy link
Copy Markdown
Collaborator Author
Automated evidence record — validation complete

Validation status: complete

Evidence summary: [3/3 validated]

AI review assessment (advisory only): sound

Validation result: pass. With NATS_SERVER set to an unreachable address, the test timed out on main during setup (TIMEOUT: Check URL ... 60.2s, with 33 worker NATS retry lines). It passed with the fix under the same condition. With only the worker change reverted, it failed again at the same health check. With the fix: 0/20 sequential runs, 0/8 concurrent runs, and 0/4 concurrent runs pinned to 2 CPUs failed. 37/37 neighbour tests and lint passed.

Limitations: 6 concurrent copies on 2 CPUs still fail the 60 s health check on both main and the fix. That is CPU starvation, not NATS. The exception text from the original CI attempt was not available, so the cause of the 2026-09-30 failure is not proven.

Evidence audit: complete [3/3 validated]. The command report comes from recorded runs.

Commands and results [3/3 validated]

Generated from the commands recorded during this run.

Check 1

Builds the changed Dynamo source and confirms that Python can import its compiled extension.

Result: Passed (exit 0)

Command:

Not shown because the exact command contained private run data.

Check 2

Checks the changed files with the repository's fast lint and formatting commands.

Result: Passed (exit 0)

Command:

pre-commit run --files tests/frontend/realtime_omni_mock_worker.py tests/frontend/test_realtime_omni_bridge.py --hook-stage manual

Check 3

Runs the relevant Python unit tests without requiring a GPU.

Result: Passed (exit 0)

Command:

/opt/dynamo/venv/bin/python -m pytest tests/frontend/test_realtime_python_bridge.py components/src/dynamo/vllm/tests/omni/test_omni_realtime_handler.py -v -p no:cacheprovider

@github-actions github-actions Bot added the external-contribution Pull request is from an external contributor label Oct 12, 2026
@glamr-agent

Copy link
Copy Markdown
Collaborator Author

No description provided.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: ai-dynamo/dynamo/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 5617f035-1266-4bdc-8abf-1027356ba951
📥 Commits

Reviewing files that changed from the base of the PR and between 8724f2d and a3595de.

📒 Files selected for processing (2)
  • tests/frontend/realtime_omni_mock_worker.py
  • tests/frontend/test_realtime_omni_bridge.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

Walkthrough

The mock worker now explicitly configures the ZMQ event plane. The fixture documentation specifies ZMQ configuration for both the frontend and worker.

Changes

Realtime bridge ZMQ configuration

Layer / File(s) Summary
Set the worker event plane
tests/frontend/realtime_omni_mock_worker.py, tests/frontend/test_realtime_omni_bridge.py
The worker sets event_plane="zmq". The fixture documentation specifies --event-plane zmq for the frontend and event_plane="zmq" for the worker.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to a3595

The mock worker and frontend use the same event plane, and ambient NATS configuration will not delay worker setup. No material merge risk was found.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the frontend test change and the specific behavior being fixed: pinning the ZMQ event plane in the realtime Omni mock worker.
Description check Passed The description is complete and relevant. It explains the problem, implementation, validation results, limitations, reviewer starting point, and related issues. The Summary heading is used instead of …
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@glamr-agent

Copy link
Copy Markdown
Collaborator Author

factory: @dynamo-ops please run full CI for a3595de

@dynamo-ops

Copy link
Copy Markdown
Contributor

/ok to test a3595de

@devin-ai-integration

devin-ai-integration Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

✅ Dynamo PR CI passed — run 38326176612 (attempt 1) on 17609881d3

Gate checks: ✅ backend-status-check · ✅ deploy-status-check · ✅ dynamo-status-check

Posted automatically by Devin for run 38326176612. Updated on every full-CI run of this PR.

Signed-off-by: GLAMR <svc-glamr@nvidia.com>
@glamr-agent
glamr-agent temporarily deployed to external_collaborator October 12, 2026 22:49 — with GitHub Actions Inactive
@glamr-agent

Copy link
Copy Markdown
Collaborator Author

nursery: Added a docstring to main() in tests/frontend/realtime_omni_mock_worker.py in cf62314. It explains that the function registers and serves the mock realtime worker with an explicit ZMQ event plane. This addresses the missing docstring reported in CodeRabbit's Docstring Coverage check; both functions touched by the PR now have docstrings.

@glamr-agent

Copy link
Copy Markdown
Collaborator Author

nursery: @dynamo-ops please run full CI for cf62314

@dynamo-ops

Copy link
Copy Markdown
Contributor

/ok to test cf62314

Signed-off-by: GLAMR <svc-glamr@nvidia.com>
@glamr-agent
glamr-agent temporarily deployed to external_collaborator October 13, 2026 00:12 — with GitHub Actions Inactive
@glamr-agent

Copy link
Copy Markdown
Collaborator Author

nursery: @dynamo-ops please run full CI for 1760988

@dynamo-ops

Copy link
Copy Markdown
Contributor

/ok to test 1760988

@nv-tusharma

Copy link
Copy Markdown
Collaborator

We should investigate why the NATS_SERVER is even relevant in this test in the first place, based on this PR it seems that NATS shouldn't be relevant here?

When NATS_SERVER is set and no event plane is passed, the runtime connects to NATS and retries that first connect for up to 120 s before the worker registers its model. If the NATS server is slow or unreachable.

@glamr-agent

Copy link
Copy Markdown
Collaborator Author

nursery: I changed nothing further after investigating nv-tusharma's question about NATS_SERVER. The test does not need NATS, but the worker's runtime constructor could still enable a NATS client during startup.

RealtimeOmniMockWorkerProcess inherits the parent environment through ManagedProcess. The file_storage_backend fixture only sets DYN_FILE_KV; it does not clear NATS_SERVER. One possible source is runtime_services_session, which leaves NATS_SERVER set until session teardown after another test requests it. The parent environment can also supply it. I cannot establish which source, if either, affected the original failure.

The Python binding's DistributedRuntime constructor enables NATS whenever NATS_SERVER is present and no explicit event_plane argument was passed, even with file discovery and TCP requests. DYN_EVENT_PLANE=zmq selects the event transport but does not bypass that separate condition. Runtime construction then waits for the NATS connection before the worker can register its model. The default startup retry window is 120 seconds, longer than the worker's 60-second health check.

The existing event_plane="zmq" argument bypasses that condition for this TCP worker and makes its setup independent of NATS_SERVER. This explains why NATS could affect startup even though the audio round trip does not use it.

@nv-tusharma

Copy link
Copy Markdown
Collaborator

so the NATS_SERVER is shared across all sessions which is why we need to explicitly pass ZMQ?

@glamr-agent

Copy link
Copy Markdown
Collaborator Author

nursery: I changed nothing in response to nv-tusharma's follow-up about sharing NATS_SERVER. The value is shared by tests within the same pytest process after runtime_services_session sets it, until that fixture tears down. Separate pytest processes have separate environments. The fixture lets xdist workers reuse the same NATS service through a lock and a saved port; each worker that requests it sets its own NATS_SERVER.

The mock worker inherits its parent pytest process's environment. Passing event_plane="zmq" explicitly tells DistributedRuntime that this TCP worker does not need a NATS client, even if that inherited environment contains NATS_SERVER. This also covers a value supplied before pytest starts. Sharing the NATS service is one possible source of the value; it is not required for the startup problem to occur.

This branch was previously deployed

1 inactive deployment
external_collaborator — 17609881 Deployed Oct 13, 2026 by glamr-agent via ok-to-test #31033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution Pull request is from an external contributor size/S test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix realtime Omni mock worker startup with ambient NATS configuration

3 participants