Route census: no way to see who a module currently serves, so restart blast radius is guesswork #17

Activity

1. iceteaSA commented on Aug 12, 2026

   

   iceteaSA

   on Aug 12, 2026

   ContributorAuthor

   More actions

   Triaged from the daemon side. This one is cheap and the data already exists — no new bookkeeping, no new state, just a read-only projection of what the forwarding table holds right now.

   ForwardingInner (crates/subc-core/src/forwarding.rs:261-281) already maintains:

   client_to_module: HashMap<ClientRouteKey, Arc<RouteBinding>>,
   module_to_client: HashMap<ModuleRouteKey, Arc<RouteBinding>>,
   modules_by_id:    HashMap<String, ModuleConnection>,
   module_id_by_endpoint: HashMap<ModuleEndpointId, String>,
   draining_endpoints: HashSet<ModuleEndpointId>,
   status: HashMap<(ClientRouteKey, u32), String>,

   A census is a fold over module_to_client grouped by module id. The blast radius of restarting module X is literally the set the drain path already computes to send GOODBYEs to — begin_module_drain returns exactly those abandoned_bindings. So the daemon can already answer "who would this restart hurt"; it just never exposes it until it's tearing them down.

   That's the sharpest argument for this issue and I'd put it in the body: the information exists at exactly one moment — while it's being destroyed. Same shape as last_exit in #12 and the pre-#7 stderr situation. A read-only op turns a teardown-time computation into an ask-first one.

   Three notes on the shape

   Route identity is connection-scoped, not global. ClientRouteKey is {connection_id, channel} and the comment is explicit: "A route channel is unique only within one client connection." So a census can't report a bare channel number as an identifier — it needs the connection id alongside, or the output is ambiguous across clients. Worth pinning in the op's schema rather than discovering at render time.

   Draining state should be in the output. draining_endpoints is tracked separately, and a route that's mid-drain is a materially different answer to "who does this serve" than a live one. If the census omits it, an operator checking blast radius during a reload sees a route that's already going away and counts it as collateral.

   There's no client identity to report, and that's the honest limit. The table keys clients by ConnectionId — an internal counter, not a name, pid, or anything an operator recognises. So ck routes can say "module aft serves 7 routes across 3 connections" but not whose connections. That's still useful for blast radius (a count is what you need before a restart), but it won't answer "which of my sessions breaks", and the issue shouldn't promise that. Making it answer that is a separate and much larger ask — clients don't currently identify themselves at all.

   Fit with the #4 ruling

   Agreed that this is the live-state complement to supervisor.list, and it's cleanly on the correct side of the state-free line: the daemon already holds this in memory as routing state, so exposing it adds no persistence and no new lifecycle. That's a different case from #12, which asks the daemon to remember something after the fact — this one only asks it to say what it currently knows.

   Worth stating that distinction on the thread, because "add an observability op" reads similar in both, and only one of them is state-free.

   I can implement this once #10 lands — it touches control.rs and subc-control in the same region as the supervisor.stderr_tail op, and I'd rather not stack two unmerged branches on the same surface.
2. ualtinok commented on Aug 14, 2026

   

   ualtinok

   on Aug 14, 2026

   Contributor

   More actions

   Accepted — this is a surfacing ask for state the daemon already holds, and the operating reality you describe ("restart and apologize" because doubt is the only available state) is exactly what today's partial answer was built to end: server.describe now carries route-concentration counters (client_connections_with_routes, max_routes_on_one_connection, shipped 30044-era) — but those are aggregates, and your ask is the per-module, per-route identity view the aggregates deliberately elided. The mux table has it; nothing serves it.

   Shape agreed as filed: read-only supervisor.routes (optionally filtered by module) returning per route the bound consumer identity (attested BindIdentity — which is exactly what the principal stamping was built to make visible), channel count, and age; ck routes [module] as the verb. Live-state only, no history claim, observers own history per #4 — your scope-honesty paragraph can go into the op's doc comment nearly verbatim.

   One design note for whoever builds it: the census read must come off the forwarding table under its read lock without touching any module (pure daemon-state read, zero frames to modules — same discipline as route.poll being answered locally). That also makes it safe to call during a drain, which is precisely when an operator will call it.

   Your pairing with #16 telemetry ("27G serving 0 routes is a free restart; serving 5 is a scheduled one") is the operator contract both features exist for and will go into ck routes' help text. PR welcome if you want it — the forwarding-table read plus a control op is a well-bounded slice, and #10 already walked you through the golden-vector process; otherwise it goes on our schedule after the #12 ring.
3. iceteaSA commented on Aug 14, 2026

   

   iceteaSA

   on Aug 14, 2026

   ContributorAuthor

   More actions

   Taking it, after #10 lands — same sequencing reason I gave on #12: this, #12's ring, and #16's telemetry all touch control.rs / subc-control, which is where #10 currently sits, and three unmerged branches on one surface turns a merge into a correctness question. #14 is running now because manifest.rs is disjoint from all of it.

   Your design note is load-bearing and I want to name why, since it's easy to read as a performance nicety:

   the census read must come off the forwarding table under its read lock without touching any module — same discipline as route.poll being answered locally.
   That also makes it safe to call during a drain, which is precisely when an operator will call it.

   That second sentence is the requirement, and it inverts the obvious implementation. A census that consulted modules would be unusable at the exact moment it's needed — mid-drain, when the operator is asking "what am I about to break," and when a draining module is by definition not answering. It would also reproduce AFT pin #1's hazard: a passive status query hitting a module mid-work and tripping a hang-restart. So the read has to be daemon-state-only, and that's a correctness property of the feature rather than an optimisation of it.

   One consequence worth pinning in the op's contract: a census taken during a drain must report the draining routes as draining, not omit them. draining_endpoints is tracked separately from the bindings, so the naive fold over module_to_client will happily report a route that's already receiving its GOODBYE as live. An operator checking blast radius mid-reload would then count collateral that's already gone — and worse, a route that vanishes between two censuses reads as "it was fine, then it wasn't" rather than "it was already going away."

   On BindIdentity: agreed that's the right thing to surface, and it's a better answer than the caveat I filed with. I'd said the census can report counts but not whose connections, because clients are keyed by an internal ConnectionId. The attested principal is exactly the identity the stamping was built to make visible — so for reserved consumers the census can name them, and for {"kind":"direct"} it honestly can't. That's a two-tier answer rather than a flat limitation, and the doc comment should say which callers get which.

   Happy to take the scope-honesty paragraph into the op's doc comment as you suggested. I'd add the draining-state line to it.
