fix(forecast): save the forecast identity outside the lock (#1883)

RefreshIdentity held forecastSiteConfig.mu across two SaveConfig calls,
each up to 5 s against a busy SQLite writer. The control tick's
Snapshot waited behind them. Compute under the lock, save after it, and
wait 30 s before retrying a failed binding save. Configure's weather
generation save moves out of the lock the same way.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: frahlg

.changeset/a9-forecast-identity-save-outside-lock.md

@@ -0,0 +1,5 @@
+---
+"ftw": patch
+---
+
+Keep the control loop on time while the forecast saves its device identity. The save no longer holds the lock the control loop reads, so a busy database can no longer delay control by up to 10 seconds. After a failed save, wait 30 seconds before trying again instead of retrying every second.

go/cmd/ftw/forecast_identity_lock_test.go

@@ -0,0 +1,102 @@
+package main
+
+import (
+	"database/sql"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/srcfl/ftw/go/internal/state"
+	_ "modernc.org/sqlite"
+)
+
+// RefreshIdentity must not hold forecastSiteConfig.mu while SaveConfig waits
+// for the SQLite write lock, or the tick's Snapshot() waits with it.
+func TestForecastSnapshotDoesNotWaitForIdentitySave(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "state.db")
+	st, err := state.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer st.Close()
+
+	s := &forecastSiteConfig{
+		store:        st,
+		baseRevision: "base",
+		required:     map[string]bool{"meter": false},
+		identity:     func(string) (string, bool) { return "serial:ABC123", true },
+		configuredAt: time.Now().Add(-time.Minute),
+	}
+
+	// Another writer (rolloff, backup, history writer) holds the WAL write lock.
+	other, err := sql.Open("sqlite", path+"?_pragma=busy_timeout(5000)")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer other.Close()
+	other.SetMaxOpenConns(1)
+	tx, err := other.Begin()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := tx.Exec(`UPDATE config SET value = value WHERE 0`); err != nil {
+		t.Fatal(err)
+	}
+
+	refreshDone := make(chan time.Duration, 1)
+	go func() {
+		start := time.Now()
+		s.RefreshIdentity(time.Now())
+		refreshDone <- time.Since(start)
+	}()
+	time.Sleep(100 * time.Millisecond) // RefreshIdentity now waits in SaveConfig
+
+	start := time.Now()
+	_ = s.Snapshot() // what the control tick does before persistTelemetryTick
+	blocked := time.Since(start)
+	_ = tx.Rollback()
+	refresh := <-refreshDone
+	t.Logf("Snapshot waited %v; RefreshIdentity took %v", blocked, refresh)
+	if blocked > time.Second {
+		t.Fatalf("tick Snapshot blocked %v behind identity SaveConfig held under forecast lock", blocked)
+	}
+}
+
+// A failed binding save stays dirty and waits forecastBindingRetry before the
+// next attempt, instead of retrying on every 1 s identity poll.
+func TestForecastIdentityBindingSaveBacksOffAfterFailure(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "state.db")
+	broken, err := state.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_ = broken.Close() // every save now fails
+
+	s := &forecastSiteConfig{
+		store:        broken,
+		baseRevision: "base",
+		required:     map[string]bool{"meter": false},
+		identity:     func(string) (string, bool) { return "serial:ABC123", true },
+		configuredAt: time.Now().Add(-time.Minute),
+	}
+	now := time.Now()
+	s.RefreshIdentity(now)
+	if !s.bindingDirty {
+		t.Fatal("failed binding save was marked clean")
+	}
+
+	st, err := state.Open(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer st.Close()
+	s.store = st
+	s.RefreshIdentity(now.Add(time.Second))
+	if _, ok := st.LoadConfig(forecastIdentityReceiptKey); ok || !s.bindingDirty {
+		t.Fatal("binding save retried inside the back-off window")
+	}
+	s.RefreshIdentity(now.Add(forecastBindingRetry))
+	if _, ok := st.LoadConfig(forecastIdentityReceiptKey); !ok || s.bindingDirty {
+		t.Fatal("binding save not retried after the back-off window")
+	}
+}

go/cmd/ftw/forecast_site.go

