From faf17bdf6c7fdbdb298c47f213a72622b8151e35 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Thu, 4 May 2023 11:52:20 +0200 Subject: [PATCH 1/2] fix(share/discvoery): fixes from Advertise logic audit --- share/availability/discovery/discovery.go | 24 ++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/share/availability/discovery/discovery.go b/share/availability/discovery/discovery.go index 0a2deb7858..27a3cc0242 100644 --- a/share/availability/discovery/discovery.go +++ b/share/availability/discovery/discovery.go @@ -35,11 +35,6 @@ const ( defaultRetryTimeout = time.Second ) -// waitF calculates time to restart announcing. -var waitF = func(ttl time.Duration) time.Duration { - return 7 * ttl / 8 -} - type Parameters struct { // PeersLimit defines the soft limit of FNs to connect to via discovery. // Set 0 to disable. @@ -67,8 +62,9 @@ func (p Parameters) withDefaults() Parameters { func DefaultParameters() Parameters { return Parameters{ - PeersLimit: 5, - AdvertiseInterval: time.Hour * 8, + PeersLimit: 5, + // based on https://github.com/libp2p/go-libp2p-kad-dht/pull/793 + AdvertiseInterval: time.Hour * 22, } } @@ -151,24 +147,31 @@ func (d *Discovery) Peers(ctx context.Context) ([]peer.ID, error) { // TODO: Start advertising only after the reachability is confirmed by AutoNAT func (d *Discovery) Advertise(ctx context.Context) { if d.params.AdvertiseInterval == -1 { + log.Warn("AdvertiseInterval is set to -1. Skipping advertising...") return } timer := time.NewTimer(d.params.AdvertiseInterval) defer timer.Stop() for { - ttl, err := d.disc.Advertise(ctx, rendezvousPoint) + _, err := d.disc.Advertise(ctx, rendezvousPoint) if err != nil { - log.Debugf("Error advertising %s: %s", rendezvousPoint, err.Error()) if ctx.Err() != nil { return } + log.Debugf("error advertising %s: %s", rendezvousPoint, err.Error()) + errTimer := time.NewTimer(time.Minute) select { - case <-timer.C: + case <-errTimer.C: + errTimer.Stop() + if !timer.Stop() { + <-timer.C + } timer.Reset(d.params.AdvertiseInterval) continue case <-ctx.Done(): + errTimer.Stop() return } } @@ -176,7 +179,6 @@ func (d *Discovery) Advertise(ctx context.Context) { log.Debugf("advertised") select { case <-timer.C: - timer.Reset(waitF(ttl)) case <-ctx.Done(): return } From 82170205b486c9c632bfd9a32435a58f2b5ed4c0 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Thu, 4 May 2023 14:11:17 +0200 Subject: [PATCH 2/2] review comments --- share/availability/discovery/discovery.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/share/availability/discovery/discovery.go b/share/availability/discovery/discovery.go index 27a3cc0242..cd1ccbb8f8 100644 --- a/share/availability/discovery/discovery.go +++ b/share/availability/discovery/discovery.go @@ -159,7 +159,7 @@ func (d *Discovery) Advertise(ctx context.Context) { if ctx.Err() != nil { return } - log.Debugf("error advertising %s: %s", rendezvousPoint, err.Error()) + log.Warn("error advertising %s: %s", rendezvousPoint, err.Error()) errTimer := time.NewTimer(time.Minute) select { @@ -168,7 +168,6 @@ func (d *Discovery) Advertise(ctx context.Context) { if !timer.Stop() { <-timer.C } - timer.Reset(d.params.AdvertiseInterval) continue case <-ctx.Done(): errTimer.Stop() @@ -177,6 +176,10 @@ func (d *Discovery) Advertise(ctx context.Context) { } log.Debugf("advertised") + if !timer.Stop() { + <-timer.C + } + timer.Reset(d.params.AdvertiseInterval) select { case <-timer.C: case <-ctx.Done():