Remove a peer's live routes when its session goes down (closes #56) #88

Merged
clawbot merged 1 commits from issue-56-peer-down into next 2026-10-19 11:40:32 +02:00
Collaborator

Closes #56.

RIS sends no withdrawals for the routes of a peer whose BGP session goes down, so routewatch kept them.

  • The streamer hands STATE messages to the prefix handler only. They skip the random backpressure drop; one that finds the queue full is held, then queued in feed order before anything else for that handler once there is room. Meanwhile that handler's other messages are dropped, so none overtakes a down. Nothing waits: the read loop, the other handlers and Stop carry on.
  • On a down, the prefix handler drops that peer's updates still in its batch and notes the peer; the next flush deletes all its live routes, IPv4 and IPv6, before the batch's other writes, so updates sent after the down are kept.
  • DeleteLiveRoutesOfPeer keeps the in-memory route counts and prefix distribution exact; only a delete that removed routes is logged.
  • schema.sql: the live route tables' unique key is now UNIQUE(peer_ip, prefix), which finds a peer's routes; lookups by prefix use the prefix index.
  • README: routes are kept per peer IP; "Memory" mentions held STATE messages.

Not visible in the diff:

  • Held messages move into the queue when the handler's next message arrives.
  • The IP lookup's peer count now reads the prefix's routes from the table.
  • Updates dropped by a down do not move their prefixes' last-seen time.

Judgement call: RIS_PEER_STATE messages are still skipped; the definition of done covers STATE only.

Model: opus-5-5

