Skip to content

Add: correlate remote task frames across hosts - #2304

Open
doraemonmj wants to merge 1 commit into
hw-native-sys:mainfrom
doraemonmj:feat/cross-host-frame-identity
Open

doraemonmj wants to merge 1 commit into
hw-native-sys:mainfrom
doraemonmj:feat/cross-host-frame-identity

Conversation

@doraemonmj

Copy link
Copy Markdown
Contributor

Summary

  • Add a compact base36 f=<session>:<sequence> key to successful remote dispatch spans and the corresponding peer remote_task span.
  • Preserve the original TASK frame identity through MPI while retaining rank-local identity for control traffic, with explicit mismatch diagnostics.
  • Keep tracing failures outside protocol state, avoid publishing identity for rejected submissions, and document cross-host correlation semantics.

Testing

  • python -m pytest tests/ut -m 'not requires_hardware' -q — 2400 passed, 13 skipped, 51 deselected.
  • ctest --test-dir tests/ut/cpp/build -LE requires_hardware -j4 --output-on-failure --timeout 300 — 151 passed.
  • Post-rebase focused Python tests — 27 passed.
  • Post-rebase focused C++ tests (test_remote_endpoint, test_scheduler) — 2 passed.
  • Two-machine A3 Network1 L4 → remote L3 hardware ST — 1 passed in 14.70s.
  • Direct two-machine hardware run produced matching keys on both hosts: network1.dispatch ... f=j2v9176v1xte:d and node.remote_task ... f=j2v9176v1xte:d; neither record was truncated.
  • pre-commit run --from-ref upstream/main --to-ref HEAD — passed.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d0fac1ba-ebf3-40e9-9ec3-de982dbba2fa

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: dec9000a-7afb-4ba7-a08c-fc999e93dca4

📥 Commits

Reviewing files that changed from the base of the PR and between 2970813 and e40fba4.

📒 Files selected for processing (11)
  • docs/dfx/host-trace.md
  • python/simpler/mpi_l3_session.py
  • python/simpler/remote_l3_session.py
  • src/common/hierarchical/remote_endpoint.cpp
  • src/common/hierarchical/remote_endpoint.h
  • src/common/hierarchical/worker_manager.cpp
  • src/common/hierarchical/worker_manager.h
  • tests/ut/cpp/hierarchical/test_remote_endpoint.cpp
  • tests/ut/cpp/hierarchical/test_scheduler.cpp
  • tests/ut/py/test_mpi_l3_group.py
  • tests/ut/py/test_remote_l3_protocol.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change preserves TASK frame identity, rewrites CONTROL frames with local sequences, emits remote task spans with base36 correlation tokens, and propagates published frame attributes into successful dispatch traces.

Changes

Remote frame identity tracing

Layer / File(s) Summary
Frame identity forwarding
python/simpler/mpi_l3_session.py, tests/ut/py/test_mpi_l3_group.py
TASK frames now require matching session and worker identities. CONTROL frames receive the local command sequence. Tests cover preservation, mismatch errors, and rewriting.
Served task span emission
python/simpler/remote_l3_session.py, tests/ut/py/test_remote_l3_protocol.py, docs/dfx/host-trace.md
Remote TASK processing emits <level>.remote_task spans with bounded base36 session:sequence attributes when host spans are active. Span diagnostics do not interrupt processing.
Dispatch correlation propagation
src/common/hierarchical/..., tests/ut/cpp/hierarchical/...
Remote endpoints retain the latest published frame identity. Successful dispatch traces include its attributes, while failed submissions do not reuse stale attributes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant WorkerThread
  participant RemoteL3Endpoint
  participant RemoteL3Session
  participant HostTrace
  WorkerThread->>RemoteL3Endpoint: submit_progress(frame)
  RemoteL3Endpoint-->>WorkerThread: progress_frame_attrs()
  WorkerThread->>HostTrace: emit dispatch trace with f=session:sequence
  RemoteL3Session->>HostTrace: emit remote_task span with same f=session:sequence
Loading

Merge Risk: ⚪ Minimal · up to e40fb

The new tracing preserves normal task failure handling and associates successful dispatches with the correct remote frame. No actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 10 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding cross-host correlation for remote task frames.
Description check ✅ Passed The description directly explains the cross-host correlation, frame identity handling, tracing behavior, documentation, and testing for the changeset.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 10 files. (1 skipped: 1 unsupported.)


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit guards the frame’s bright thread
Base36 marks where tasks have sped
TASKs keep the names they bore
CONTROLs gain sequences at the door
Spans now carry tokens light
And failed dispatch leaves traces right

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

@doraemonmj
doraemonmj force-pushed the feat/cross-host-frame-identity branch from 2c5a046 to e40fba4 Compare September 18, 2026 06:29
Comment thread src/common/hierarchical/remote_endpoint.h Outdated
- Add one frame key to dispatch and peer remote_task spans.

- Encode session and sequence in base36; keep worker identity separate.

- Preserve TASK identity through MPI while rewriting control traffic.

- Keep trace failures and rejected submissions outside protocol state.
@doraemonmj
doraemonmj force-pushed the feat/cross-host-frame-identity branch from e40fba4 to f01dbd7 Compare September 20, 2026 08:46
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.

2 participants