4. iceteaSA commented on Aug 15, 2026

   

   iceteaSA

   on Aug 15, 2026

   ContributorAuthor

   More actions

   Starting this now that #10 has landed. Read the bind path first, and I need to correct my own framing on this thread before building to it — two of the three fields in the agreed shape aren't retained anywhere.

   I said this was "a surfacing ask for state the daemon already holds" and "a fold over module_to_client." That's true for the routes themselves. It is not true for identity or age.

   RouteBinding at 06a2e76 in full:

   pub(crate) struct RouteBinding {
       pub client_connection_id: ConnectionId,
       pub client_sink: FrameSink,
       pub client_negotiated_ver: u8,
       pub client_channel: u16,
       pub client_epoch: u32,
       pub module_id: String,
       pub module_endpoint: ModuleEndpointId,
       pub module_sink: FrameSink,
       pub module_negotiated_ver: u8,
       pub module_channel: u16,
       pub module_epoch: u32,
       pub flow: Arc<ChannelFlow>,
   }

   No principal. No timestamp.

   The principal is computed in handle_route_open (control.rs:1125), used for the admission-facts carrier check, then moved into the RouteBind relay frame at :1245 and dropped. It is transited, not stored — the module receives it, the daemon keeps nothing. Same shape as the quiescence bool in #22 and last_exit in #12: computed, used once, discarded.

   So the agreed shape costs more than a read:

   field	status
   module id, channels, connection, epoch, draining	free — a fold over existing state
   consumer identity	needs Principal retained on RouteBinding, populated at bind
   age	needs an Instant stamped at bind

   Neither is expensive — two fields on a struct that's already Arc-wrapped per route, written once at bind, never mutated. But it is a write-path change, not a read-only projection, and I told you it wasn't. Worth knowing before you schedule it.

   What I'd like to settle before writing code

   Take both fields, or ship the free half first? I'd take both — an age-less census still answers blast radius ("how many, whose"), but identity is the part that turns a count into a decision, and retrofitting a bind-time field later means touching the same code twice. The write-path cost is one Instant::now() and one clone per route.open, which is not a hot path.

   One thing I want to flag rather than decide: retaining the principal means the daemon holds an attested identity for the lifetime of every route. That's a small standing increase in what a supervisor.routes caller can learn about who else is connected — currently nobody can enumerate that because nobody stores it. Given the op is operator-facing and the box is single-tenant it seems fine, but it's a real change in what the daemon knows about its clients, and you should get to say so rather than discover it.

   Unchanged from your spec

   The read comes off the forwarding table under its read lock, zero frames to modules, safe during a drain. draining_endpoints is consulted so draining routes report as draining rather than being omitted — a route already receiving its GOODBYE must not read as live collateral.

   Two-tier identity holds and gets clearer with the principal retained: {"kind":"reserved"} consumers get named, {"kind":"direct"} honestly cannot be. The doc comment will say which callers get which rather than implying the census can always answer.

   Ordering note

   This produces the read-only endpoint-route enumeration route.closing needs in #22 — no such read exists today (release_module_endpoint_routes is the only thing in that shape and it releases as it enumerates). Building it here first, shaped for both, unless you'd rather it land with the push family.
5. ualtinok commented on Aug 15, 2026

   

   ualtinok

   on Aug 15, 2026

   Contributor

   More actions

   Take both fields — your retrofit argument decides it (a bind-time field added later touches the same code twice), and the write cost is one Instant::now() and one Principal clone on a path that is nowhere near hot.

   The framing correction is accepted with the same note as #22's: transited-not-stored is exactly the last_exit/quiescence-bool shape, and catching it before building is the system working. The census is now honestly a write-path change plus a projection, and that's fine — the fields are written once at bind, never mutated, and the struct is already per-route.

   On the privacy flag — approved, with the boundary stated so it survives us: retaining the principal adds no new knowledge (the daemon computed it at bind; it chose to drop it), only new enumerability. That's acceptable on two conditions, both structural:

   1. supervisor.routes lives on the control plane only — never advertised through the MCP facade or any tool surface. Same placement rule as every operator op: a privileged read must be unaddressable from the agent-tool direction, not merely policy-denied. State it in the op's doc comment.
   2. Two-tier identity stays honest as you specified: reserved consumers named, direct consumers rendered as direct — the census must never dress an unattested connection in a name. The doc comment says which callers get which.

   Ordering confirmed: build the read-only endpoint enumeration here, shaped for both this census and #22's route.closing (per-route data sufficient to build a GoodbyeTarget, draining state included). #22 starts when it lands.
6. ualtinok

   closed this as completedin #26on Aug 16, 2026
7. ualtinok

   added a commit that references this issue on Aug 16, 2026

   forwarding: a read-only route census so blast radius is knowable befo…

   ...

   This issue will close once commit f0bbd1b is merged into the 'master' branch.

   f0bbd1b