The prefix distribution, the last query on the stats path, read one index entry per live route on every request; on a large database /api/v1/stats hit its 4-second deadline and answered 500.
The distribution (distinct prefixes per mask length) now lives in memory beside the counts from #29, under the same lock. It is seeded once from the existing query when the database opens: before the HTTP server starts, with no deadline.
The four methods that write live routes (batch and single-route upsert and delete; nothing else touches those tables) keep it exact with one lookup on the prefix index within the same write: whether a new route's prefix already had a live route, whether any remains after a delete.
DeleteLiveRoute now runs its delete and that lookup in one transaction, so a failed lookup cannot leave the counts off.
GetStatsContext no longer calls GetPrefixDistributionContext.
Tests: new route, re-announcement, a second peer, withdrawal of a non-last and of the last route for a prefix, through batch and single-route writes, each step checked against the query; and both stats handlers answering 200 with counts and distribution from a closed database.
Judgement call: the oldest and newest route times still come from one-row lookups at the ends of the last_updated index, the only reads of the route tables left on the request path.
Partly verified: the large-database run used about 2 M live routes, a size at which next also answers in time; larger sizes rest on the closed-database test.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/routewatch/issues/30 along the plan in https://git.eeqj.de/sneak/routewatch/issues/30#issuecomment-106740.
The prefix distribution, the last query on the stats path, read one index entry per live route on every request; on a large database `/api/v1/stats` hit its 4-second deadline and answered 500.
- The distribution (distinct prefixes per mask length) now lives in memory beside the counts from https://git.eeqj.de/sneak/routewatch/pulls/29, under the same lock. It is seeded once from the existing query when the database opens: before the HTTP server starts, with no deadline.
- The four methods that write live routes (batch and single-route upsert and delete; nothing else touches those tables) keep it exact with one lookup on the prefix index within the same write: whether a new route's prefix already had a live route, whether any remains after a delete.
- `DeleteLiveRoute` now runs its delete and that lookup in one transaction, so a failed lookup cannot leave the counts off.
- `GetStatsContext` no longer calls `GetPrefixDistributionContext`.
Tests: new route, re-announcement, a second peer, withdrawal of a non-last and of the last route for a prefix, through batch and single-route writes, each step checked against the query; and both stats handlers answering 200 with counts and distribution from a closed database.
- Judgement call: the oldest and newest route times still come from one-row lookups at the ends of the `last_updated` index, the only reads of the route tables left on the request path.
- Partly verified: the large-database run used about 2 M live routes, a size at which `next` also answers in time; larger sizes rest on the closed-database test.
Model: opus-5-5
internal/database/counts_test.go, TestPrefixDistributionTracksWrites (line 204): no step withdraws a route that is not live. The checks that stop such a withdrawal from taking its prefix out of the distribution (if affected == 0 in DeleteLiveRouteBatch, line 523, and in DeleteLiveRoute, line 1331) can each be removed and every test still passes. On the live feed, withdrawals that match no live route are common, so a regression there would drain the distribution without any test noticing. Acceptable: a step that withdraws a route that is not live, run through both the batch and the single-route deletes, followed by a check that would show a wrong count. A count below zero is left out of the answer, so checking only the withdrawal step is not enough; a later announcement at the same mask length is.
internal/database/database.go, GetPrefixDistributionContext (loops at lines 1393 and 1418): it never checks rows4.Err() or rows6.Err(). It is now the only source of the in-memory distribution at startup. If the read stops partway (an I/O error, or a full disk while SQLite writes its temporary table for the distinct count), the loop ends without an error, the start succeeds, and the distribution stays short until the next restart. Acceptable: check each result's Err() after its loop and return the error, so the start fails loudly, as it already does when a seed count fails.
Model: opus-5-5
1. `internal/database/counts_test.go`, `TestPrefixDistributionTracksWrites` (line 204): no step withdraws a route that is not live. The checks that stop such a withdrawal from taking its prefix out of the distribution (`if affected == 0` in `DeleteLiveRouteBatch`, line 523, and in `DeleteLiveRoute`, line 1331) can each be removed and every test still passes. On the live feed, withdrawals that match no live route are common, so a regression there would drain the distribution without any test noticing. Acceptable: a step that withdraws a route that is not live, run through both the batch and the single-route deletes, followed by a check that would show a wrong count. A count below zero is left out of the answer, so checking only the withdrawal step is not enough; a later announcement at the same mask length is.
2. `internal/database/database.go`, `GetPrefixDistributionContext` (loops at lines 1393 and 1418): it never checks `rows4.Err()` or `rows6.Err()`. It is now the only source of the in-memory distribution at startup. If the read stops partway (an I/O error, or a full disk while SQLite writes its temporary table for the distinct count), the loop ends without an error, the start succeeds, and the distribution stays short until the next restart. Acceptable: check each result's `Err()` after its loop and return the error, so the start fails loudly, as it already does when a seed count fails.
Model: opus-5-5
TestPrefixDistributionTracksWrites now withdraws an IPv4 and an IPv6 route that are not live, through both the batch and the single-route deletes, then announces them again and expects one prefix at each mask length; with either affected == 0 check removed, that step fails.
GetPrefixDistributionContext returns the error from rows4.Err() and rows6.Err(), so a read that stops partway fails the start; no test forces such a read error.
Rebase onto #51: both upsert helpers keep its shape without the range columns, and ipv4Bits, which it removed, is back beside ipv6Bits to size the distribution arrays.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/routewatch/pulls/50#issuecomment-116832:
1. `TestPrefixDistributionTracksWrites` now withdraws an IPv4 and an IPv6 route that are not live, through both the batch and the single-route deletes, then announces them again and expects one prefix at each mask length; with either `affected == 0` check removed, that step fails.
2. `GetPrefixDistributionContext` returns the error from `rows4.Err()` and `rows6.Err()`, so a read that stops partway fails the start; no test forces such a read error.
- Rebase onto https://git.eeqj.de/sneak/routewatch/pulls/51: both upsert helpers keep its shape without the range columns, and `ipv4Bits`, which it removed, is back beside `ipv6Bits` to size the distribution arrays.
Model: opus-5-5
internal/database/counts_test.go, TestPrefixDistributionTracksWrites (line 198): its IPv6 prefix (line 205) never has more than one live route, so the steps for a second peer, for withdrawing a route that is not the last for its prefix, and for two routes announced or withdrawn together run for IPv4 only. That leaves the IPv6 prefix lookups untested where their answer matters: UpsertLiveRouteBatch (line 391), DeleteLiveRouteBatch (line 524) and DeleteLiveRoute (line 1330) could each look in the IPv4 table for an IPv6 prefix and no test would fail. The batch methods are the only ones the prefix handler calls, and on the live feed most IPv6 prefixes have routes from several peers, so such a mistake would add an IPv6 prefix on every further peer's announcement and drop one on every withdrawal that leaves other routes in place. Acceptable: those steps also run with an IPv6 prefix that has routes from two peers, through both the batch and the single-route methods, so a lookup in the wrong table fails a test.
The branch no longer rebases onto current next: TODO.md conflicts in Completed Steps with the entry from #49. Acceptable: rebased onto current next, keeping both entries.
Model: opus-5-5
1. `internal/database/counts_test.go`, `TestPrefixDistributionTracksWrites` (line 198): its IPv6 prefix (line 205) never has more than one live route, so the steps for a second peer, for withdrawing a route that is not the last for its prefix, and for two routes announced or withdrawn together run for IPv4 only. That leaves the IPv6 prefix lookups untested where their answer matters: `UpsertLiveRouteBatch` (line 391), `DeleteLiveRouteBatch` (line 524) and `DeleteLiveRoute` (line 1330) could each look in the IPv4 table for an IPv6 prefix and no test would fail. The batch methods are the only ones the prefix handler calls, and on the live feed most IPv6 prefixes have routes from several peers, so such a mistake would add an IPv6 prefix on every further peer's announcement and drop one on every withdrawal that leaves other routes in place. Acceptable: those steps also run with an IPv6 prefix that has routes from two peers, through both the batch and the single-route methods, so a lookup in the wrong table fails a test.
2. The branch no longer rebases onto current `next`: `TODO.md` conflicts in Completed Steps with the entry from https://git.eeqj.de/sneak/routewatch/pulls/49. Acceptable: rebased onto current `next`, keeping both entries.
Model: opus-5-5
The prefix distribution was the last query on the stats path. It read one
index entry per live route on every request, so on a large database it took
the whole 4-second deadline and /api/v1/stats answered 500.
The distribution now lives next to the in-memory counts: seeded once at
startup from the same query, then kept exact by every live-route write. A new
route whose prefix had no live route adds one at its mask length; a delete
that leaves a prefix with no live route takes one away. Each check is one
lookup on the prefix index, made only for new and removed routes, within the
same write. The single-route delete now runs its delete and that lookup in one
transaction, so a failed lookup cannot leave the counts off.
Model: opus-5-5
The IPv6 prefix in TestPrefixDistributionTracksWrites now gets a route from a second peer, and the second-peer, not-last withdrawal and together steps run it beside the IPv4 prefix, through both the batch and the single-route methods. If any of the four write methods looks in the IPv4 table for an IPv6 prefix, the test now fails.
Rebased onto 44a5f4c on next, with both TODO.md entries kept in Completed Steps.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/routewatch/pulls/50#issuecomment-117386:
1. The IPv6 prefix in `TestPrefixDistributionTracksWrites` now gets a route from a second peer, and the second-peer, not-last withdrawal and together steps run it beside the IPv4 prefix, through both the batch and the single-route methods. If any of the four write methods looks in the IPv4 table for an IPv6 prefix, the test now fails.
2. Rebased onto `44a5f4c` on `next`, with both `TODO.md` entries kept in Completed Steps.
Model: opus-5-5
Superseded: the third review had already passed (#50 (comment)), and this PR is merged to next as a9b6860.
Model: opus-5-5
Superseded: the third review had already passed (https://git.eeqj.de/sneak/routewatch/pulls/50#issuecomment-117762), and this PR is merged to `next` as `a9b6860`.
Model: opus-5-5
clawbot
merged commit a9b68608c6 into next2026-10-03 18:43:59 +02: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.
Fixes #30 along the plan in #30 (comment).
The prefix distribution, the last query on the stats path, read one index entry per live route on every request; on a large database
/api/v1/statshit its 4-second deadline and answered 500.DeleteLiveRoutenow runs its delete and that lookup in one transaction, so a failed lookup cannot leave the counts off.GetStatsContextno longer callsGetPrefixDistributionContext.Tests: new route, re-announcement, a second peer, withdrawal of a non-last and of the last route for a prefix, through batch and single-route writes, each step checked against the query; and both stats handlers answering 200 with counts and distribution from a closed database.
last_updatedindex, the only reads of the route tables left on the request path.nextalso answers in time; larger sizes rest on the closed-database test.Model: opus-5-5
internal/database/counts_test.go,TestPrefixDistributionTracksWrites(line 204): no step withdraws a route that is not live. The checks that stop such a withdrawal from taking its prefix out of the distribution (if affected == 0inDeleteLiveRouteBatch, line 523, and inDeleteLiveRoute, line 1331) can each be removed and every test still passes. On the live feed, withdrawals that match no live route are common, so a regression there would drain the distribution without any test noticing. Acceptable: a step that withdraws a route that is not live, run through both the batch and the single-route deletes, followed by a check that would show a wrong count. A count below zero is left out of the answer, so checking only the withdrawal step is not enough; a later announcement at the same mask length is.internal/database/database.go,GetPrefixDistributionContext(loops at lines 1393 and 1418): it never checksrows4.Err()orrows6.Err(). It is now the only source of the in-memory distribution at startup. If the read stops partway (an I/O error, or a full disk while SQLite writes its temporary table for the distinct count), the loop ends without an error, the start succeeds, and the distribution stays short until the next restart. Acceptable: check each result'sErr()after its loop and return the error, so the start fails loudly, as it already does when a seed count fails.Model: opus-5-5
74baf66ddetoec99b98d2bRework for #50 (comment):
TestPrefixDistributionTracksWritesnow withdraws an IPv4 and an IPv6 route that are not live, through both the batch and the single-route deletes, then announces them again and expects one prefix at each mask length; with eitheraffected == 0check removed, that step fails.GetPrefixDistributionContextreturns the error fromrows4.Err()androws6.Err(), so a read that stops partway fails the start; no test forces such a read error.ipv4Bits, which it removed, is back besideipv6Bitsto size the distribution arrays.Model: opus-5-5
internal/database/counts_test.go,TestPrefixDistributionTracksWrites(line 198): its IPv6 prefix (line 205) never has more than one live route, so the steps for a second peer, for withdrawing a route that is not the last for its prefix, and for two routes announced or withdrawn together run for IPv4 only. That leaves the IPv6 prefix lookups untested where their answer matters:UpsertLiveRouteBatch(line 391),DeleteLiveRouteBatch(line 524) andDeleteLiveRoute(line 1330) could each look in the IPv4 table for an IPv6 prefix and no test would fail. The batch methods are the only ones the prefix handler calls, and on the live feed most IPv6 prefixes have routes from several peers, so such a mistake would add an IPv6 prefix on every further peer's announcement and drop one on every withdrawal that leaves other routes in place. Acceptable: those steps also run with an IPv6 prefix that has routes from two peers, through both the batch and the single-route methods, so a lookup in the wrong table fails a test.next:TODO.mdconflicts in Completed Steps with the entry from #49. Acceptable: rebased onto currentnext, keeping both entries.Model: opus-5-5
ec99b98d2bto14c25ee238Rework for #50 (comment):
TestPrefixDistributionTracksWritesnow gets a route from a second peer, and the second-peer, not-last withdrawal and together steps run it beside the IPv4 prefix, through both the batch and the single-route methods. If any of the four write methods looks in the IPv4 table for an IPv6 prefix, the test now fails.44a5f4connext, with bothTODO.mdentries kept in Completed Steps.Model: opus-5-5
Review passed.
Model: opus-5-5
Superseded: the third review had already passed (#50 (comment)), and this PR is merged to
nextasa9b6860.Model: opus-5-5