mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-29 02:51:29 +02:00
[client] Resume peer reconnection when the network comes back
Ticks taken while the OS reported no network were skipped, but they still advanced the exponential backoff, so a peer could be tens of seconds from its next attempt by the time connectivity returned. Recovery then depended on a signal or relay event, which never arrives when both stayed up across the outage, e.g. a short airplane mode toggle over Wi-Fi. A peer parked in ICE hourly mode stayed there for the same reason. React to the offline-to-online transition directly: re-arm the reconnect ticker and reset the ICE retry state, so the peer retries at once. Add netstate.Changed for callers that own a select loop and cannot block in Wait.
This commit is contained in:
@@ -104,12 +104,13 @@ func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) {
|
||||
iceState := &iceRetryState{log: g.log}
|
||||
defer iceState.reset()
|
||||
|
||||
netChanged := g.netState.Changed()
|
||||
|
||||
for {
|
||||
select {
|
||||
case <-tickerChannel:
|
||||
// skip attempts while the OS reports no usable network; the ticker
|
||||
// keeps running so other events remain responsive, and the guard
|
||||
// resumes via the signal/relay reconnect events once network returns
|
||||
// skip attempts while the OS reports no usable network; the
|
||||
// netChanged case below resumes the loop once it returns
|
||||
if !g.netState.IsOnline() {
|
||||
continue
|
||||
}
|
||||
@@ -149,6 +150,23 @@ func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) {
|
||||
tickerChannel = ticker.C
|
||||
iceState.reset()
|
||||
|
||||
case <-netChanged:
|
||||
// Re-arm for the next transition before acting on this one.
|
||||
netChanged = g.netState.Changed()
|
||||
if !g.netState.IsOnline() {
|
||||
continue
|
||||
}
|
||||
// Ticks skipped while offline drove the backoff towards its
|
||||
// maximum without ever attempting, and left the ICE budget
|
||||
// frozen — possibly in hourly mode. Recover on our own so the
|
||||
// peer does not depend on a signal or relay event that never
|
||||
// comes when both stayed up across the outage.
|
||||
g.log.Debugf("network is back, reset reconnection ticker")
|
||||
ticker.Stop()
|
||||
ticker = g.newReconnectTicker(ctx)
|
||||
tickerChannel = ticker.C
|
||||
iceState.reset()
|
||||
|
||||
case <-ctx.Done():
|
||||
g.log.Debugf("context is done, stop reconnect loop")
|
||||
return
|
||||
|
||||
98
client/internal/peer/guard/guard_netstate_test.go
Normal file
98
client/internal/peer/guard/guard_netstate_test.go
Normal file
@@ -0,0 +1,98 @@
|
||||
package guard
|
||||
|
||||
import (
|
||||
"context"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
log "github.com/sirupsen/logrus"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/peer/ice"
|
||||
"github.com/netbirdio/netbird/client/netstate"
|
||||
)
|
||||
|
||||
// newTestGuardWithNetState builds a guard with a realistic MaxInterval: the
|
||||
// backoff must be able to grow well past the outage, as it does in production
|
||||
// where the timeout is seconds to minutes.
|
||||
func newTestGuardWithNetState(status connStatusFunc, netState *netstate.State) *Guard {
|
||||
srw := NewSRWatcher(nil, nil, nil, ice.Config{})
|
||||
return NewGuard(log.WithField("test", "guard"), status, 30*time.Second, srw, netState)
|
||||
}
|
||||
|
||||
// TestGuard_RecoversAfterOfflineToOnline covers a peer that stays disconnected
|
||||
// across a network outage while neither signal nor relay reports an event —
|
||||
// both stayed up, as on a short airplane mode toggle over Wi-Fi.
|
||||
//
|
||||
// Every tick taken while offline is skipped, but it still advances the
|
||||
// exponential backoff, so by the time the network returns the next tick can be
|
||||
// tens of seconds away. Without an explicit reaction to the transition the
|
||||
// peer waits out that interval for a recovery that could start immediately.
|
||||
func TestGuard_RecoversAfterOfflineToOnline(t *testing.T) {
|
||||
netState := netstate.New()
|
||||
|
||||
var attempts atomic.Int32
|
||||
g := newTestGuardWithNetState(func() ConnStatus { return ConnStatusDisconnected }, netState)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
// Start from the reconnect ticker (800ms initial interval), the state a
|
||||
// peer is in after it loses its connection.
|
||||
go g.Start(ctx, func() { attempts.Add(1) })
|
||||
g.SetRelayedConnDisconnected()
|
||||
|
||||
// Let the backoff climb: 0.8s, 1.6s, 3.2s, 6.4s ... every tick is skipped
|
||||
// while offline, but each one doubles the wait for the next.
|
||||
netState.Set(false)
|
||||
time.Sleep(8 * time.Second)
|
||||
|
||||
offlineAttempts := attempts.Load()
|
||||
if offlineAttempts != 0 {
|
||||
t.Fatalf("callback ran %d times while offline, want 0", offlineAttempts)
|
||||
}
|
||||
|
||||
netState.Set(true)
|
||||
|
||||
// The next organic tick is now several seconds out, so anything within
|
||||
// this window can only come from reacting to the transition itself.
|
||||
select {
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("peer was not retried within 2s of the network coming back, " +
|
||||
"with neither a signal nor a relay event to fall back on")
|
||||
case <-pollUntil(func() bool { return attempts.Load() > 0 }):
|
||||
}
|
||||
}
|
||||
|
||||
// TestGuard_OfflineTransitionDoesNotRetry checks the other direction: going
|
||||
// offline must not itself trigger an attempt.
|
||||
func TestGuard_OfflineTransitionDoesNotRetry(t *testing.T) {
|
||||
netState := netstate.New()
|
||||
|
||||
var attempts atomic.Int32
|
||||
g := newTestGuardWithNetState(func() ConnStatus { return ConnStatusDisconnected }, netState)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
go g.Start(ctx, func() { attempts.Add(1) })
|
||||
|
||||
netState.Set(false)
|
||||
time.Sleep(5 * time.Second)
|
||||
|
||||
if got := attempts.Load(); got != 0 {
|
||||
t.Fatalf("callback ran %d times after going offline, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
func pollUntil(cond func() bool) <-chan struct{} {
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
for {
|
||||
if cond() {
|
||||
close(done)
|
||||
return
|
||||
}
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
}
|
||||
}()
|
||||
return done
|
||||
}
|
||||
@@ -61,6 +61,20 @@ func (s *State) IsOnline() bool {
|
||||
return s.online
|
||||
}
|
||||
|
||||
// Changed returns a channel closed on the next availability transition, for
|
||||
// callers that already own a select loop and cannot block in Wait. Re-read it
|
||||
// after every fire: each transition installs a fresh channel. On a nil
|
||||
// receiver — no State injected — it returns nil, which blocks forever in a
|
||||
// select, so the caller simply never observes a transition.
|
||||
func (s *State) Changed() <-chan struct{} {
|
||||
if s == nil {
|
||||
return nil
|
||||
}
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
return s.changed
|
||||
}
|
||||
|
||||
// Wait blocks while the network is offline. It reports whether it had to
|
||||
// wait, so callers can reset their backoff after an outage. It returns early
|
||||
// with the context error when ctx is done. On a nil receiver — no State
|
||||
|
||||
Reference in New Issue
Block a user