Skip to content

relay: a re-REQ can cancel the read that replaced it #950

Closed

Description

@chappie-daemon

Correction after recon on job 010: this is not a regression from the TypeScript capture. The capture has no per-id table, cleans up by object identity, and does not abort on a plain re-REQ. The silent stop is a shape the Rust port introduced. Anyone fixing this should not "restore" capture behaviour — there is none to restore here.

Measured: a probe deterministically reads Err(RecvError) where a live read should be. That is the deciding check — one assertion in the shape of the existing connection test, not a scenario.

Where

crates/nostream-relay/src/connection.rs:213-241 — the replacement path begins a new read for an id that already has one; the old read's end_read then tears down the new read.

Ownership idiom

No epoch or token idiom exists to reuse: AtomicU64 appears only in a test double, and tokio watch handles have no PartialEq, so a per-read token cannot be derived from the receiver. A (u64, Sender) pair under the existing map lock is the cheap fit — the lock is held while the previous sender is signalled, so a token allocated before the lock would change the lock discipline and a counter incremented under it does not.

Acceptance

  • A re-REQ for a live id delivers frames from the new read.
  • The old read's termination does not cancel the new one.
  • A regression test that fails at 740ef69 and passes after.

Activity

  1. added
    bugSomething isn't working
    rewritenostream 4.0 Rust/LMDB rewrite
    rustRust implementation work
    factoryBuilt by the software factory
    on Oct 24, 2026
  2. chappie-daemon commented on Oct 24, 2026

    @chappie-daemon
    CollaboratorAuthor

    Build started — factory job 010-relay-concurrency-fixes (spec stage done, builder running on factory/010-relay-concurrency-fixes).

    The spec's requirement for this issue (R1–R5): the in-flight table records a connection-wide monotone generation beside each watch::Sender, end_read takes the read's own handle by value and removes the entry only when the stored generation still matches, and begin_read returns InFlightRead { subscription_id, generation, receiver }. The deciding test is a_finished_read_does_not_remove_the_read_that_replaced_it, which fails at 740ef69 on has_changed() → Err(RecvError(())) (probe A2). Re-REQ must still reach EOSE.

  3. chappie-daemon commented on Oct 24, 2026

    @chappie-daemon
    CollaboratorAuthor

    Landed in #962 (merge commit bca0f6f) on rewrite/main.

    Cost — job 010, actual vs expected: $2.34 actual against $250 expected (0.9%). Measured by factory-spans.ts over 17 sessions, 802 requests, 6.0 h summed wall, 3.3M in / 0.7M out / 85.2M cache-read. The job fixed all three defects in one branch; pro-rata by changed source lines this issue's share is about $0.82 (read ownership (generation guard on end_read): connection.rs 97 + handlers/subscribe.rs 54 changed lines), which is an attribution and not a separate bill — the job is the billing unit.

    The estimate was the defect, not the work: the model is being replaced with one built from measured jobs (see .factory/estimates.md).

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingfactoryBuilt by the software factoryrewritenostream 4.0 Rust/LMDB rewriterustRust implementation work

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions