Skip to content

[Test] Add: deferred-delivery mode to the mock network hub #149

Description

@thep2p

Summary

NetworkHub::route_event in src/network/mock/hub.rs delivers each event by calling the target's MockNetwork::incoming_event inline, inside the sender's synchronous Network::send_event call. A request and its whole reply chain therefore finish before send_event returns. A test that runs real nodes over the hub can never have two requests outstanding at the same time, whether they come from one node or from two. It also can never deliver replies out of send order.

The gap came up in the PR #139 review thread (#139 (comment)). That comment showed that tokio::join! in BaseNode::join_stage1_link_level0 does not overlap the two stage-1 link requests on the mock network. The PR corrected the documentation in commit 311630f, because stage 1 does not need the overlap to be correct. This issue tracks the test-infrastructure gap that the review exposed.

Why it matters

  • Every production transport is asynchronous. It queues a frame and returns, so requests overlap and replies arrive in any order.
  • Code that is correct only because delivery is synchronous passes on the mock and fails on a real transport. Two examples of that bug class are code that assumes a reply was already applied by the time send_event returns, and a waiter-registration order that works only with inline delivery.
  • End-to-end tests of concurrent joins over the hub cannot be written today. One example is two joiners whose GetLinkOp chains interleave at a shared neighbor, which exercises the forwarding rule in docs/protocol/concurrent-insert.md, Section 4.1. The Section 7 regression test ([Test] Add: deterministic concurrent-insert repair regression test #97) avoids the gap by calling handlers directly, so [Test] Add: deterministic concurrent-insert repair regression test #97 does not depend on this issue.

Location

  • src/network/mock/hub.rs: NetworkHub and route_event, the home of the new mode.
  • src/network/mock/network.rs: MockNetwork::incoming_event, the delivery target that a release calls.
  • A new test for the acceptance scenario below, placed per the node suite's colocated *_test.rs convention.

Recommended direction

  • An opt-in delivery mode on NetworkHub. In that mode route_event pushes (origin, target, event) onto a bounded queue and returns Ok(()).
  • Test-facing controls that inspect pending events and release them one at a time or in a chosen order, for example pending(), deliver_next(), and deliver_matching(predicate). The test drives every release. No wall-clock sleep is used, and every async wait is timeout-bounded.
  • The default mode stays synchronous. Every existing test, and the [Test] Add: deterministic concurrent-insert repair regression test #97 Section 7 test, is therefore unaffected.
  • Keep the existing single-lock, shallow-clone NetworkHub shape. Do not hold the hub lock while a release calls a node's handler. Today's synchronous route_event holds the hub's read lock across incoming_event, and the release path must not copy that shape.
  • Coordinate with [Test] Add: fault-injection to the mock network (partitions, latency, Byzantine nodes) #62. Its latency simulation can build on this queue instead of adding a second deferral mechanism, per the project's no-duplication rule.

Non-goals

Acceptance Criteria

  • The deferred mode exists and is off by default. The full existing suite passes with no edit to any existing test.
  • One test runs join_stage1_link_level0 over the hub in deferred mode. It shows both stage-1 GetLinkOp requests pending before either SetLinkOp reply is delivered. It then delivers the two replies in reverse send order and asserts the joiner's final level-0 lookup-table state. The assertion is on the final table state, not on send order alone.
  • All gates pass. These are cargo fmt --check, cargo clippy --all-targets -- -D warnings with and without --all-features, and cargo test.

Scope

Test infrastructure only. No file outside src/network/mock/ and the new test changes, and no node or core logic changes.

Dependencies

Sub-issue of epic #81. Blocked by #90, which introduces join_stage1_link_level0, the function the acceptance test drives. Related to #62, whose latency simulation is to build on this queue. This issue blocks nothing, and #97 in particular does not wait on it.

Dependency graph (unblocked first):
EPIC #81 -- local search & join protocol
  #67 [DONE] search_by_id            #71 [OPEN] search_by_mem_vec (independent)
  #74 [OPEN] search_by_id timeout fix   #76 [OPEN] delete/leave
  Concurrent join + repair (Algorithm 2 + Algorithm 8; supersedes closed #66, #77):
    L0 (no deps):         #84  #85  #93  #86  #87
    L1:                   #94  #88  #89
    L2:                   #90  #91  #95  #96
    L2.5 (link round-trip follow-ups; #146 blocks #92): #145  #146
    L2.5 (stage-1 concurrent-join fix; blocked by #90, blocks #97): #148
    L3:                   #92
    L4 (acceptance gate): #97
    L5 (post-gate cleanup; also blocked by #76): #100
  Test-suite cleanup, blocked by #90:
    #147
  Test infrastructure, mock network hub (#62's latency simulation builds on #149's queue):
    #149 (blocked by #90, blocks nothing) <-- YOU ARE HERE
    #62 (no blocker)

Activity

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions