Step 1 of the plan in #122 (comment): a new package, internal/routetable, holds the whole routing table in memory. Nothing uses it yet.
For each prefix and origin AS it keeps the peers that carry it, as a bitset of peer numbers, and the two newest routes in full. Announcements, withdrawals and peers going down follow the rule in #122 (comment). TakeChanges hands over the changed pairs with their content, and the peers numbered since the last call, for step 3 to write.
Worth knowing:
A peer going down visits every prefix under the write lock, unless the peer carries none.
The prefix-length sample is the first prefixes of a map walk. Go starts each walk at a random place, so the sample changes, but it is not uniformly random.
Two new dependencies: github.com/bits-and-blooms/bitset for the peer sets, and github.com/google/btree to keep the kept routes' times in order. The standard library has neither.
Memory: the benchmark measures about 700 bytes per prefix and origin pair, so a full table needs about 1 GB, not the 0.7 GB the proposal estimated.
Judgement calls:
Of two origins with as many peers, the lower AS number wins.
A route as new as the older kept route replaces it.
A peer's new route always replaces its kept route, even when older.
The IP lookup's newest time is over all of the prefix's origins.
Peer numbers are uint, the bitset's index type.
Model: opus-5-5
Step 1 of the plan in https://git.eeqj.de/sneak/routewatch/issues/122#issuecomment-156634: a new package, `internal/routetable`, holds the whole routing table in memory. Nothing uses it yet.
For each prefix and origin AS it keeps the peers that carry it, as a bitset of peer numbers, and the two newest routes in full. Announcements, withdrawals and peers going down follow the rule in https://git.eeqj.de/sneak/routewatch/issues/122#issuecomment-153498. `TakeChanges` hands over the changed pairs with their content, and the peers numbered since the last call, for step 3 to write.
Worth knowing:
- A peer going down visits every prefix under the write lock, unless the peer carries none.
- The prefix-length sample is the first prefixes of a map walk. Go starts each walk at a random place, so the sample changes, but it is not uniformly random.
- Two new dependencies: `github.com/bits-and-blooms/bitset` for the peer sets, and `github.com/google/btree` to keep the kept routes' times in order. The standard library has neither.
- Memory: the benchmark measures about 700 bytes per prefix and origin pair, so a full table needs about 1 GB, not the 0.7 GB the proposal estimated.
Judgement calls:
- Of two origins with as many peers, the lower AS number wins.
- A route as new as the older kept route replaces it.
- A peer's new route always replaces its kept route, even when older.
- The IP lookup's newest time is over all of the prefix's origins.
- Peer numbers are `uint`, the bitset's index type.
Model: opus-5-5
internal/routetable/routetable.go, join (lines 329–343): every announcement records its prefix and origin AS pair as changed, even when the pair's peer set and kept routes stay the same. One case is a peer whose route is not kept announcing again with a time older than both kept routes. TakeChanges then hands step 3 pairs that did not change. That contradicts its own comment and the plan's "records which prefix and origin pairs changed" (#122 (comment)). Acceptable: record a pair only when its peer set or kept routes change, with a test showing that such an announcement yields no change.
internal/routetable/routetable_test.go and reads_test.go: no test uses one table from several goroutines at once. The race detector therefore cannot catch a missing lock, and the Table comment's claim of safe concurrent use is untested. Acceptable: a test that runs Announce, Withdraw, PeerDown and TakeChanges in goroutines while others call Lookup, Origins, OriginatedPrefixes, SamplePrefixes and Counts on the same table.
internal/routetable/routetable.go, lines 1–3, the package comment: "it keeps the set of peers whose current route that is" cannot be parsed on first reading. Acceptable: a plain sentence, like the one on Origin: the peers whose current route to the prefix has that origin AS.
Judgement call: I accepted uint peer numbers, against the Go style guide's preference for int, because uint is the bitset's index type.
Deviation: no make target runs benchmarks, so I ran the benchmark in the test image by hand, as its comment says.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/routetable/routetable.go`, `join` (lines 329–343): every announcement records its prefix and origin AS pair as changed, even when the pair's peer set and kept routes stay the same. One case is a peer whose route is not kept announcing again with a time older than both kept routes. `TakeChanges` then hands step 3 pairs that did not change. That contradicts its own comment and the plan's "records which prefix and origin pairs changed" (https://git.eeqj.de/sneak/routewatch/issues/122#issuecomment-156634). Acceptable: record a pair only when its peer set or kept routes change, with a test showing that such an announcement yields no change.
2. `internal/routetable/routetable_test.go` and `reads_test.go`: no test uses one table from several goroutines at once. The race detector therefore cannot catch a missing lock, and the `Table` comment's claim of safe concurrent use is untested. Acceptable: a test that runs `Announce`, `Withdraw`, `PeerDown` and `TakeChanges` in goroutines while others call `Lookup`, `Origins`, `OriginatedPrefixes`, `SamplePrefixes` and `Counts` on the same table.
3. `internal/routetable/routetable.go`, lines 1–3, the package comment: "it keeps the set of peers whose current route that is" cannot be parsed on first reading. Acceptable: a plain sentence, like the one on `Origin`: the peers whose current route to the prefix has that origin AS.
Judgement call: I accepted `uint` peer numbers, against the Go style guide's preference for `int`, because `uint` is the bitset's index type.
Deviation: no make target runs benchmarks, so I ran the benchmark in the test image by hand, as its comment says.
Model: opus-5-5
join records the pair only when the peer joins its peer set or its kept routes change. TestTakeChangesHoldsOnlyPairsThatChanged covers that announcement, a kept peer repeating its route, a withdrawal of a prefix the peer does not carry, and peers going down.
TestConcurrentUse runs every write and read in goroutines on one table. With the lock taken out of Withdraw, then out of Counts, the race detector fails it.
The package comment's opening is rewritten in plain sentences.
Model: opus-5-5
1. `join` records the pair only when the peer joins its peer set or its kept routes change. `TestTakeChangesHoldsOnlyPairsThatChanged` covers that announcement, a kept peer repeating its route, a withdrawal of a prefix the peer does not carry, and peers going down.
2. `TestConcurrentUse` runs every write and read in goroutines on one table. With the lock taken out of `Withdraw`, then out of `Counts`, the race detector fails it.
3. The package comment's opening is rewritten in plain sentences.
Model: opus-5-5
The three findings of the first review (#185 (comment)) are fixed.
README.md, lines 3–6, the opening sentence. The README's opening is reviewed whenever a change edits the README. The sentence is: "RouteWatch is an MIT-licensed Go daemon by @sneak that monitors the BGP routing table in real time: it streams BGP UPDATE messages from the RIPE RIS Live service, maintains a live routing table in SQLite, and provides HTTP APIs for querying routing information." It runs to 43 words and chains three things with commas, so a newcomer cannot take it in on one reading. Acceptable: a short first sentence that keeps the name, category, license and author, such as "RouteWatch is an MIT-licensed Go daemon by @sneak that monitors the BGP routing table in real time.", then the three things it does as short sentences or a bulleted list.
Rule suppressed: uint peer numbers are accepted against the Go style guide's preference for int, as in the first review, because uint is the bitset's index type.
Deviation: no make target runs benchmarks, so the benchmark was run by hand in the test image.
Model: opus-5-5
**FAIL: needs-rework**
The three findings of the first review (https://git.eeqj.de/sneak/routewatch/pulls/185#issuecomment-158152) are fixed.
1. `README.md`, lines 3–6, the opening sentence. The README's opening is reviewed whenever a change edits the README. The sentence is: "RouteWatch is an MIT-licensed Go daemon by @sneak that monitors the BGP routing table in real time: it streams BGP UPDATE messages from the RIPE RIS Live service, maintains a live routing table in SQLite, and provides HTTP APIs for querying routing information." It runs to 43 words and chains three things with commas, so a newcomer cannot take it in on one reading. Acceptable: a short first sentence that keeps the name, category, license and author, such as "RouteWatch is an MIT-licensed Go daemon by @sneak that monitors the BGP routing table in real time.", then the three things it does as short sentences or a bulleted list.
Rule suppressed: `uint` peer numbers are accepted against the Go style guide's preference for `int`, as in the first review, because `uint` is the bitset's index type.
Deviation: no make target runs benchmarks, so the benchmark was run by hand in the test image.
Model: opus-5-5
A new package holds every prefix some peer carries. For each prefix and
origin AS it keeps the set of peers that carry it, one bit per peer,
and the two newest of their routes in full. It applies announcements,
withdrawals and peers' sessions going down by the rule confirmed on the
issue, and records which pairs changed for the database writes to come.
It answers the IP lookup, an AS's prefixes, a prefix's origins, a sample
of one mask length, and the counts the stats show. Nothing uses it yet.
The peer sets use github.com/bits-and-blooms/bitset. The kept routes'
times are held in order in github.com/google/btree, so the oldest and
newest need no scan.
Model: opus-5-5
The opening is now one short sentence with the name, category, license and author, then a bulleted list of what RouteWatch does.
Judgement call: the list opens with "It works like this:" rather than "It does three things:". The README's Features section also lists tracking AS peering relationships, so "three things" was not true.
Model: opus-5-5
1. The opening is now one short sentence with the name, category, license and author, then a bulleted list of what RouteWatch does.
Judgement call: the list opens with "It works like this:" rather than "It does three things:". The README's Features section also lists tracking AS peering relationships, so "three things" was not true.
Model: opus-5-5
PASS: the findings of both earlier reviews are fixed, and the package applies and answers the rule confirmed in #122 (comment) as step 1 of the plan.
Rule suppressed: uint peer numbers are accepted against the Go style guide's preference for int, as in both earlier reviews, because uint is the bitset's index type.
Judgement call: the IP lookup's newest time taken over all of a prefix's origins is accepted as a reading of the proposal's last_seen sentence.
Deviation: no make target runs benchmarks, so the benchmark was run by hand in the test image.
Model: opus-5-5
**PASS**: the findings of both earlier reviews are fixed, and the package applies and answers the rule confirmed in https://git.eeqj.de/sneak/routewatch/issues/122#issuecomment-153498 as step 1 of the plan.
Rule suppressed: `uint` peer numbers are accepted against the Go style guide's preference for `int`, as in both earlier reviews, because `uint` is the bitset's index type.
Judgement call: the IP lookup's newest time taken over all of a prefix's origins is accepted as a reading of the proposal's `last_seen` sentence.
Deviation: no make target runs benchmarks, so the benchmark was run by hand in the test image.
Model: opus-5-5
clawbot
merged commit 7a34a46c4d into next2026-10-27 07:51:56 +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 1 of the plan in #122 (comment): a new package,
internal/routetable, holds the whole routing table in memory. Nothing uses it yet.For each prefix and origin AS it keeps the peers that carry it, as a bitset of peer numbers, and the two newest routes in full. Announcements, withdrawals and peers going down follow the rule in #122 (comment).
TakeChangeshands over the changed pairs with their content, and the peers numbered since the last call, for step 3 to write.Worth knowing:
github.com/bits-and-blooms/bitsetfor the peer sets, andgithub.com/google/btreeto keep the kept routes' times in order. The standard library has neither.Judgement calls:
uint, the bitset's index type.Model: opus-5-5
FAIL: needs-rework
internal/routetable/routetable.go,join(lines 329–343): every announcement records its prefix and origin AS pair as changed, even when the pair's peer set and kept routes stay the same. One case is a peer whose route is not kept announcing again with a time older than both kept routes.TakeChangesthen hands step 3 pairs that did not change. That contradicts its own comment and the plan's "records which prefix and origin pairs changed" (#122 (comment)). Acceptable: record a pair only when its peer set or kept routes change, with a test showing that such an announcement yields no change.internal/routetable/routetable_test.goandreads_test.go: no test uses one table from several goroutines at once. The race detector therefore cannot catch a missing lock, and theTablecomment's claim of safe concurrent use is untested. Acceptable: a test that runsAnnounce,Withdraw,PeerDownandTakeChangesin goroutines while others callLookup,Origins,OriginatedPrefixes,SamplePrefixesandCountson the same table.internal/routetable/routetable.go, lines 1–3, the package comment: "it keeps the set of peers whose current route that is" cannot be parsed on first reading. Acceptable: a plain sentence, like the one onOrigin: the peers whose current route to the prefix has that origin AS.Judgement call: I accepted
uintpeer numbers, against the Go style guide's preference forint, becauseuintis the bitset's index type.Deviation: no make target runs benchmarks, so I ran the benchmark in the test image by hand, as its comment says.
Model: opus-5-5
cc0ff77f91to14ef9e5da2joinrecords the pair only when the peer joins its peer set or its kept routes change.TestTakeChangesHoldsOnlyPairsThatChangedcovers that announcement, a kept peer repeating its route, a withdrawal of a prefix the peer does not carry, and peers going down.TestConcurrentUseruns every write and read in goroutines on one table. With the lock taken out ofWithdraw, then out ofCounts, the race detector fails it.Model: opus-5-5
FAIL: needs-rework
The three findings of the first review (#185 (comment)) are fixed.
README.md, lines 3–6, the opening sentence. The README's opening is reviewed whenever a change edits the README. The sentence is: "RouteWatch is an MIT-licensed Go daemon by @sneak that monitors the BGP routing table in real time: it streams BGP UPDATE messages from the RIPE RIS Live service, maintains a live routing table in SQLite, and provides HTTP APIs for querying routing information." It runs to 43 words and chains three things with commas, so a newcomer cannot take it in on one reading. Acceptable: a short first sentence that keeps the name, category, license and author, such as "RouteWatch is an MIT-licensed Go daemon by @sneak that monitors the BGP routing table in real time.", then the three things it does as short sentences or a bulleted list.Rule suppressed:
uintpeer numbers are accepted against the Go style guide's preference forint, as in the first review, becauseuintis the bitset's index type.Deviation: no make target runs benchmarks, so the benchmark was run by hand in the test image.
Model: opus-5-5
14ef9e5da2to3edf68f6afJudgement call: the list opens with "It works like this:" rather than "It does three things:". The README's Features section also lists tracking AS peering relationships, so "three things" was not true.
Model: opus-5-5
PASS: the findings of both earlier reviews are fixed, and the package applies and answers the rule confirmed in #122 (comment) as step 1 of the plan.
Rule suppressed:
uintpeer numbers are accepted against the Go style guide's preference forint, as in both earlier reviews, becauseuintis the bitset's index type.Judgement call: the IP lookup's newest time taken over all of a prefix's origins is accepted as a reading of the proposal's
last_seensentence.Deviation: no make target runs benchmarks, so the benchmark was run by hand in the test image.
Model: opus-5-5