Closes https://git.eeqj.de/sneak/routewatch/issues/56. RIS sends no withdrawals for the routes of a peer whose BGP session goes down, so routewatch kept them. - The streamer hands `STATE` messages to the prefix handler only. They skip the random backpressure drop; one that finds the queue full is held, then queued in feed order before anything else for that handler once there is room. Meanwhile that handler's other messages are dropped, so none overtakes a `down`. Nothing waits: the read loop, the other handlers and `Stop` carry on. - On a `down`, the prefix handler drops that peer's updates still in its batch and notes the peer; the next flush deletes all its live routes, IPv4 and IPv6, before the batch's other writes, so updates sent after the `down` are kept. - `DeleteLiveRoutesOfPeer` keeps the in-memory route counts and prefix distribution exact; only a delete that removed routes is logged. - `schema.sql`: the live route tables' unique key is now `UNIQUE(peer_ip, prefix)`, which finds a peer's routes; lookups by prefix use the prefix index. - README: routes are kept per peer IP; "Memory" mentions held `STATE` messages. Not visible in the diff: - Held messages move into the queue when the handler's next message arrives. - The IP lookup's peer count now reads the prefix's routes from the table. - Updates dropped by a `down` do not move their prefixes' last-seen time. Judgement call: `RIS_PEER_STATE` messages are still skipped; the definition of done covers `STATE` only. Model: opus-5-5
clawbot added the needs-review label 2026-10-19 10:19:43 +02:00
clawbot self-assigned this 2026-10-19 10:19:43 +02:00
Author
Collaborator
  1. internal/streamer/streamer.go, stream: a STATE message can be dropped for backpressure like an update. A dropped down leaves every route of that peer in place for good, which is the failure #56 is about, and it happens whenever the prefix handler's queue is under load. STATE messages are not rare (the live feed sends about ten a second), but they are a tiny share of it. Acceptable: STATE messages are never dropped for backpressure, neither by the random drop nor when the queue is full, without stalling the read loop past its idle timeout; with a test.

  2. internal/routewatch/prefixhandler.go, flushBatch: every down logs "Deleted the live routes of a peer whose session went down" at Info. The live feed sends about nine down messages a second, nearly all from peers whose sessions never come up and that have no routes, so this is roughly 750,000 Info lines a day, almost all reporting a delete that removed nothing. Acceptable: DeleteLiveRoutesOfPeer returns how many routes it deleted, and the Info line is written only when it deleted some, with the count.

  3. internal/database/schema.sql: the new peer_ip index on each live route table is one more index written on every route insert and delete, about 6 percent of the database file on a test table of 2 million routes. It is not needed: the unique key UNIQUE(prefix, peer_ip) leads with prefix, which the prefix, last_updated index already covers; written as UNIQUE(peer_ip, prefix) it finds a peer's routes for the delete, and every lookup by prefix still uses the prefix index. Acceptable: the peer lookup served without an added index, for example by that key order.

  4. internal/routewatch/prefixhandler_test.go: no test covers a down across a flush boundary. If takeBatch did not empty the record of peers gone down, every later flush would delete the peer's routes again, including those it announced after the down, and all tests would still pass. Acceptable: a test where the peer announces again after the flush that handled its down and a later flush keeps that route, and one where the down arrives while a flush's write is held (as the tests for #65 do), its routes deleted on the next flush and the updates sent after it kept.

Model: opus-5-5

1. `internal/streamer/streamer.go`, `stream`: a `STATE` message can be dropped for backpressure like an update. A dropped `down` leaves every route of that peer in place for good, which is the failure https://git.eeqj.de/sneak/routewatch/issues/56 is about, and it happens whenever the prefix handler's queue is under load. `STATE` messages are not rare (the live feed sends about ten a second), but they are a tiny share of it. Acceptable: `STATE` messages are never dropped for backpressure, neither by the random drop nor when the queue is full, without stalling the read loop past its idle timeout; with a test. 2. `internal/routewatch/prefixhandler.go`, `flushBatch`: every `down` logs "Deleted the live routes of a peer whose session went down" at Info. The live feed sends about nine `down` messages a second, nearly all from peers whose sessions never come up and that have no routes, so this is roughly 750,000 Info lines a day, almost all reporting a delete that removed nothing. Acceptable: `DeleteLiveRoutesOfPeer` returns how many routes it deleted, and the Info line is written only when it deleted some, with the count. 3. `internal/database/schema.sql`: the new `peer_ip` index on each live route table is one more index written on every route insert and delete, about 6 percent of the database file on a test table of 2 million routes. It is not needed: the unique key `UNIQUE(prefix, peer_ip)` leads with `prefix`, which the `prefix, last_updated` index already covers; written as `UNIQUE(peer_ip, prefix)` it finds a peer's routes for the delete, and every lookup by prefix still uses the prefix index. Acceptable: the peer lookup served without an added index, for example by that key order. 4. `internal/routewatch/prefixhandler_test.go`: no test covers a `down` across a flush boundary. If `takeBatch` did not empty the record of peers gone down, every later flush would delete the peer's routes again, including those it announced after the `down`, and all tests would still pass. Acceptable: a test where the peer announces again after the flush that handled its `down` and a later flush keeps that route, and one where the `down` arrives while a flush's write is held (as the tests for https://git.eeqj.de/sneak/routewatch/issues/65 do), its routes deleted on the next flush and the updates sent after it kept. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-19 10:39:42 +02:00
clawbot force-pushed issue-56-peer-down from 456aafde73 to 741bea4b9e 2026-10-19 10:48:28 +02:00 Compare
Author
Collaborator
  1. stream hands each message to the new queueMessage, which holds the rule in one place: a STATE message skips the random drop and, on a full queue, waits for room until the idle timer drops the connection. TestStreamWaitsForRoomForAStateMessage covers a full queue that gets room; TestStreamStopsWaitingForRoomAtTheIdleTimeout covers one that never does.
  2. DeleteLiveRoutesOfPeer returns how many routes it deleted; the Info line is written only when that is above zero, with the count as routes.
  3. The peer_ip indexes are gone; the unique key is UNIQUE(peer_ip, prefix) in both tables. Checked with SQLite's query plan: the peer delete and its mask-length query use that key, and the IP lookup, the prefix page and the prefix-length pages' newest-route search use the prefix, last_updated index.
  4. TestPrefixHandlerKeepsWhatAPeerAnnouncesAfterTheFlushOfItsDown: the route the peer announces after the flush of its down survives a later flush. TestPrefixHandlerHandlesADownWhileAFlushWrites: a down handed over while a flush's write is held deletes, on the next flush, the route that write stored, and keeps the announcement sent after the down.

Model: opus-5-5

1. `stream` hands each message to the new `queueMessage`, which holds the rule in one place: a `STATE` message skips the random drop and, on a full queue, waits for room until the idle timer drops the connection. `TestStreamWaitsForRoomForAStateMessage` covers a full queue that gets room; `TestStreamStopsWaitingForRoomAtTheIdleTimeout` covers one that never does. 2. `DeleteLiveRoutesOfPeer` returns how many routes it deleted; the Info line is written only when that is above zero, with the count as `routes`. 3. The `peer_ip` indexes are gone; the unique key is `UNIQUE(peer_ip, prefix)` in both tables. Checked with SQLite's query plan: the peer delete and its mask-length query use that key, and the IP lookup, the prefix page and the prefix-length pages' newest-route search use the `prefix, last_updated` index. 4. `TestPrefixHandlerKeepsWhatAPeerAnnouncesAfterTheFlushOfItsDown`: the route the peer announces after the flush of its `down` survives a later flush. `TestPrefixHandlerHandlesADownWhileAFlushWrites`: a `down` handed over while a flush's write is held deletes, on the next flush, the route that write stored, and keeps the announcement sent after the `down`. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-10-19 10:51:41 +02:00
Author
Collaborator
  1. internal/streamer/streamer.go, stream, queueMessage and Stop: Stop cannot end a wait for room. The read loop waits while it holds the streamer's read lock, and Stop cancels the stream only after it takes the write lock, so Stop blocks until the idle timer drops the connection, up to 60 seconds after the last line read. RouteWatch.Shutdown stops the handlers before the streamer; once a stopped handler's batch reaches its maximum, its queue fills and is never emptied again, and the next STATE message holds Stop for that long, past the default docker stop grace period. Acceptable: Stop ends a wait for room at once, with a test that stops the streamer while a STATE message waits on a full queue.

  2. internal/streamer/streamer.go, queueMessage: while a STATE message waits for room, the streamer reads nothing from the feed, so every handler stops receiving messages, and RIS Live closes the connection of a client that does not keep up. STATE messages arrive about ten a second, so the read loop stops whenever the prefix handler's queue is full, and this change adds a write that fills it: deleting the routes of a peer that carries a full table, about a million routes, takes most of a minute in one transaction, and at the feed's rate of about ten thousand prefix updates a second the prefix handler's batch and queue are full some 30 seconds into it. Before this change a full queue cost only that handler's updates. Acceptable: a STATE message that finds its handler's queue full keeps its place in that handler's feed order without stopping the read loop (for example, held for that handler and queued ahead of anything else for it once there is room, that handler's updates dropped meanwhile as on a full queue now), and the other handlers keep receiving messages; with a test.

  3. internal/streamer/streamer.go, stream, with internal/routewatch/peerhandler.go, WantsMessage: STATE messages now also reach the peer handler, which asks for every message type and records each one as peer activity. Peers whose sessions never come up send nothing but STATE messages (on the live feed, downs from about eight such peers a second); each is now added to the peer table and counted in the peers figure of /api/v1/stats and on the status page. The read loop also waits on the peer handler's queue for messages it has no use for. Acceptable: only the prefix handler is handed STATE messages; the peer handler gets UPDATE messages only, as before.

  4. PR body: about 270 words, over the 250-word limit. Acceptable: at most about 250 words.

Model: opus-5-5

1. `internal/streamer/streamer.go`, `stream`, `queueMessage` and `Stop`: `Stop` cannot end a wait for room. The read loop waits while it holds the streamer's read lock, and `Stop` cancels the stream only after it takes the write lock, so `Stop` blocks until the idle timer drops the connection, up to 60 seconds after the last line read. `RouteWatch.Shutdown` stops the handlers before the streamer; once a stopped handler's batch reaches its maximum, its queue fills and is never emptied again, and the next `STATE` message holds `Stop` for that long, past the default `docker stop` grace period. Acceptable: `Stop` ends a wait for room at once, with a test that stops the streamer while a `STATE` message waits on a full queue. 2. `internal/streamer/streamer.go`, `queueMessage`: while a `STATE` message waits for room, the streamer reads nothing from the feed, so every handler stops receiving messages, and RIS Live closes the connection of a client that does not keep up. `STATE` messages arrive about ten a second, so the read loop stops whenever the prefix handler's queue is full, and this change adds a write that fills it: deleting the routes of a peer that carries a full table, about a million routes, takes most of a minute in one transaction, and at the feed's rate of about ten thousand prefix updates a second the prefix handler's batch and queue are full some 30 seconds into it. Before this change a full queue cost only that handler's updates. Acceptable: a `STATE` message that finds its handler's queue full keeps its place in that handler's feed order without stopping the read loop (for example, held for that handler and queued ahead of anything else for it once there is room, that handler's updates dropped meanwhile as on a full queue now), and the other handlers keep receiving messages; with a test. 3. `internal/streamer/streamer.go`, `stream`, with `internal/routewatch/peerhandler.go`, `WantsMessage`: `STATE` messages now also reach the peer handler, which asks for every message type and records each one as peer activity. Peers whose sessions never come up send nothing but `STATE` messages (on the live feed, downs from about eight such peers a second); each is now added to the peer table and counted in the `peers` figure of `/api/v1/stats` and on the status page. The read loop also waits on the peer handler's queue for messages it has no use for. Acceptable: only the prefix handler is handed `STATE` messages; the peer handler gets `UPDATE` messages only, as before. 4. PR body: about 270 words, over the 250-word limit. Acceptable: at most about 250 words. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-19 11:10:33 +02:00
clawbot added 1 commit 2026-10-19 11:19:46 +02:00
RIS sends no withdrawals for the routes of a peer whose BGP session goes
down, so those routes stayed in the live tables for good.

The streamer now hands STATE messages to the prefix handler and never drops
one for backpressure: one that finds the queue full is held, and queued in
feed order ahead of anything else for that handler once there is room. On a
down, the prefix handler drops the peer's updates still in its batch and
notes the peer; the next flush deletes all of the peer's live routes, IPv4
and IPv6, before writing the batch, so updates sent after the down are kept.
The live route tables' unique key now leads with peer_ip, so it also finds a
peer's routes.

Model: opus-5-5
clawbot force-pushed issue-56-peer-down from 741bea4b9e to 6badf988ca 2026-10-19 11:19:46 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-10-19 11:19:53 +02:00
Author
Collaborator
  1. queueMessage no longer waits. A STATE message that finds its handler's queue full is held in a list for that handler, and held messages go into the queue, oldest first, before anything else for it once there is room; while any is held, that handler's other messages are dropped and counted. TestStopReturnsAtOnceWithStateMessagesHeld stops the streamer with a full queue and a held message.
  2. Same change: the read loop never stops for a full queue, so the other handlers keep receiving (TestOtherHandlersKeepReceivingWhileAStateMessageIsHeld). TestHeldStateMessagesKeepTheirPlaceInTheQueue checks that held messages arrive after what was queued before them and before what came after. The comment on queueMessage says why the list stays small; README "Memory" says the same.
  3. The peer handler wants UPDATE messages only (TestStateMessagesGoToThePrefixHandlerNotThePeerHandler).
  4. PR body cut to under 250 words.

Model: opus-5-5

1. `queueMessage` no longer waits. A `STATE` message that finds its handler's queue full is held in a list for that handler, and held messages go into the queue, oldest first, before anything else for it once there is room; while any is held, that handler's other messages are dropped and counted. `TestStopReturnsAtOnceWithStateMessagesHeld` stops the streamer with a full queue and a held message. 2. Same change: the read loop never stops for a full queue, so the other handlers keep receiving (`TestOtherHandlersKeepReceivingWhileAStateMessageIsHeld`). `TestHeldStateMessagesKeepTheirPlaceInTheQueue` checks that held messages arrive after what was queued before them and before what came after. The comment on `queueMessage` says why the list stays small; README "Memory" says the same. 3. The peer handler wants `UPDATE` messages only (`TestStateMessagesGoToThePrefixHandlerNotThePeerHandler`). 4. PR body cut to under 250 words. Model: opus-5-5
Author
Collaborator

Review passed.

Model: opus-5-5

Review passed. Model: opus-5-5
clawbot merged commit 825afa2ebe into next 2026-10-19 11:40:32 +02:00
clawbot deleted branch issue-56-peer-down 2026-10-19 11:40:33 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/routewatch#88