mirror of
https://github.com/openziti/ziti.git
synced 2026-09-10 16:55:41 +00:00
Paul Lorenz
d5d9d936d1
BuildRouterLinks runs once per router connect and scanned the whole link table to find the connecting router's links. The table is proportional to the square of the router count in a full mesh, so a wave in which every router reconnects walks the cube of it in link visits. That never shows in steady state; it arrives during a mass reconnect, when the connect path is already the bottleneck. Indexing every link under both endpoint router ids fixes the cost, and introduces the problem that the table and the index are two structures written in sequence. Three things follow from that, and the destination repair below turned out to depend on the connect path's ordering as well. Performance: - indexes every link under both endpoint router ids, maintained in the add and remove paths all link mutations funnel through, so a connect costs its own router's link count rather than the whole table - prunes a full refresh against the reporting router's own links. That report is what every reconnect sends, and it walked the whole table to find the router's stale links, so the cost the connect path shed was still being paid one message later - indexes link ids rather than link pointers, so a per-router query resolves each id against the table and hands back whatever it currently holds. A reconnect replaces a link's Router object and a higher iteration replaces the Link object, but neither replaces the id, which is exactly why the scan existed - holds each router's link ids in a LockedSet rather than a sharded map, which cost a map and a mutex per shard for an index that is usually empty and is held for as long as the router has existed: 2.9KB per router id against 150 bytes - drops a router's index when the router is deleted, guarded on the store still having no router under that id and checked inside the index map's removal callback. A router id can come back, since fabric router ids are the enrollment certificate's common name, and nothing orders the cleanup against that: store event handlers run after bolt has released the writer lock, so one can execute after the id was recreated, connected and reported links. Indexing a link takes the same shard as the guarded read, so an entry for a live router cannot appear without its create having committed first, which the read then sees, and an entry arriving later blocks and lands in a freshly created index - keeps an index when the store cannot be read, since that is not evidence the router is gone, and dropping one otherwise could discard a link another goroutine had already fetched the index to record Keeping the two structures agreeing: - serializes Add and Remove on the link's id. Without it a removal running between an add's two writes takes the table entry, finds nothing yet in the index, and lets the add index a link the table no longer holds, which nothing cleans up. Only RouterReportedLink held that lock before; the four other removal paths held nothing - serializes the connect-time pairing on that id as well, which was the last writer left outside it. A removal reading no destination could complete while the pairing was still deciding, leaving the link in the destination's link set after the table had dropped it, where path computation still routed over it - unindexes an id only when the table no longer holds a link under it, so a replacement that has taken the table slot is not unindexed by the older link's removal. A stale id is the safe direction to err in, since a query filters it, where an id wrongly dropped leaves a live link no per-router query finds - has per-router queries resolve ids against the table, so an id it no longer holds is not handed out as a live link Pairing a link with its destination: - re-resolves the destination once the link is in the table, and on every later report, so a link that reached a bad state heals. A report resolves the destination before the link is in the table and a router's connect repairs links already in it, so a router connecting between those two moments was seen by neither, leaving a link with no adjacency that every operator-facing view still called healthy - makes that re-resolve unconditional rather than only for a link holding no destination. The resolve happens under the source router's connect stripe, not the destination's, so the destination can be replaced between the resolve and the report landing, and a link left on a connection that is no longer registered carries no adjacency just as one holding none does - registers a connecting router before building its links, which is what makes that re-resolve close the window rather than narrow it - adds Link.PointDestAt so the two repair paths cannot both index the link on the destination, since that index is a slice that does not deduplicate Also here: - logs the link count on both sides of the reconnect exchange, and why a report was discarded. A router announces its links once per reconnect and is not asked again, so a discarded report left it with no links for a router that believed it had announced them, with nothing recorded to compare. The discard reports the state it decided on, read once, since a reconnect landing between the decision and the log would otherwise have it name a state that discards nothing - gives test routers version info, which a connected router always has because the accept path refuses a hello without it, and which the router sync path dereferences with no nil check The destination repair is backported from the gossip link-state branch, where the connect path registers before building links for unrelated reasons. That ordering is load-bearing here and is now explicit at the call site.