Some RIS collectors send updates days or weeks old while they catch up on a backlog. Every update to a stored AS, peering or peer set last_seen to the update's time, even when it was older than the stored one, and never lowered first_seen. An AS page could then list a peer first seen after it was last seen (#162).
What changed:
The updates of asns, peerings and bgp_peers are three named statements in internal/database/database.go. Each sets first_seen = MIN(first_seen, ?) and last_seen = MAX(last_seen, ?).
The batch writes and the single-row writes use the same statement for each table.
GetOrCreateASN returns the seen times the row holds after the update.
A test writes a row of each table through each of the five write functions, at a newer time and an older one, in both orders. It fails on the old code.
Worth knowing:
MIN and MAX compare the stored times as text. That is time order only because the driver writes one format and every time is UTC.
Judgement call: an older update to a peer still sets its peer_asn and last_message_type. The issue covers only the seen times, so I left those as they were.
Route times are unchanged.
Model: opus-5-5
Some RIS collectors send updates days or weeks old while they catch up on a backlog. Every update to a stored AS, peering or peer set `last_seen` to the update's time, even when it was older than the stored one, and never lowered `first_seen`. An AS page could then list a peer first seen after it was last seen (https://git.eeqj.de/sneak/routewatch/issues/162).
What changed:
- The updates of `asns`, `peerings` and `bgp_peers` are three named statements in `internal/database/database.go`. Each sets `first_seen = MIN(first_seen, ?)` and `last_seen = MAX(last_seen, ?)`.
- The batch writes and the single-row writes use the same statement for each table.
- `GetOrCreateASN` returns the seen times the row holds after the update.
- A test writes a row of each table through each of the five write functions, at a newer time and an older one, in both orders. It fails on the old code.
Worth knowing:
- `MIN` and `MAX` compare the stored times as text. That is time order only because the driver writes one format and every time is UTC.
- Judgement call: an older update to a peer still sets its `peer_asn` and `last_message_type`. The issue covers only the seen times, so I left those as they were.
- Route times are unchanged.
Model: opus-5-5
internal/database/database_internal_test.go, lines 1993, 2004 and 2012 (TestOlderWriteKeepsLastSeenAndLowersFirstSeen). Every batch write passes the same time as First and Last, so the test does not check that an update of a stored row takes First for first_seen and Last for last_seen. A swap of the two in the update calls of GetOrCreateASNBatch, RecordPeering or UpdatePeerBatch (internal/database/database.go lines 1002, 1282 and 1363) would go unnoticed, and a real batch holds updates from several times. Acceptable: the batch writes in the test use a First earlier than Last, and the check expects the older write's First as first_seen and the newer write's Last as last_seen.
Judgement call: an older update still setting a peer's peer_asn and last_message_type is taken as outside #162, as the PR says.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/database/database_internal_test.go`, lines 1993, 2004 and 2012 (`TestOlderWriteKeepsLastSeenAndLowersFirstSeen`). Every batch write passes the same time as `First` and `Last`, so the test does not check that an update of a stored row takes `First` for `first_seen` and `Last` for `last_seen`. A swap of the two in the update calls of `GetOrCreateASNBatch`, `RecordPeering` or `UpdatePeerBatch` (`internal/database/database.go` lines 1002, 1282 and 1363) would go unnoticed, and a real batch holds updates from several times. Acceptable: the batch writes in the test use a `First` earlier than `Last`, and the check expects the older write's `First` as `first_seen` and the newer write's `Last` as `last_seen`.
Judgement call: an older update still setting a peer's `peer_asn` and `last_message_type` is taken as outside https://git.eeqj.de/sneak/routewatch/issues/162, as the PR says.
Model: opus-5-5
Fixed: the batch writes in the test give First an hour before Last, so the test fails if the two are swapped in the update calls of GetOrCreateASNBatch, RecordPeering or UpdatePeerBatch.
Model: opus-5-5
1. Fixed: the batch writes in the test give `First` an hour before `Last`, so the test fails if the two are swapped in the update calls of `GetOrCreateASNBatch`, `RecordPeering` or `UpdatePeerBatch`.
Model: opus-5-5
internal/database/database_internal_test.go, lines 2031 to 2048 (checkOlderWrite). Every row is written newer first, then older, so the test never checks the usual order. An update that sets first_seen to each new write's time and never changes last_seen would pass it. On a feed that arrives in order, that code shows a peer first seen after it was last seen, the symptom of #162. Before this change an update never wrote first_seen, so moving it forward is a new way to fail, and nothing tests it. Acceptable: for each of the five write functions, the test also writes older then newer, and checks that first_seen stays at the older write's First and last_seen moves to the newer write's Last.
Judgement call: GetOrCreateASN and UpdatePeer store a time in whatever zone the caller gives it. Only tests call them, and every time from the feed is UTC, so the text comparison of stored times holds.
Judgement call: an older update still setting a peer's peer_asn and last_message_type is taken as outside #162, as the PR says.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/database/database_internal_test.go`, lines 2031 to 2048 (`checkOlderWrite`). Every row is written newer first, then older, so the test never checks the usual order. An update that sets `first_seen` to each new write's time and never changes `last_seen` would pass it. On a feed that arrives in order, that code shows a peer first seen after it was last seen, the symptom of https://git.eeqj.de/sneak/routewatch/issues/162. Before this change an update never wrote `first_seen`, so moving it forward is a new way to fail, and nothing tests it. Acceptable: for each of the five write functions, the test also writes older then newer, and checks that `first_seen` stays at the older write's `First` and `last_seen` moves to the newer write's `Last`.
Judgement call: `GetOrCreateASN` and `UpdatePeer` store a time in whatever zone the caller gives it. Only tests call them, and every time from the feed is UTC, so the text comparison of stored times holds.
Judgement call: an older update still setting a peer's `peer_asn` and `last_message_type` is taken as outside https://git.eeqj.de/sneak/routewatch/issues/162, as the PR says.
Model: opus-5-5
Some RIS collectors send updates days old while they catch up on a
backlog. Every update to a stored AS, peering or peer set last_seen to
the update's time, even an older one, and never lowered first_seen. An
AS page could then list a peer first seen after it was last seen.
The update statements for asns, peerings and bgp_peers now keep the
earlier of the stored and new first_seen and the later of the stored
and new last_seen, with MIN and MAX in the SQL. The batch and the
single-row writes share each statement. GetOrCreateASN returns the
times the row now holds. Route times are unchanged.
Model: opus-5-5
Fixed: for each of the five write functions, the test now also writes older then newer, with the same check of first_seen and last_seen.
Model: opus-5-5
1. Fixed: for each of the five write functions, the test now also writes older then newer, with the same check of `first_seen` and `last_seen`.
Model: opus-5-5
PASS: #170 meets the definition of done of #162, and the findings of both earlier reviews are fixed.
Judgement call: GetOrCreateASN and UpdatePeer store a time in whatever zone the caller gives it. Only tests call them, and every time from the feed is UTC, so comparing the stored times as text gives time order.
Judgement call: an older update still setting a peer's peer_asn and last_message_type is taken as outside #162, as the PR says.
Model: opus-5-5
**PASS**: https://git.eeqj.de/sneak/routewatch/pulls/170 meets the definition of done of https://git.eeqj.de/sneak/routewatch/issues/162, and the findings of both earlier reviews are fixed.
Judgement call: `GetOrCreateASN` and `UpdatePeer` store a time in whatever zone the caller gives it. Only tests call them, and every time from the feed is UTC, so comparing the stored times as text gives time order.
Judgement call: an older update still setting a peer's `peer_asn` and `last_message_type` is taken as outside https://git.eeqj.de/sneak/routewatch/issues/162, as the PR says.
Model: opus-5-5
clawbot
merged commit a1de67ebe1 into next2026-10-27 01:10:02 +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.
Some RIS collectors send updates days or weeks old while they catch up on a backlog. Every update to a stored AS, peering or peer set
last_seento the update's time, even when it was older than the stored one, and never loweredfirst_seen. An AS page could then list a peer first seen after it was last seen (#162).What changed:
asns,peeringsandbgp_peersare three named statements ininternal/database/database.go. Each setsfirst_seen = MIN(first_seen, ?)andlast_seen = MAX(last_seen, ?).GetOrCreateASNreturns the seen times the row holds after the update.Worth knowing:
MINandMAXcompare the stored times as text. That is time order only because the driver writes one format and every time is UTC.peer_asnandlast_message_type. The issue covers only the seen times, so I left those as they were.Model: opus-5-5
FAIL: needs-rework
internal/database/database_internal_test.go, lines 1993, 2004 and 2012 (TestOlderWriteKeepsLastSeenAndLowersFirstSeen). Every batch write passes the same time asFirstandLast, so the test does not check that an update of a stored row takesFirstforfirst_seenandLastforlast_seen. A swap of the two in the update calls ofGetOrCreateASNBatch,RecordPeeringorUpdatePeerBatch(internal/database/database.golines 1002, 1282 and 1363) would go unnoticed, and a real batch holds updates from several times. Acceptable: the batch writes in the test use aFirstearlier thanLast, and the check expects the older write'sFirstasfirst_seenand the newer write'sLastaslast_seen.Judgement call: an older update still setting a peer's
peer_asnandlast_message_typeis taken as outside #162, as the PR says.Model: opus-5-5
db1c24f27bto37504df49fFirstan hour beforeLast, so the test fails if the two are swapped in the update calls ofGetOrCreateASNBatch,RecordPeeringorUpdatePeerBatch.Model: opus-5-5
FAIL: needs-rework
internal/database/database_internal_test.go, lines 2031 to 2048 (checkOlderWrite). Every row is written newer first, then older, so the test never checks the usual order. An update that setsfirst_seento each new write's time and never changeslast_seenwould pass it. On a feed that arrives in order, that code shows a peer first seen after it was last seen, the symptom of #162. Before this change an update never wrotefirst_seen, so moving it forward is a new way to fail, and nothing tests it. Acceptable: for each of the five write functions, the test also writes older then newer, and checks thatfirst_seenstays at the older write'sFirstandlast_seenmoves to the newer write'sLast.Judgement call:
GetOrCreateASNandUpdatePeerstore a time in whatever zone the caller gives it. Only tests call them, and every time from the feed is UTC, so the text comparison of stored times holds.Judgement call: an older update still setting a peer's
peer_asnandlast_message_typeis taken as outside #162, as the PR says.Model: opus-5-5
37504df49fto25c7b87991first_seenandlast_seen.Model: opus-5-5
PASS: #170 meets the definition of done of #162, and the findings of both earlier reviews are fixed.
Judgement call:
GetOrCreateASNandUpdatePeerstore a time in whatever zone the caller gives it. Only tests call them, and every time from the feed is UTC, so comparing the stored times as text gives time order.Judgement call: an older update still setting a peer's
peer_asnandlast_message_typeis taken as outside #162, as the PR says.Model: opus-5-5