diff --git a/TODO.md b/TODO.md index d1ad2a0..d19cfd4 100644 --- a/TODO.md +++ b/TODO.md @@ -35,6 +35,12 @@ tested. # Completed Steps +- 2026-11-02: working out the home page's cards no longer stops the prefix + handler. The walk over the routing table holds the table's lock for 10,000 + prefixes at a time, then lets the lock go for as long again, as `PeerDown` + does. It used to take the lock back at once. The prefix handler then applied + about one update per 10,000 prefixes walked. During a walk, its queue could + fill, and the streamer then dropped updates for it (closes #226) - 2026-11-02: like a failed WHOIS lookup, a lookup skipped because its RIR is paused makes the fetcher wait longer before the next lookup. The longest wait is one minute. The status page does not count a skip as a failure. A skip used diff --git a/internal/routetable/overview.go b/internal/routetable/overview.go index 3b5bbca..d816970 100644 --- a/internal/routetable/overview.go +++ b/internal/routetable/overview.go @@ -79,8 +79,9 @@ type OriginPeers struct { // Overview works out the table's overview. It lists at most maxListed bogon // announcements and at most maxListed prefixes with more than one origin AS: -// the first in prefix order. It walks over every prefix, but holds the read -// lock for walkBatch prefixes at a time. +// the first in prefix order. It visits every prefix in the table, in batches +// of walkBatch. After each batch, it lets the table's lock go for as long as +// it held the lock for the batch. func (t *Table) Overview(maxListed int) Overview { var overview Overview @@ -110,15 +111,18 @@ func (t *Table) Overview(maxListed int) Overview { } // walkPrefixes calls visit with every prefix of the table and its origins. It -// holds the read lock, but lets it go after every walkBatch prefixes, so the -// updates waiting for it get in. A prefix that is added or removed in such a -// gap may or may not be visited, as Go allows for a map that changes during a -// range over it. visit must not keep origins once it returns. +// holds the read lock for walkBatch prefixes at a time. After each batch, it +// lets the table's lock go for as long as it held the lock for the batch. +// During a walk over many batches, updates can then take the lock about half +// of the time. The walk may or may not visit a prefix that is added or removed +// while the lock is let go. Go allows this in a range over a map that changes. +// visit must not keep origins once it returns. func (t *Table) walkPrefixes(visit func(prefix netip.Prefix, origins []*origin)) { t.mu.RLock() defer t.mu.RUnlock() visited := 0 + batchStart := time.Now() for _, f := range []*family{&t.ipv4, &t.ipv6} { for _, prefixes := range f.byLength { @@ -127,8 +131,13 @@ func (t *Table) walkPrefixes(visit func(prefix netip.Prefix, origins []*origin)) visited++ if visited%walkBatch == 0 { + held := time.Since(batchStart) + t.mu.RUnlock() + time.Sleep(held) t.mu.RLock() + + batchStart = time.Now() } } } diff --git a/internal/routetable/overview_internal_test.go b/internal/routetable/overview_internal_test.go index 7ace291..4943411 100644 --- a/internal/routetable/overview_internal_test.go +++ b/internal/routetable/overview_internal_test.go @@ -131,3 +131,61 @@ func TestWalkPrefixesLetsUpdatesIn(t *testing.T) { "before its second batch") } } + +// TestOverviewLetsUpdatesIn applies another peer's 1,000 announcements while +// Overview walks a table with more prefixes than five of the walk's batches. +// After each batch, the walk lets the lock go for as long as it held the lock. +// So all 1,000 announcements must be applied before Overview returns. A walk +// that takes the lock back at once lets in about one update per batch. +func TestOverviewLetsUpdatesIn(t *testing.T) { + t.Parallel() + + const ( + asn = 1000 + peer = "192.0.2.1" + otherPeer = "192.0.2.2" + // prefixCount is more prefixes than five of the walk's batches. + prefixCount = 5*walkBatch + 1 + announcements = 1000 + ) + + routeTime := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + table := New(Params{}) + prefixes := make([]netip.Prefix, 0, prefixCount) + addr := netip.MustParseAddr("1.0.0.0") + + for range prefixCount { + prefix := netip.PrefixFrom(addr, addr.BitLen()) + table.Announce(prefix, asn, Route{Peer: peer, Time: routeTime}) + prefixes = append(prefixes, prefix) + addr = addr.Next() + } + + overviewDone := make(chan struct{}) + + go func() { + defer close(overviewDone) + + table.Overview(0) + }() + + // Wait until Overview holds the read lock. TryLock fails while a reader + // holds it. + for table.mu.TryLock() { + table.mu.Unlock() + runtime.Gosched() + } + + for _, prefix := range prefixes[:announcements] { + table.Announce(prefix, asn, Route{Peer: otherPeer, Time: routeTime}) + } + + select { + case <-overviewDone: + t.Fatal("Overview returned before another peer's 1,000 announcements " + + "were applied") + default: + } + + <-overviewDone +}