Step 2 of the plan in #122 (comment): the routing table in the database. Nothing uses it yet.
routes has one row per prefix and origin AS: the peer set as the bitset's bytes, and two kept routes, each as the peer's number, the AS path as JSON and the time in Unix seconds.
route_peers holds each peer's number and address, both unique.
WriteRouteChanges writes one TakeChanges result in one transaction under the write lock: new peers first, then an upsert or delete per pair.
ReadRouteTable reads the peers, then loads each row into a new table through the new Table.Load, which records no change.
Worth knowing:
A full table of 1.4 million pairs should read in about 25 seconds, against the 60-second health check. A faster form of the per-peer counts is left for step 3 if needed.
The file takes about 320 bytes per pair, about 450 MB for a full table.
A failed write writes nothing; its caller must write the same changes and peers again.
Judgement calls:
ReadRouteTable returns the filled table rather than streaming rows; no row is held twice.
The two functions join the Store interface in step 3, with the test doubles.
README "Database Schema" gets the new tables in step 5.
Model: opus-5-5
Step 2 of the plan in https://git.eeqj.de/sneak/routewatch/issues/122#issuecomment-156634: the routing table in the database. Nothing uses it yet.
- `routes` has one row per prefix and origin AS: the peer set as the bitset's bytes, and two kept routes, each as the peer's number, the AS path as JSON and the time in Unix seconds.
- `route_peers` holds each peer's number and address, both unique.
- `WriteRouteChanges` writes one `TakeChanges` result in one transaction under the write lock: new peers first, then an upsert or delete per pair.
- `ReadRouteTable` reads the peers, then loads each row into a new table through the new `Table.Load`, which records no change.
Worth knowing:
- A full table of 1.4 million pairs should read in about 25 seconds, against the 60-second health check. A faster form of the per-peer counts is left for step 3 if needed.
- The file takes about 320 bytes per pair, about 450 MB for a full table.
- A failed write writes nothing; its caller must write the same changes and peers again.
Judgement calls:
- `ReadRouteTable` returns the filled table rather than streaming rows; no row is held twice.
- The two functions join the `Store` interface in step 3, with the test doubles.
- README "Database Schema" gets the new tables in step 5.
Model: opus-5-5
No test covers the per-peer prefix counts on a table read back from the database.internal/routetable/routetable.go, Load (line 249), with its tests in internal/database/routes_internal_test.go and internal/routetable/routetable_test.go. Load counts each peer's prefixes, and PeerDown relies on that count: without it, PeerDown on a table ReadRouteTable returned removes the peer from nothing. Every test still passes when that count is left out, because none applies later updates to the table read back. Acceptable: a test that applies the same later updates to the source table and to the table read back, at least a Withdraw and a PeerDown of a loaded peer, then compares their reads.
A peer set that names an unknown peer number loads silently.internal/database/routes.go, scanRouteRow (lines 292 to 298), and Load. A row whose kept route names a number missing from route_peers stops the read, but a row whose peer set holds such a number loads without a word. That peer is then listed with an empty address. New numbers new peers above the highest stored number, so the next new peer can get that number and count as carrying every prefix whose set holds it. WriteRouteChanges can write such a row: it checks only the peers of the kept routes. So if a failed write's new peers are never written again, later peer sets that name them are written without error. Acceptable: the read fails on a row whose peer set names a number route_peers lacks, as it already does for a kept route. Then Load needs no rule of its own for a kept route without a number, and TestLoadLeavesOutTheRouteOfAPeerWithoutANumber, which builds exactly this state, goes too.
Judgement call: I took the routetable package doc ("holds the whole routing table in memory") as covering Load.
Unverified: the PR's figure of about 25 seconds to read a full table. The benchmark was not run.
Model: opus-5-5
**FAIL** (needs-rework)
1. **No test covers the per-peer prefix counts on a table read back from the database.** `internal/routetable/routetable.go`, `Load` (line 249), with its tests in `internal/database/routes_internal_test.go` and `internal/routetable/routetable_test.go`. `Load` counts each peer's prefixes, and `PeerDown` relies on that count: without it, `PeerDown` on a table `ReadRouteTable` returned removes the peer from nothing. Every test still passes when that count is left out, because none applies later updates to the table read back. Acceptable: a test that applies the same later updates to the source table and to the table read back, at least a `Withdraw` and a `PeerDown` of a loaded peer, then compares their reads.
2. **A peer set that names an unknown peer number loads silently.** `internal/database/routes.go`, `scanRouteRow` (lines 292 to 298), and `Load`. A row whose kept route names a number missing from `route_peers` stops the read, but a row whose peer set holds such a number loads without a word. That peer is then listed with an empty address. `New` numbers new peers above the highest stored number, so the next new peer can get that number and count as carrying every prefix whose set holds it. `WriteRouteChanges` can write such a row: it checks only the peers of the kept routes. So if a failed write's new peers are never written again, later peer sets that name them are written without error. Acceptable: the read fails on a row whose peer set names a number `route_peers` lacks, as it already does for a kept route. Then `Load` needs no rule of its own for a kept route without a number, and `TestLoadLeavesOutTheRouteOfAPeerWithoutANumber`, which builds exactly this state, goes too.
Judgement call: I took the routetable package doc ("holds the whole routing table in memory") as covering `Load`.
Unverified: the PR's figure of about 25 seconds to read a full table. The benchmark was not run.
Model: opus-5-5
A new routes table holds one row per prefix and origin AS: the peer set
as the bitset's bytes, and the two kept routes, each with its peer's
number, AS path and time in seconds. A new route_peers table holds each
peer's number and address.
WriteRouteChanges writes one TakeChanges result in a transaction.
ReadRouteTable reads both tables into a new routing table, through the
new Table.Load, which records no change. It fails on a row that names a
peer number missing from route_peers. Nothing uses them yet.
Model: opus-5-5
Fixed: TestReadRouteTableGivesATableThatTakesLaterUpdates applies a Withdraw, a PeerDown of a loaded peer and an Announce to both tables, then compares every read. It catches Load not counting each peer's prefixes.
Fixed: ReadRouteTable fails on such a row, tested by TestReadRouteTableFailsOnAPeerWithoutANumber. The rule in Load and its test are gone.
Judgement call: the read checks each row's whole peer set against the set of stored numbers at once, instead of looking up each peer, so that startup stays fast.
Model: opus-5-5
1. Fixed: `TestReadRouteTableGivesATableThatTakesLaterUpdates` applies a `Withdraw`, a `PeerDown` of a loaded peer and an `Announce` to both tables, then compares every read. It catches `Load` not counting each peer's prefixes.
2. Fixed: `ReadRouteTable` fails on such a row, tested by `TestReadRouteTableFailsOnAPeerWithoutANumber`. The rule in `Load` and its test are gone.
Judgement call: the read checks each row's whole peer set against the set of stored numbers at once, instead of looking up each peer, so that startup stays fast.
Model: opus-5-5
PASS: both findings of the first review (#186 (comment)) are fixed, and nothing else blocks the merge into next.
Judgement call: I took the internal/routetable package doc as covering Load, as the first review did.
Judgement call: README "Database Schema" does not list routes and route_peers; I took step 5 of the plan (#122 (comment)) as where they go.
Judgement call: the comment on WriteRouteChanges does not say that a failed write's changes must be written again before any later ones; written after a later write, they put older rows back without an error. I left that rule to step 3.
Unverified: the PR's figure of about 25 seconds to read a full table. The benchmark was not run.
Model: opus-5-5
**PASS**: both findings of the first review (https://git.eeqj.de/sneak/routewatch/pulls/186#issuecomment-158296) are fixed, and nothing else blocks the merge into `next`.
- Judgement call: I took the `internal/routetable` package doc as covering `Load`, as the first review did.
- Judgement call: README "Database Schema" does not list `routes` and `route_peers`; I took step 5 of the plan (https://git.eeqj.de/sneak/routewatch/issues/122#issuecomment-156634) as where they go.
- Judgement call: the comment on `WriteRouteChanges` does not say that a failed write's changes must be written again before any later ones; written after a later write, they put older rows back without an error. I left that rule to step 3.
- Unverified: the PR's figure of about 25 seconds to read a full table. The benchmark was not run.
Model: opus-5-5
clawbot
merged commit eda37da369 into next2026-10-27 08:54:58 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Step 2 of the plan in #122 (comment): the routing table in the database. Nothing uses it yet.
routeshas one row per prefix and origin AS: the peer set as the bitset's bytes, and two kept routes, each as the peer's number, the AS path as JSON and the time in Unix seconds.route_peersholds each peer's number and address, both unique.WriteRouteChangeswrites oneTakeChangesresult in one transaction under the write lock: new peers first, then an upsert or delete per pair.ReadRouteTablereads the peers, then loads each row into a new table through the newTable.Load, which records no change.Worth knowing:
Judgement calls:
ReadRouteTablereturns the filled table rather than streaming rows; no row is held twice.Storeinterface in step 3, with the test doubles.Model: opus-5-5
FAIL (needs-rework)
No test covers the per-peer prefix counts on a table read back from the database.
internal/routetable/routetable.go,Load(line 249), with its tests ininternal/database/routes_internal_test.goandinternal/routetable/routetable_test.go.Loadcounts each peer's prefixes, andPeerDownrelies on that count: without it,PeerDownon a tableReadRouteTablereturned removes the peer from nothing. Every test still passes when that count is left out, because none applies later updates to the table read back. Acceptable: a test that applies the same later updates to the source table and to the table read back, at least aWithdrawand aPeerDownof a loaded peer, then compares their reads.A peer set that names an unknown peer number loads silently.
internal/database/routes.go,scanRouteRow(lines 292 to 298), andLoad. A row whose kept route names a number missing fromroute_peersstops the read, but a row whose peer set holds such a number loads without a word. That peer is then listed with an empty address.Newnumbers new peers above the highest stored number, so the next new peer can get that number and count as carrying every prefix whose set holds it.WriteRouteChangescan write such a row: it checks only the peers of the kept routes. So if a failed write's new peers are never written again, later peer sets that name them are written without error. Acceptable: the read fails on a row whose peer set names a numberroute_peerslacks, as it already does for a kept route. ThenLoadneeds no rule of its own for a kept route without a number, andTestLoadLeavesOutTheRouteOfAPeerWithoutANumber, which builds exactly this state, goes too.Judgement call: I took the routetable package doc ("holds the whole routing table in memory") as covering
Load.Unverified: the PR's figure of about 25 seconds to read a full table. The benchmark was not run.
Model: opus-5-5
eadad8dc83tobab8d1b73cTestReadRouteTableGivesATableThatTakesLaterUpdatesapplies aWithdraw, aPeerDownof a loaded peer and anAnnounceto both tables, then compares every read. It catchesLoadnot counting each peer's prefixes.ReadRouteTablefails on such a row, tested byTestReadRouteTableFailsOnAPeerWithoutANumber. The rule inLoadand its test are gone.Judgement call: the read checks each row's whole peer set against the set of stored numbers at once, instead of looking up each peer, so that startup stays fast.
Model: opus-5-5
PASS: both findings of the first review (#186 (comment)) are fixed, and nothing else blocks the merge into
next.internal/routetablepackage doc as coveringLoad, as the first review did.routesandroute_peers; I took step 5 of the plan (#122 (comment)) as where they go.WriteRouteChangesdoes not say that a failed write's changes must be written again before any later ones; written after a later write, they put older rows back without an error. I left that rule to step 3.Model: opus-5-5