feat(net)!: price a route warm and cold so warm relays can be ranked - #2925
feat(net)!: price a route warm and cold so warm relays can be ranked#2925kixelated wants to merge 4 commits into
Conversation
The carrying discount works: a relay already serving a broadcast advertises
cost 0, so a cluster deduplicates onto the warm copy instead of each relay
opening its own upstream fetch. But the discount is also lossy. Once two
relays both carry the broadcast they both advertise 0, the marginal costs
tie, and only the FNV hash breaks it. A coin flip picks the aggregation
root, so a relay one cheap link from the publisher can end up pulling
through a relay that is two expensive links away.
The missing signal is the undiscounted path cost, and no receiver can derive
it locally: it needs topology the announcement does not carry. So it has to
go on the wire. Rather than add a second metric beside the first, this makes
one metric a pair.
`broadcast::Cost { warm, cold }` prices the same path against two cache
states. `warm` is what one more subscription costs the mesh today, and takes
the carrying discount. `cold` prices the identical path as if nothing along
it were carrying, so it flows through a warm relay unchanged and still says
which of two discounted relays sits closer to the publisher. Both accumulate
per link together.
Because the pair derives `Ord` in field order, the cold tie-break falls out
of the existing ordering with no new code: `route_order` still names
`route.cost`. `Route` keeps the field count it had, since `advertised` also
becomes a `Cost`. `with_cost` takes `impl Into<Cost>`, so a bare `u64` still
means an undiscounted route and existing call sites are unchanged.
The handover gate now descends `(cold, hash)` instead of the hash alone.
Adopting a parent charges that link onto our own cold cost, so afterwards we
rank strictly above the relay we adopted: the order descends, which forbids a
cycle of any length rather than only the two-relay case. The hash now only
separates relays that are equally far from the publisher, which is what it
is good for.
A wire with no room for a cold cost (pre-lite-06, or the MoQ Cluster
extension) reads as the saturation ceiling, not as zero. Zero would make a
peer that told us nothing look like the publisher itself and outrank every
relay that told the truth. The ceiling ties against other unknowns and falls
through to the hash, which is exactly how those peers behaved before.
`lite::RouteCost` is gone: `Cost` carries its own `Encode`/`Decode`, two
varints on lite-06 and nothing before. That type was never public (`mod lite`
is private), so the only breaking change is `Route.cost`'s type.
The FFI surface stays a plain `u64` mapped from `cost.warm`, since cold is a
relay-mesh concept no application routes on, so nothing ripples to libmoq or
the language wrappers. The relay's `/nodes` JSON does gain `cold_cost`, which
the rollout needs to prove routing from outside.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The equal-warm-cost test priced a one-hop route at warm 1 / cold 9, which cannot happen: a one-hop chain means the announcing peer is the publisher, and a publisher seeds both halves equal while a link charges both equally, so warm and cold are the same number there. More generally `warm <= cold` always, and they only diverge once something upstream is warm. Which also means a direct publisher route can never lose the cold comparison at equal warm cost, so it cannot demonstrate the property at all. Two warm relays can: the one rooted nearer the publisher sits deeper in the aggregation tree and so advertises the longer chain, which is exactly the case hop count reads backwards. Generalize the fixture to `warm_route(hops, cold, link)` and state the property the ordering actually has, rather than the one the quest sketch described: the cold cost and hop count both estimate how far away the content is, and the cold cost does it in the operator's own prices. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ffba9e82a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * that id instead of repeating the path. Older versions send a single ANNOUNCE_BROADCAST | ||
| * message that retracts by path (`ended`). | ||
| */ | ||
| /** |
There was a problem hiding this comment.
Inserting the Cost declaration here leaves the existing announcement JSDoc attached to Cost rather than AnnounceBroadcast, so the exported union loses its description in generated API documentation. Move the announcement block directly above export type AnnounceBroadcast. (Written by GPT-5.6 Sol)
AGENTS.md reference: AGENTS.md:L97-L97
Useful? React with 👍 / 👎.
The cluster page stated the rule without the intuition, which left a reader to work out for themselves why one price is not enough. Lead with what the discount erases: once several relays are all carrying a broadcast they all advertise 0, so the question "what does this cost you?" gets the same answer everywhere and stops distinguishing them. Then name the two prices for what they each decide. Warm picks where a relay fetches from; cold picks which relay does the fetching. Spell out what breaks without each, since that is what makes the pair look necessary rather than redundant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2304150823
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| && incumbent.route.announce | ||
| && candidate.route.cost < incumbent.route.cost | ||
| && candidate.route.advertised == 0 | ||
| && candidate.route.advertised.warm == 0 | ||
| && candidate.route.hops.len() >= 2 | ||
| && !self.handover_allowed(&candidate.route) | ||
| && !self.handover_allowed(&candidate.route, &incumbent.route) |
There was a problem hiding this comment.
When an active route is updated to pass through an already-exposed peer while a more expensive clean warm route remains, best_route() deliberately selects the clean route, but this broadened handover gate can reject it based on relay rank. active then remains the tainted route, so routes_snapshot() advertises and discounts that route as serving even though serve_route() refuses it and actually serves the clean source. Advertisements to other peers consequently describe a different chain from the one dispatch uses, breaking the advertise/dispatch split-horizon invariant and potentially rebuilding a subscription loop. The handover gate should not retain an incumbent for which taints_a_reader() is true.
Useful? React with 👍 / 👎.
The Cost interface landed between the announcement doc block and the union it describes, leaving two consecutive JSDoc blocks above Cost and orphaning the first. AnnounceBroadcast is exported, so its description has to reach the generated API docs. Found by the Codex reviewer on the PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
What
Splits the lite-06 route cost into a pair:
broadcast::Cost { warm, cold }, the same path priced against two cache states.warmtakes the carrying discount and is what routing minimizes;coldprices the identical path as if nothing along it were carrying, and is what ranks two relays that have both discounted to zero.This implements the pop-skipping quest's
rankslice and supersedes #2894, which solved the same problem by adding twoOption<u64>fields beside the existing cost.Why
#2424's carrying discount is right, and it already lands the deduplication: a relay serving a broadcast advertises 0, so a sibling pulls the warm copy over a free intra-DC link instead of opening a second metered fetch.
But the discount is lossy. Once two relays both carry the broadcast they both advertise 0, marginal cost ties, and only the FNV hash breaks it. A coin flip picks the aggregation root. On the quest's topology that means SJC (two links from IAD) can win the flip and DAL (one link from IAD) aggregates onto it, wasting DAL's cheap link and paying for SJC's expensive one twice.
The signal that breaks that tie correctly is the undiscounted path cost, and a receiver cannot derive it: it needs topology the announcement does not carry. So it has to go on the wire. The design question is only what shape it takes.
How
Rather than a second metric beside the first, one metric becomes a pair:
Ordin field order, so the cold tie-break falls out of the existing ordering with no new code:route_orderstill namesroute.cost, unchanged.Routekeeps its field count.cost: Costandadvertised: Costreplace twou64s; there is no third or fourth cost-shaped field, no hand-writtenDefault, norankparameter threaded through selection, and noown_rank()recomputed insidemin_by_key(which feat(net)!: add cold route rank for pop skipping #2894 made O(n²) per selection).with_cost(impl Into<Cost>)withFrom<u64> for Cost: a bareu64prices the route undiscounted, so every existing call site compiles unchanged and a publisher seeding its production cost still writeswith_cost(1000).charged(link)adds to both,discounted()zeroeswarmonly,clamped()caps both.(cold, hash)instead of the hash alone. Adopting a parent charges that link onto our own cold cost, so afterwards we rank strictly above the relay we adopted. The order descends, which forbids a cycle of any length rather than only the two-relay case the hash covered. The hash now only separates relays equally far from the publisher, which is what it is good for. Two escapes stay always-allowed: an empty hop chain (a local publish) and a route from the relay we already serve from (that session reconnecting, not a new parent). The oldcandidate.cost < incumbent.costprecondition is gone, since the gate must now fire on an equal-marginal-cost tie too.lite::RouteCostis deleted.Costcarries its ownEncode/Decode: two varints on lite-06, nothing before. That type was never public (mod liteis private), soRoute.cost's type is the only breaking change.The unknown-cold rule
A wire with no room for a cold cost (pre-lite-06, or the MoQ Cluster extension) decodes
Cost::UNKNOWN = { warm: 0, cold: MAX_COST }, not zero.Zero would be actively wrong: a peer that told us nothing would look like the publisher itself and outrank every relay that told the truth, so a mixed-version mesh would drag its aggregation point onto whichever peer said the least. The ceiling ties against other unknowns and falls through to the hash, which is exactly how those peers behaved before cold cost existed. This is why no
Optionis needed anywhere.Public API (moq-net)
broadcast::Route.costis nowCost, notu64.Route::with_coststill accepts au64.broadcast::Cost(#[non_exhaustive], withnew,DRAIN, andFrom<u64>).advertised,UNKNOWN,charged,discounted,clamped) stayspub(crate).Deliberately not changed:
moq-ffi'sMoqRoute.coststays a plainu64mapped fromcost.warm. Cold cost is a relay-mesh concept no application routes on, so nothing ripples tolibmoq,py/,swift/,kt/,go/, or their docs.moq-relay's/nodesJSON does gaincold_costbesidecost, which the rollout needs in order to prove routing from outside.Tests
Every new mechanism was mutation-verified — each of these mutations was applied and confirmed to fail a specific test:
follows_the_cheaper_rootdiscounted()zeroes cold tooCost::UNKNOWN.cold= 0unknown_cold_ranks_last,..._falls_back_to_the_hashTwo vacuity holes were found and closed this way. The first pass hardcoded
MAX_COSTin the unknown-cold test instead of namingCost::UNKNOWN, so mutating the constant passed; and nothing pinned "an unknown cold path must not outrank a known one" at all.New tests:
follows_the_cheaper_rootandrejects_the_pricier_root(both looping over both hash directions, so they prove the cold cost decides rather than a lucky hash),descends(the cycle-freedom invariant),reverts_when_the_parent_goes,unknown_cold_ranks_last,unknown_cold_falls_back_to_the_hash,equal_warm_cost_prefers_the_cheaper_root(the SEA case: an equal-marginal warm relay beats a shorter direct chain), plusCostarithmetic and an asymmetric wire round-trip in both Rust and TS — a symmetric pair round-trips fine even if the encoder writes one field twice.just fix,just check, andjust testall pass.Cross-package sync
js/net:Costinterface mirrored; the two varints encode and decode; a wire that cannot carry them now decodesundefinedrather than a misleading0n.drafts/draft-lcurley-moq-lite.md:Route CostbecomesWarm Route Cost+Cold Route Costin ANNOUNCE_START and ANNOUNCE_UPDATE, with the rank rule replacing the "deterministic tie-break" guidance, the unknown-cold rule, and the -06 changelog entry amended in place (the draft is unpublished WIP, so this is not a new entry).just drafts checkrenders it.doc/bin/relay/{cluster,http}.md: the second price and the newcold_costfield.(written by Opus 5)