From d0a3cc17f47ae7b43bd1b229a8f5a3bd894eed16 Mon Sep 17 00:00:00 2001 From: vyzo Date: Sun, 28 Apr 2019 11:35:23 +0300 Subject: [PATCH 1/4] don't delete under the read lock let gc clean up the addrs --- pstoremem/addr_book.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pstoremem/addr_book.go b/pstoremem/addr_book.go index 002f8b1..5061f0f 100644 --- a/pstoremem/addr_book.go +++ b/pstoremem/addr_book.go @@ -188,11 +188,9 @@ func (mab *memoryAddrBook) Addrs(p peer.ID) []ma.Multiaddr { now := time.Now() good := make([]ma.Multiaddr, 0, len(amap)) - for k, m := range amap { + for _, m := range amap { if !m.ExpiredBy(now) { good = append(good, m.Addr) - } else { - delete(amap, k) } } From e4cf50160fef0da727b8fad056eb2d9cd222d768 Mon Sep 17 00:00:00 2001 From: vyzo Date: Sun, 28 Apr 2019 12:17:48 +0300 Subject: [PATCH 2/4] periodically schedule gc --- pstoremem/addr_book.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/pstoremem/addr_book.go b/pstoremem/addr_book.go index 5061f0f..f2e89ec 100644 --- a/pstoremem/addr_book.go +++ b/pstoremem/addr_book.go @@ -37,14 +37,41 @@ type memoryAddrBook struct { addrs map[peer.ID]map[string]*expiringAddr nextGC time.Time + ctx context.Context + cancel func() subManager *AddrSubManager } func NewAddrBook() pstore.AddrBook { - return &memoryAddrBook{ + ctx, cancel := context.WithCancel(context.Background()) + + ab := &memoryAddrBook{ addrs: make(map[peer.ID]map[string]*expiringAddr), subManager: NewAddrSubManager(), + ctx: ctx, + cancel: cancel, + } + + go ab.background() + return ab +} + +// background periodically schedules a gc +func (mab *memoryAddrBook) background() { + ticker := time.NewTicker(1 * time.Hour) + defer ticker.Stop() + + for { + select { + case <-ticker.C: + mab.addrmu.Lock() + mab.gc() + mab.addrmu.Unlock() + + case <-mab.ctx.Done(): + return + } } } From c58ce1f56fe13ffae980782d1b706ab18887ea9f Mon Sep 17 00:00:00 2001 From: vyzo Date: Sun, 28 Apr 2019 12:19:26 +0300 Subject: [PATCH 3/4] add Close method to memory address book, which cancels the gc process --- pstoremem/addr_book.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pstoremem/addr_book.go b/pstoremem/addr_book.go index f2e89ec..66290bc 100644 --- a/pstoremem/addr_book.go +++ b/pstoremem/addr_book.go @@ -75,6 +75,11 @@ func (mab *memoryAddrBook) background() { } } +func (mab *memoryAddrBook) Close() error { + mab.cancel() + return nil +} + // gc garbage collects the in-memory address book. The caller *must* hold the addrmu lock. func (mab *memoryAddrBook) gc() { now := time.Now() From f21f6eac4f77c828be1c662a814e349da9595fd3 Mon Sep 17 00:00:00 2001 From: vyzo Date: Sun, 28 Apr 2019 16:21:07 +0300 Subject: [PATCH 4/4] don't eager gc --- pstoremem/addr_book.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/pstoremem/addr_book.go b/pstoremem/addr_book.go index 66290bc..cb9576a 100644 --- a/pstoremem/addr_book.go +++ b/pstoremem/addr_book.go @@ -36,7 +36,6 @@ type memoryAddrBook struct { // drastically increase the space waste. In our case, by 6x. addrs map[peer.ID]map[string]*expiringAddr - nextGC time.Time ctx context.Context cancel func() @@ -65,9 +64,7 @@ func (mab *memoryAddrBook) background() { for { select { case <-ticker.C: - mab.addrmu.Lock() mab.gc() - mab.addrmu.Unlock() case <-mab.ctx.Done(): return @@ -80,12 +77,12 @@ func (mab *memoryAddrBook) Close() error { return nil } -// gc garbage collects the in-memory address book. The caller *must* hold the addrmu lock. +// gc garbage collects the in-memory address book. func (mab *memoryAddrBook) gc() { + mab.addrmu.Lock() + defer mab.addrmu.Unlock() + now := time.Now() - if !now.After(mab.nextGC) { - return - } for p, amap := range mab.addrs { for k, addr := range amap { if addr.ExpiredBy(now) { @@ -96,7 +93,6 @@ func (mab *memoryAddrBook) gc() { delete(mab.addrs, p) } } - mab.nextGC = time.Now().Add(pstore.AddressTTL) } func (mab *memoryAddrBook) PeersWithAddrs() peer.IDSlice { @@ -146,7 +142,6 @@ func (mab *memoryAddrBook) AddAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Du mab.subManager.BroadcastAddr(p, addr) } } - mab.gc() } // SetAddr calls mgr.SetAddrs(p, addr, ttl) @@ -183,7 +178,6 @@ func (mab *memoryAddrBook) SetAddrs(p peer.ID, addrs []ma.Multiaddr, ttl time.Du delete(amap, addrstr) } } - mab.gc() } // UpdateAddrs updates the addresses associated with the given peer that have @@ -205,7 +199,6 @@ func (mab *memoryAddrBook) UpdateAddrs(p peer.ID, oldTTL time.Duration, newTTL t amap[k] = addr } } - mab.gc() } // Addresses returns all known (and valid) addresses for a given