feat(viperblockd): retry LoadState on transient backend errors (siv-25 medium-term) (#210)

* feat(viperblockd): retry LoadState on transient backend errors

ebs.mount used to call vb.LoadState once and treat any failure as fatal.
When predastore was still warming up after a coordinated daemon restart,
the backend Read came back with a network/timeout error, viperblock
folded it into "block size is 0 in both local and backend state", and
the daemon's recovery path then ran the cleanup branch — destroying
DeleteOnTermination=true volumes (mulga-siv-25).

Wrap LoadState in retryLoadState: 5 attempts, 200ms * 1.5^n backoff
(total worst-case ~3.7s, well inside the 30s ebs.mount NATS deadline).
Retry only on viperblock.ErrStateBackendUnavailable; ErrStateNotFound
and any unclassified error fail fast so genuine problems are still
visible.

Retry loop is parameterised over sleep and loadFn so the retry policy
itself is unit-tested without spinning up a real VB.

* chore: bump viperblock to v1.2.2-0.20260511064619-1aa54f0dc1f6

Pulls in the ErrStateNotFound / ErrStateBackendUnavailable sentinels
introduced by mulgadc/viperblock#11 (merged on dev). Required for the
retry wrapper added in the previous commit to compile against the
public viperblock module.

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/kdomanski/iso9660 v0.4.0
 	github.com/klauspost/cpuid/v2 v2.3.0
 	github.com/mulgadc/predastore v1.2.1
-	github.com/mulgadc/viperblock v1.2.1
+	github.com/mulgadc/viperblock v1.2.2-0.20260511064619-1aa54f0dc1f6
 	github.com/nats-io/nats-server/v2 v2.14.0
 	github.com/nats-io/nats.go v1.51.0
 	github.com/ovn-kubernetes/libovsdb v0.8.1
@@ -120,7 +120,6 @@ require (
 	go.opentelemetry.io/otel/trace v1.42.0 // indirect
 	go.yaml.in/yaml/v2 v2.4.4 // indirect
 	go.yaml.in/yaml/v3 v3.0.4 // indirect
-	golang.org/x/exp v0.0.0-20231110203233-9a3e6036ecaa // indirect
 	golang.org/x/mod v0.34.0 // indirect
 	golang.org/x/net v0.53.0 // indirect
 	golang.org/x/sync v0.20.0 // indirect

go.sum

@@ -288,8 +288,8 @@ github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc
 github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk=
 github.com/mulgadc/predastore v1.2.1 h1:Os+CTjjs/IneRMWd/+m/Hi24sHI4M+vVxcBSRF2Pr10=
 github.com/mulgadc/predastore v1.2.1/go.mod h1:WGfbbzk0ExniHXkXsvlmIILSfONakrb7h9yDSxFktaY=
-github.com/mulgadc/viperblock v1.2.1 h1:G3cSgh/yspZd7Wdg3C9QGqkqzYbEfYpusd6uMddSDmg=
-github.com/mulgadc/viperblock v1.2.1/go.mod h1:CisHy13Uam0gUg5dxV0rKryJF2yGbgfDv09FYApRaNk=
+github.com/mulgadc/viperblock v1.2.2-0.20260511064619-1aa54f0dc1f6 h1:PngaxM0orWymcmDMR97JbeedXlgpxoLiykrjSSmPNEU=
+github.com/mulgadc/viperblock v1.2.2-0.20260511064619-1aa54f0dc1f6/go.mod h1:WCP9h+u2DvN8/+ccai5oMOa48hOJPUzSbh7Lia0Q8nY=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=

spinifex/services/viperblockd/viperblockd.go

@@ -2,6 +2,7 @@ package viperblockd
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"os"
@@ -22,6 +23,52 @@ import (
 	"github.com/nats-io/nats.go"
 )
 
+// loadStateRetryAttempts and loadStateRetryBaseDelay tune the mount-time
+// retry loop for transient backend errors (predastore restarting, NATS
+// disconnect, etc.). 5 attempts at 200ms * 1.5^n caps total wait at ~3.7s,
+// well under the daemon's 30s ebs.mount NATS timeout. See mulga-siv-25.
+const (
+	loadStateRetryAttempts  = 5
+	loadStateRetryBaseDelay = 200 * time.Millisecond
+)
+
+// retryLoadState invokes loadFn up to attempts times, retrying with
+// exponential backoff (delay * 3/2 each step) only when the returned error
+// is viperblock.ErrStateBackendUnavailable. Success, ErrStateNotFound, and
+// any unclassified error return immediately so callers see them. Extracted
+// for unit testability — see loadStateWithRetry for the production caller.
+func retryLoadState(volume string, attempts int, baseDelay time.Duration, sleep func(time.Duration), loadFn func() error) error {
+	delay := baseDelay
+	var err error
+	for attempt := 1; attempt <= attempts; attempt++ {
+		err = loadFn()
+		if err == nil {
+			if attempt > 1 {
+				slog.Info("LoadState succeeded after retry",
+					"volume", volume, "attempt", attempt)
+			}
+			return nil
+		}
+		if !errors.Is(err, viperblock.ErrStateBackendUnavailable) {
+			return err
+		}
+		if attempt == attempts {
+			break
+		}
+		slog.Warn("LoadState transient failure, retrying",
+			"volume", volume, "attempt", attempt, "delay", delay, "err", err)
+		sleep(delay)
+		delay = delay * 3 / 2
+	}
+	return fmt.Errorf("LoadState exhausted %d retries: %w", attempts, err)
+}
+
+// loadStateWithRetry calls vb.LoadState with the production retry budget
+// (loadStateRetryAttempts / loadStateRetryBaseDelay).
+func loadStateWithRetry(vb *viperblock.VB, volume string) error {
+	return retryLoadState(volume, loadStateRetryAttempts, loadStateRetryBaseDelay, time.Sleep, vb.LoadState)
+}
+
 var serviceName = "viperblock"
 
 type MountedVolume struct {
@@ -463,8 +510,10 @@ func launchService(cfg *Config) (err error) {
 		}
 
 		// Next, connect to the volume and confirm the state
-		// First, fetch the state from the remote backend
-		err = vb.LoadState()
+		// First, fetch the state from the remote backend. Retry on transient
+		// backend errors (predastore restart races, NATS hiccups) so daemon
+		// recovery doesn't tip a healthy volume into the cleanup path.
+		err = loadStateWithRetry(vb, ebsRequest.Name)
 
 		if err != nil {
 			ebsResponse.Error = err.Error()

spinifex/services/viperblockd/viperblockd_test.go

@@ -1,12 +1,16 @@
 package viperblockd
 
 import (
+	"errors"
+	"fmt"
 	"os"
 	"sync"
 	"testing"
 	"time"
 
+	"github.com/mulgadc/viperblock/viperblock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TestNew tests the service constructor
@@ -439,3 +443,74 @@ func TestMountedVolumePortRange(t *testing.T) {
 		assert.True(t, vol.Port <= 65535)
 	}
 }
+
+// TestRetryLoadState pins the retry policy used by ebs.mount: transient
+// backend errors retry with backoff, ErrStateNotFound fails fast, and the
+// retry budget is honoured. Required by mulga-siv-25 medium-term fix.
+func TestRetryLoadState(t *testing.T) {
+	t.Run("success_first_attempt_no_sleep", func(t *testing.T) {
+		calls := 0
+		sleeps := 0
+		err := retryLoadState("vol-x", 5, 10*time.Millisecond,
+			func(time.Duration) { sleeps++ },
+			func() error { calls++; return nil })
+		require.NoError(t, err)
+		assert.Equal(t, 1, calls)
+		assert.Equal(t, 0, sleeps)
+	})
+
+	t.Run("transient_recovers_within_budget", func(t *testing.T) {
+		calls := 0
+		sleeps := 0
+		err := retryLoadState("vol-x", 5, 10*time.Millisecond,
+			func(time.Duration) { sleeps++ },
+			func() error {
+				calls++
+				if calls < 3 {
+					return fmt.Errorf("wrap: %w", viperblock.ErrStateBackendUnavailable)
+				}
+				return nil
+			})
+		require.NoError(t, err)
+		assert.Equal(t, 3, calls)
+		assert.Equal(t, 2, sleeps)
+	})
+
+	t.Run("not_found_fails_fast", func(t *testing.T) {
+		calls := 0
+		sleeps := 0
+		err := retryLoadState("vol-x", 5, 10*time.Millisecond,
+			func(time.Duration) { sleeps++ },
+			func() error { calls++; return viperblock.ErrStateNotFound })
+		require.Error(t, err)
+		assert.ErrorIs(t, err, viperblock.ErrStateNotFound)
+		assert.Equal(t, 1, calls, "ErrStateNotFound must not be retried")
+		assert.Equal(t, 0, sleeps)
+	})
+
+	t.Run("unclassified_fails_fast", func(t *testing.T) {
+		calls := 0
+		sentinel := errors.New("permission denied")
+		err := retryLoadState("vol-x", 5, 10*time.Millisecond,
+			func(time.Duration) {},
+			func() error { calls++; return sentinel })
+		require.Error(t, err)
+		assert.ErrorIs(t, err, sentinel)
+		assert.Equal(t, 1, calls)
+	})
+
+	t.Run("exhausts_budget_on_persistent_transient", func(t *testing.T) {
+		calls := 0
+		sleeps := 0
+		var lastDelay time.Duration
+		err := retryLoadState("vol-x", 4, 100*time.Millisecond,
+			func(d time.Duration) { sleeps++; lastDelay = d },
+			func() error { calls++; return viperblock.ErrStateBackendUnavailable })
+		require.Error(t, err)
+		assert.ErrorIs(t, err, viperblock.ErrStateBackendUnavailable)
+		assert.Equal(t, 4, calls)
+		assert.Equal(t, 3, sleeps, "sleep happens between attempts, not after the final one")
+		// Backoff multiplier is 1.5: 100ms, 150ms, 225ms.
+		assert.Equal(t, 225*time.Millisecond, lastDelay)
+	})
+}