Drop in case of error, will reconcile with next update

This commit is contained in:
riccardom
2026-06-26 17:57:21 +02:00
parent d3f2ef9adb
commit bc22926fe0
2 changed files with 73 additions and 16 deletions

View File

@@ -90,14 +90,16 @@ func (m *mapStateManager) run(ctx context.Context) {
if ctx.Err() != nil {
return
}
log.Errorf("apply sync pass: %v", err)
// avoid a tight error loop; retry on the next target or after a short delay
select {
case <-ctx.Done():
return
case <-m.wake:
case <-time.After(time.Second):
}
// Log and DROP this target — do not retry it. A deterministic failure
// (e.g. a malformed peer in the map) would otherwise spin every pass
// making no progress. Management is the source of truth and re-delivers
// the full map on the next sync, so dropping is safe; peers already
// applied this convergence stay (idempotent diffs) and the remainder is
// reconciled by the next target. Mirrors the legacy handleSync path,
// where the apply error was logged by the gRPC client and the update
// dropped. No onConverged: this target did not converge.
log.Errorf("apply sync pass, dropping update: %v", err)
m.markProcessed(tg)
continue
}
@@ -108,16 +110,24 @@ func (m *mapStateManager) run(ctx context.Context) {
}
// This pass converged. Mark applied only if no newer target arrived during it.
m.mu.Lock()
converged := m.targetGen == tg
if converged {
m.appliedGen = tg
}
m.mu.Unlock()
if converged && m.onConverged != nil {
if m.markProcessed(tg) && m.onConverged != nil {
m.onConverged(time.Since(setAt))
}
// if a newer target arrived mid-pass, ag<tg next iteration -> apply it
}
}
// markProcessed records that the loop has finished working generation tg (whether
// it converged or the pass was dropped on error), so it goes idle instead of
// re-applying the same target. It is a no-op when a newer target arrived during
// the pass (targetGen != tg), leaving appliedGen behind so that target re-applies.
// Returns true when tg was the latest target (i.e. this was a genuine settle).
func (m *mapStateManager) markProcessed(tg uint64) bool {
m.mu.Lock()
defer m.mu.Unlock()
if m.targetGen != tg {
return false
}
m.appliedGen = tg
return true
}

View File

@@ -2,6 +2,7 @@ package internal
import (
"context"
"errors"
"sync/atomic"
"testing"
"time"
@@ -41,6 +42,52 @@ func TestMapStateManager_ConvergesThenStops(t *testing.T) {
require.EqualValues(t, 3, atomic.LoadInt32(&passes), "apply must not run after convergence")
}
// an apply error drops the target: no retry of the same target, no onConverged,
// the loop goes idle — and a fresh target is still applied afterwards.
func TestMapStateManager_DropsTargetOnError(t *testing.T) {
applied := make(chan struct{}, 8)
var failNext atomic.Bool
failNext.Store(true)
apply := func(*mgmProto.SyncResponse) (bool, error) {
applied <- struct{}{}
if failNext.Load() {
return false, errors.New("boom")
}
return false, nil // converge in one pass
}
var converged atomic.Int32
m := newMapStateManager(apply, func(time.Duration) { converged.Add(1) })
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go m.run(ctx)
// first target errors -> applied once, then dropped (no retry, no onConverged)
require.NoError(t, m.SetTarget(&mgmProto.SyncResponse{}))
select {
case <-applied:
case <-time.After(2 * time.Second):
t.Fatal("errored target not applied")
}
select {
case <-applied:
t.Fatal("errored target must not be retried")
case <-time.After(150 * time.Millisecond):
}
require.EqualValues(t, 0, converged.Load(), "onConverged must not fire on error")
// a new target is still processed normally and converges
failNext.Store(false)
require.NoError(t, m.SetTarget(&mgmProto.SyncResponse{}))
select {
case <-applied:
case <-time.After(2 * time.Second):
t.Fatal("new target after error not applied")
}
require.Eventually(t, func() bool { return converged.Load() == 1 }, 2*time.Second, 10*time.Millisecond)
}
// a new target after convergence triggers a fresh apply; an idle (converged)
// manager does not apply on its own.
func TestMapStateManager_ReappliesOnNewTarget(t *testing.T) {