Skip to content

feat(addressbook): prune stale entries by last-seen time#5513

Open
martinconic wants to merge 1 commit into
masterfrom
feat/addressbook-pruning
Open

feat(addressbook): prune stale entries by last-seen time#5513
martinconic wants to merge 1 commit into
masterfrom
feat/addressbook-pruning

Conversation

@martinconic

Copy link
Copy Markdown
Contributor

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

Implements address book pruning (#5491).

The address book recently gained a wrapping verifiedAddress{Address, Verified}
struct (v2.8.0, #5477). This PR extends it with a last-seen timestamp and uses
it to drop peers we have not seen for over a month, preventing the address book
from accumulating stale, unreachable peers indefinitely.

Changes

  • pkg/addressbook
    • Add LastSeen (unix seconds) to verifiedAddress.
    • Put stamps LastSeen on every write — covers the hive gossip path, which Puts on each sighting.
    • New UpdateLastSeen(overlay): read-modify-write that bumps only the timestamp; no-op if the overlay is absent.
    • New Prune(before time.Time): removes entries last seen before the cutoff. Entries with LastSeen == 0 are kept.
    • Inject a clock for testability.
  • pkg/topology/kademlia: call UpdateLastSeen on successful connection, both outbound (connectionAttemptsHandler) and inbound (onConnected). Failures are debug-logged and non-fatal.
  • pkg/node: prune entries not seen for 30 days at startup, right after the address book is constructed. Warning-logged and non-fatal so it never blocks boot.
  • pkg/statestore/storeadapter: migration step 10 stamps last_seen = now() onto existing entries that lack it (merging into the JSON object so all fields are preserved). Without this, the first prune after upgrade would wipe the whole address book.

AI Disclosure

  • This PR contains code that has been generated by an LLM.
  • I have reviewed the AI generated code thoroughly.
  • I possess the technical expertise to responsibly review the code generated in this PR.

@martinconic martinconic self-assigned this Jun 22, 2026
@martinconic martinconic force-pushed the feat/addressbook-pruning branch from 155323e to 1716cdb Compare June 23, 2026 07:49
@martinconic martinconic marked this pull request as ready for review June 23, 2026 07:53
@gacevicljubisa

gacevicljubisa commented Jun 24, 2026

Copy link
Copy Markdown
Member

Here, prune here runs once, at startup only, and it can lead that if we have some stable connections over bigger period of time, where we haven't updated LastSeen, it will remove those on next restart. But those are our most valuable connections.

Maybe we could use manage() method in kademlia, where we could trigger prune addressbook every 24h (those older then 30 days), but excluding all connected peers, and prune should not start before warmup is done (when node is started). Also, we could stamp last contact at the disconnect, so the prune clock starts from when the peer was actually lost.

One gap is left, what do to with the peer we only hear over hive gossip, but we didn't connect to?

now := time.Now().Unix()

for _, e := range batch {
var fields map[string]json.RawMessage

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not needed. you can shadow the type which is currently on master as a local type scoped to the function, and use it directly to unmarshal and conversion onto the new type. when i think of it, you could even have the latest version (just annotate the last seen field with the json omitempty instruction), and you could unmarshal the existing old serialization format into it, just set the timestamp and write it back into the db.

k.connectedPeers.Add(peer.addr)

if err := k.addressBook.UpdateLastSeen(peer.addr); err != nil {
k.logger.Debug("could not update last seen for peer", "peer_address", peer.addr, "error", err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe warn?

k.connectedPeers.Add(addr)
k.waitNext.Remove(addr)
if err := k.addressBook.UpdateLastSeen(addr); err != nil {
k.logger.Debug("could not update last seen for peer", "peer_address", addr, "error", err)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warn?

@acud

acud commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Here, prune here runs once, at startup only, and it can lead that if we have some stable connections over bigger period of time, where we haven't updated LastSeen, it will remove those on next restart. But those are our most valuable connections.

Maybe we could use manage() method in kademlia, where we could trigger prune addressbook every 24h (those older then 30 days), but excluding all connected peers, and prune should not start before warmup is done (when node is started). Also, we could stamp last contact at the disconnect, so the prune clock starts from when the peer was actually lost.

This is what we've converged to with @janos as a naive starting point to ship.

One gap is left, what do to with the peer we only hear over hive gossip, but we didn't connect to?

For now let's leave them in. The first line of defense is to get rid of garbage which is in the statestore. A peer which is seen over hive and is not connected to is still a potential peer you'd like to connect to. I agree that over time yes, probably you don't wanna keep those. I suggest to ship this first and see it works, then evolve into more well defined scenarios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants