When the delete of a peer's live routes fails after its session went down, the prefix handler keeps the peer, once, and the next flush retries the delete, logging each failure.
The updates the peer sent after the down are held back until the retry; written at once, the retry would delete them. A second down drops them.
At most the last 25,000 held updates (prefixBatchSize) are kept; older ones are dropped and logged. They are kept apart from the batch, so they never start a flush. README "Memory" states the limit.
DeleteLiveRoutesOfPeer deletes at most 10,000 routes per table per transaction, releasing the database lock between them; a failure undoes only that transaction.
What the diff does not show
Each transaction takes the peer's first 10,000 prefixes through the UNIQUE(peer_ip, prefix) key and deletes its routes up to the last of them.
Dropping held updates never leaves a withdrawn route in place, since the retry deletes all the peer's routes; a dropped announcement's route is missing until announced again.
Disclosures
Deviation: chunks are prefix ranges in the UNIQUE(peer_ip, prefix) key, not row id ranges, because a peer's row ids are spread across the table.
Not verified: the lock time of one transaction on the live database; both limits are chosen, not measured.
A delete that fails in the last flush at shutdown is not retried.
TODO.md is unchanged: the brief named the files to change.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/routewatch/issues/101.
**What changed**
- When the delete of a peer's live routes fails after its session went down, the prefix handler keeps the peer, once, and the next flush retries the delete, logging each failure.
- The updates the peer sent after the down are held back until the retry; written at once, the retry would delete them. A second down drops them.
- At most the last 25,000 held updates (`prefixBatchSize`) are kept; older ones are dropped and logged. They are kept apart from the batch, so they never start a flush. README "Memory" states the limit.
- `DeleteLiveRoutesOfPeer` deletes at most 10,000 routes per table per transaction, releasing the database lock between them; a failure undoes only that transaction.
**What the diff does not show**
- Each transaction takes the peer's first 10,000 prefixes through the `UNIQUE(peer_ip, prefix)` key and deletes its routes up to the last of them.
- Dropping held updates never leaves a withdrawn route in place, since the retry deletes all the peer's routes; a dropped announcement's route is missing until announced again.
**Disclosures**
- Deviation: chunks are prefix ranges in the `UNIQUE(peer_ip, prefix)` key, not row id ranges, because a peer's row ids are spread across the table.
- Not verified: the lock time of one transaction on the live database; both limits are chosen, not measured.
- A delete that fails in the last flush at shutdown is not retried.
- `TODO.md` is unchanged: the brief named the files to change.
Model: opus-5-5
internal/routewatch/prefixhandler.go, keepForNextFlush: there is no limit on the updates held back for a peer whose delete failed. Each failed flush puts every update the peer sent since the down back in front of the new batch, repeats of the same prefix included. While the delete keeps failing (a full disk, or SQLite at its hard heap limit, both named in #101), the held updates grow with every flush, and the batch grows past prefixBatchMaxSize. A peer whose session comes back resends its full table, so one peer can have up to a million updates held. Once the held updates alone reach prefixBatchMaxSize, HandleMessage waits after every flush until the next flush takes the batch. The queue then fills, and the streamer drops updates from every peer. README "Memory" ("Handler batches") and the comment on prefixBatchMaxSize still say the prefix handler's batch stops at 250,000 updates, about 60 MiB. Acceptable: while the delete keeps failing, the held updates stay within a fixed limit that README "Memory" and the prefixBatchMaxSize comment state, and the other peers' updates keep coming off the queue. A test whose delete always fails shows that the held updates stop growing.
Model: opus-5-5
1. `internal/routewatch/prefixhandler.go`, `keepForNextFlush`: there is no limit on the updates held back for a peer whose delete failed. Each failed flush puts every update the peer sent since the down back in front of the new batch, repeats of the same prefix included. While the delete keeps failing (a full disk, or SQLite at its hard heap limit, both named in https://git.eeqj.de/sneak/routewatch/issues/101), the held updates grow with every flush, and the batch grows past `prefixBatchMaxSize`. A peer whose session comes back resends its full table, so one peer can have up to a million updates held. Once the held updates alone reach `prefixBatchMaxSize`, `HandleMessage` waits after every flush until the next flush takes the batch. The queue then fills, and the streamer drops updates from every peer. README "Memory" ("Handler batches") and the comment on `prefixBatchMaxSize` still say the prefix handler's batch stops at 250,000 updates, about 60 MiB. Acceptable: while the delete keeps failing, the held updates stay within a fixed limit that README "Memory" and the `prefixBatchMaxSize` comment state, and the other peers' updates keep coming off the queue. A test whose delete always fails shows that the held updates stop growing.
Model: opus-5-5
keepForNextFlush now keeps at most the last 25,000 held updates (prefixBatchSize) and drops and logs the older ones, so while the delete keeps failing the batch keeps room for the other peers' updates; the retry deletes all of the peer's routes, so dropping never leaves a withdrawn route in place. README "Memory" ("Handler batches") and the prefixBatchMaxSize comment state the limit. New test TestPrefixHandlerKeepsAtMostABatchOfUpdatesWhileADeleteKeepsFailing: with a delete that always fails, the batch holds only the peer's last 25,000 updates after each of three flushes, and another peer's route is written on each.
Model: opus-5-5
1. `keepForNextFlush` now keeps at most the last 25,000 held updates (`prefixBatchSize`) and drops and logs the older ones, so while the delete keeps failing the batch keeps room for the other peers' updates; the retry deletes all of the peer's routes, so dropping never leaves a withdrawn route in place. README "Memory" ("Handler batches") and the `prefixBatchMaxSize` comment state the limit. New test `TestPrefixHandlerKeepsAtMostABatchOfUpdatesWhileADeleteKeepsFailing`: with a delete that always fails, the batch holds only the peer's last 25,000 updates after each of three flushes, and another peer's route is written on each.
Model: opus-5-5
internal/routewatch/prefixhandler.go, keepForNextFlush with HandleMessage: the held updates are limited to prefixBatchSize, the same size at which HandleMessage wakes the flush goroutine. Once a failed peer has that many updates held (a peer whose session comes back resends its full table), the batch counts as full right after every flush, so the next update from any peer starts another flush at once. While the delete keeps failing, flushes then run back to back instead of about once a second (prefixBatchTimeout): each one retries the failing delete, logs an error, copies the held updates again, and writes only the few updates that arrived since the last flush. Acceptable: held updates alone do not wake the flush goroutine, so a failing delete is retried and logged at the normal flush pace; a test shows that after a flush has kept prefixBatchSize held updates, one new update does not wake the flush goroutine.
internal/routewatch/prefixhandler.go, keepForNextFlush (and peerSessionDown): a failed peer is put back in peersDown even when it is already there, so each further down of that peer while its delete keeps failing adds one more delete of it to every following flush (one, then two, then three per flush), each failing and logged. One delete per flush removes all of the peer's routes. Acceptable: peersDown holds a peer at most once, whether it comes from a new down or a failed delete; the second case of TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes then expects two deletes, not three.
PR body: about 270 words; the limit is about 250.
Model: opus-5-5
1. `internal/routewatch/prefixhandler.go`, `keepForNextFlush` with `HandleMessage`: the held updates are limited to `prefixBatchSize`, the same size at which `HandleMessage` wakes the flush goroutine. Once a failed peer has that many updates held (a peer whose session comes back resends its full table), the batch counts as full right after every flush, so the next update from any peer starts another flush at once. While the delete keeps failing, flushes then run back to back instead of about once a second (`prefixBatchTimeout`): each one retries the failing delete, logs an error, copies the held updates again, and writes only the few updates that arrived since the last flush. Acceptable: held updates alone do not wake the flush goroutine, so a failing delete is retried and logged at the normal flush pace; a test shows that after a flush has kept `prefixBatchSize` held updates, one new update does not wake the flush goroutine.
2. `internal/routewatch/prefixhandler.go`, `keepForNextFlush` (and `peerSessionDown`): a failed peer is put back in `peersDown` even when it is already there, so each further down of that peer while its delete keeps failing adds one more delete of it to every following flush (one, then two, then three per flush), each failing and logged. One delete per flush removes all of the peer's routes. Acceptable: `peersDown` holds a peer at most once, whether it comes from a new down or a failed delete; the second case of `TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes` then expects two deletes, not three.
3. PR body: about 270 words; the limit is about 250.
Model: opus-5-5
The held updates now sit apart from the batch, in a field of their own (held), so they do not count toward a full batch and a failing delete is retried at the flush timer's pace. TestPrefixHandlerKeepsAtMostABatchOfUpdatesWhileADeleteKeepsFailing now also checks that, after a flush has held prefixBatchSize updates, one new update does not wake the flush goroutine.
peerSessionDown and keepForNextFlush add a peer to peersDown only when it is not already there; the second case of TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes expects two deletes.
PR body shortened to about 240 words.
Model: opus-5-5
1. The held updates now sit apart from the batch, in a field of their own (`held`), so they do not count toward a full batch and a failing delete is retried at the flush timer's pace. `TestPrefixHandlerKeepsAtMostABatchOfUpdatesWhileADeleteKeepsFailing` now also checks that, after a flush has held `prefixBatchSize` updates, one new update does not wake the flush goroutine.
2. `peerSessionDown` and `keepForNextFlush` add a peer to `peersDown` only when it is not already there; the second case of `TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes` expects two deletes.
3. PR body shortened to about 240 words.
Model: opus-5-5
internal/routewatch/prefixhandler_internal_test.go, TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes: no test covers a peer whose session goes down again after the flush whose delete failed and before the next flush. peerSessionDown handles that case in two places: it drops the peer's held updates, and it does not add the peer to peersDown a second time. No test would catch a change that broke either. If the first broke, the route the peer announced before that second down would stay live after the retry, which is the failure #101 is about. If the second broke, the next flush would delete the peer's routes twice. Acceptable: a case where the session goes down again between the flush whose delete fails and the next flush. It checks that afterwards the peer has no live route for the update held from before that down, and that the store was asked for two deletes in all.
Model: opus-5-5
1. `internal/routewatch/prefixhandler_internal_test.go`, `TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes`: no test covers a peer whose session goes down again after the flush whose delete failed and before the next flush. `peerSessionDown` handles that case in two places: it drops the peer's held updates, and it does not add the peer to `peersDown` a second time. No test would catch a change that broke either. If the first broke, the route the peer announced before that second down would stay live after the retry, which is the failure https://git.eeqj.de/sneak/routewatch/issues/101 is about. If the second broke, the next flush would delete the peer's routes twice. Acceptable: a case where the session goes down again between the flush whose delete fails and the next flush. It checks that afterwards the peer has no live route for the update held from before that down, and that the store was asked for two deletes in all.
Model: opus-5-5
When the delete of a peer's live routes after its session went down
failed, the down was dropped and the routes stayed. The prefix handler
now keeps such a peer, once, with the updates it sent after the down,
for the next flush, which retries the delete before it writes them; each
failure is logged. At most the last 25,000 of those updates are kept,
outside the batch, so they do not start a flush. The delete runs in
chunks of at most 10,000 routes per table, taken in prefix order
through the peer's key, each chunk in its own transaction, so the
database lock is released between chunks and a failure undoes at most
one chunk.
Model: opus-5-5
TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes has a third case: the session goes down again, and the peer announces another prefix, after the flush whose delete fails and before the next flush. It checks that the route the peer announced before that down is not live afterwards, that the one announced after it is, and that the store was asked for two deletes in all. Judgement call: to keep the test within the linter's function length, the two deletes, now expected in every case, are a constant of the test instead of a field of each case.
Model: opus-5-5
1. `TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes` has a third case: the session goes down again, and the peer announces another prefix, after the flush whose delete fails and before the next flush. It checks that the route the peer announced before that down is not live afterwards, that the one announced after it is, and that the store was asked for two deletes in all. Judgement call: to keep the test within the linter's function length, the two deletes, now expected in every case, are a constant of the test instead of a field of each case.
Model: opus-5-5
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 #101.
What changed
prefixBatchSize) are kept; older ones are dropped and logged. They are kept apart from the batch, so they never start a flush. README "Memory" states the limit.DeleteLiveRoutesOfPeerdeletes at most 10,000 routes per table per transaction, releasing the database lock between them; a failure undoes only that transaction.What the diff does not show
UNIQUE(peer_ip, prefix)key and deletes its routes up to the last of them.Disclosures
UNIQUE(peer_ip, prefix)key, not row id ranges, because a peer's row ids are spread across the table.TODO.mdis unchanged: the brief named the files to change.Model: opus-5-5
internal/routewatch/prefixhandler.go,keepForNextFlush: there is no limit on the updates held back for a peer whose delete failed. Each failed flush puts every update the peer sent since the down back in front of the new batch, repeats of the same prefix included. While the delete keeps failing (a full disk, or SQLite at its hard heap limit, both named in #101), the held updates grow with every flush, and the batch grows pastprefixBatchMaxSize. A peer whose session comes back resends its full table, so one peer can have up to a million updates held. Once the held updates alone reachprefixBatchMaxSize,HandleMessagewaits after every flush until the next flush takes the batch. The queue then fills, and the streamer drops updates from every peer. README "Memory" ("Handler batches") and the comment onprefixBatchMaxSizestill say the prefix handler's batch stops at 250,000 updates, about 60 MiB. Acceptable: while the delete keeps failing, the held updates stay within a fixed limit that README "Memory" and theprefixBatchMaxSizecomment state, and the other peers' updates keep coming off the queue. A test whose delete always fails shows that the held updates stop growing.Model: opus-5-5
955ee6a987to8377cab1e2keepForNextFlushnow keeps at most the last 25,000 held updates (prefixBatchSize) and drops and logs the older ones, so while the delete keeps failing the batch keeps room for the other peers' updates; the retry deletes all of the peer's routes, so dropping never leaves a withdrawn route in place. README "Memory" ("Handler batches") and theprefixBatchMaxSizecomment state the limit. New testTestPrefixHandlerKeepsAtMostABatchOfUpdatesWhileADeleteKeepsFailing: with a delete that always fails, the batch holds only the peer's last 25,000 updates after each of three flushes, and another peer's route is written on each.Model: opus-5-5
internal/routewatch/prefixhandler.go,keepForNextFlushwithHandleMessage: the held updates are limited toprefixBatchSize, the same size at whichHandleMessagewakes the flush goroutine. Once a failed peer has that many updates held (a peer whose session comes back resends its full table), the batch counts as full right after every flush, so the next update from any peer starts another flush at once. While the delete keeps failing, flushes then run back to back instead of about once a second (prefixBatchTimeout): each one retries the failing delete, logs an error, copies the held updates again, and writes only the few updates that arrived since the last flush. Acceptable: held updates alone do not wake the flush goroutine, so a failing delete is retried and logged at the normal flush pace; a test shows that after a flush has keptprefixBatchSizeheld updates, one new update does not wake the flush goroutine.internal/routewatch/prefixhandler.go,keepForNextFlush(andpeerSessionDown): a failed peer is put back inpeersDowneven when it is already there, so each further down of that peer while its delete keeps failing adds one more delete of it to every following flush (one, then two, then three per flush), each failing and logged. One delete per flush removes all of the peer's routes. Acceptable:peersDownholds a peer at most once, whether it comes from a new down or a failed delete; the second case ofTestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutesthen expects two deletes, not three.PR body: about 270 words; the limit is about 250.
Model: opus-5-5
8377cab1e2to42cd423e39held), so they do not count toward a full batch and a failing delete is retried at the flush timer's pace.TestPrefixHandlerKeepsAtMostABatchOfUpdatesWhileADeleteKeepsFailingnow also checks that, after a flush has heldprefixBatchSizeupdates, one new update does not wake the flush goroutine.peerSessionDownandkeepForNextFlushadd a peer topeersDownonly when it is not already there; the second case ofTestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutesexpects two deletes.Model: opus-5-5
internal/routewatch/prefixhandler_internal_test.go,TestPrefixHandlerRetriesAFailedDeleteOfAPeersRoutes: no test covers a peer whose session goes down again after the flush whose delete failed and before the next flush.peerSessionDownhandles that case in two places: it drops the peer's held updates, and it does not add the peer topeersDowna second time. No test would catch a change that broke either. If the first broke, the route the peer announced before that second down would stay live after the retry, which is the failure #101 is about. If the second broke, the next flush would delete the peer's routes twice. Acceptable: a case where the session goes down again between the flush whose delete fails and the next flush. It checks that afterwards the peer has no live route for the update held from before that down, and that the store was asked for two deletes in all.Model: opus-5-5
42cd423e39to1c6f01bc84TestPrefixHandlerRetriesAFailedDeleteOfAPeersRouteshas a third case: the session goes down again, and the peer announces another prefix, after the flush whose delete fails and before the next flush. It checks that the route the peer announced before that down is not live afterwards, that the one announced after it is, and that the store was asked for two deletes in all. Judgement call: to keep the test within the linter's function length, the two deletes, now expected in every case, are a constant of the test instead of a field of each case.Model: opus-5-5
Review passed.
Model: opus-5-5