@@ -34,6 +34,7 @@ type forecastSiteConfig struct {
 	required              map[string]bool // value identifies PV sources
 	accepted              forecastIdentityReceipt
 	bindingDirty          bool
+	bindingRetryAt        time.Time // a failed binding save waits until then
 	configuredAt          time.Time
 	heatingPrior, ratedPV float64
 	annualBase            float64
@@ -49,6 +50,10 @@ type forecastIdentityReceipt struct {
 
 const forecastIdentityReceiptKey = "forecast/live_identity_v1"
 
+// forecastBindingRetry spaces out binding saves after a failure. Each attempt
+// can wait seconds on SQLite, and the identity poll runs every second.
+const forecastBindingRetry = 30 * time.Second
+
 // forecastPipelinePolicy names how Core composes the forecast it plans with.
 // The evaluation cohort keys on it rather than on the Core version, so error
 // bands and baselines survive updates that leave forecasting alone. Bump it
@@ -137,6 +142,7 @@ func (s *forecastSiteConfig) Configure(cfg *config.Config, catalog []drivers.Cat
 	}
 	weatherData, _ := json.Marshal(weather)
 	weatherRevision := fmt.Sprintf("%x", sha256.Sum256(weatherData))
+	var weatherReceipt []byte
 	s.mu.Lock()
 	for _, old := range earlier {
 		if err != nil || old == baseRevision || s.accepted.BaseRevision != old {
@@ -152,13 +158,10 @@ func (s *forecastSiteConfig) Configure(cfg *config.Config, catalog []drivers.Cat
 	if s.weatherRevision != weatherRevision || s.weatherSinceMS <= 0 {
 		s.weatherRevision = weatherRevision
 		s.weatherSinceMS = time.Now().UnixMilli()
-		receipt, _ := json.Marshal(struct {
+		weatherReceipt, _ = json.Marshal(struct {
 			Revision string
 			SinceMS  int64
 		}{weatherRevision, s.weatherSinceMS})
-		if err := s.store.SaveConfig("forecast/weather_generation", string(receipt)); err != nil {
-			slog.Warn("forecast weather generation not saved", "err", err)
-		}
 	}
 	v.WeatherSinceMS = s.weatherSinceMS
 	v.SiteID = s.value.SiteID
@@ -190,6 +193,12 @@ func (s *forecastSiteConfig) Configure(cfg *config.Config, catalog []drivers.Cat
 	}{weather, s.annualBase, s.annualIncludesHeating})
 	s.priorRevision = fmt.Sprintf("%x", sha256.Sum256(priorData))
 	s.mu.Unlock()
+	// Save outside s.mu: the control tick reads Snapshot and must not wait on SQLite.
+	if weatherReceipt != nil {
+		if err := s.store.SaveConfig("forecast/weather_generation", string(weatherReceipt)); err != nil {
+			slog.Warn("forecast weather generation not saved", "err", err)
+		}
+	}
 	s.RefreshIdentity(time.Now())
 	return previous != s.Snapshot().Revision
 }
@@ -198,10 +207,10 @@ func (s *forecastSiteConfig) Configure(cfg *config.Config, catalog []drivers.Cat
 // Serial/MAC identities are usable immediately. Endpoint fallback retains the
 // existing device contract after a short init grace. A previously stronger
 // identity must return before it can qualify again after restart.
-// Caller serializes this with model rebinding under forecastConfigMu.
+// Caller serializes this with model rebinding under forecastConfigMu, so the
+// saves below, made outside s.mu, keep their order.
 func (s *forecastSiteConfig) RefreshIdentity(now time.Time) bool {
 	s.mu.Lock()
-	defer s.mu.Unlock()
 	ids := make(map[string]string, len(s.required))
 	pending, pvPending := false, false
 	for name, isPV := range s.required {
@@ -250,12 +259,25 @@ func (s *forecastSiteConfig) RefreshIdentity(now time.Time) bool {
 			s.accepted.LearningBaseRevision = learningBase
 		}
 		s.bindingDirty = true
-		if encoded, err := json.Marshal(s.accepted); err == nil {
-			if err = s.store.SaveConfig(forecastIdentityReceiptKey, string(encoded)); err != nil {
-				slog.Warn("forecast identity binding not saved", "err", err)
-			} else {
-				s.bindingDirty = false
-			}
+	}
+	var binding []byte
+	if !pending && s.bindingDirty && !now.Before(s.bindingRetryAt) {
+		binding, _ = json.Marshal(s.accepted)
+	}
+	s.mu.Unlock()
+
+	// Save outside s.mu: the control tick reads Snapshot and must not wait on SQLite.
+	if binding != nil {
+		err := s.store.SaveConfig(forecastIdentityReceiptKey, string(binding))
+		s.mu.Lock()
+		if err != nil {
+			s.bindingRetryAt = now.Add(forecastBindingRetry)
+		} else {
+			s.bindingDirty = false
+		}
+		s.mu.Unlock()
+		if err != nil {
+			slog.Warn("forecast identity binding not saved", "err", err, "retry_in", forecastBindingRetry)
 		}
 	}
 	if changed {