fix(recovery): make failed restores discardable
- commit
- 8849f70e2a835005be10c83faefb9925b5c4a1b9
- author
- brice · · 2026-07-26 10:51 UTC
- parent
- cbe62e5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +15 -15
M internal/usecase/daemon/coderabbit_regression_test.go +6 -4
| @@ -11,7 +11,7 @@ import ( | |||
| 11 | 11 | "github.com/bnema/vev/internal/ports" | |
| 12 | 12 | ) | |
| 13 | 13 | ||
| 14 | - | func TestRestoreTransientLoadFailurePreservesHealthyAuthority(t *testing.T) { | |
| 14 | + | func TestRestoreTransientLoadFailureBecomesDiscardable(t *testing.T) { | |
| 15 | 15 | record := durableRecoveryRecord(0) | |
| 16 | 16 | repository := &durableRecoveryRepository{ | |
| 17 | 17 | errors: map[string]error{record.Name: ports.ErrBudgetExhausted}, | |
| @@ -25,12 +25,14 @@ func TestRestoreTransientLoadFailurePreservesHealthyAuthority(t *testing.T) { | |||
| 25 | 25 | persisted, ok, err := catalogue.Record(record.Name) | |
| 26 | 26 | require.NoError(t, err) | |
| 27 | 27 | require.True(t, ok) | |
| 28 | - | require.Equal(t, domain.RecoveryHealthy, persisted.RecoveryState) | |
| 28 | + | require.Equal(t, domain.RecoveryDegraded, persisted.RecoveryState) | |
| 29 | + | require.Equal(t, "checkpoint load failed", persisted.DegradedReason) | |
| 29 | 30 | d.mu.Lock() | |
| 30 | 31 | entry := d.stopped[record.Name] | |
| 31 | 32 | d.mu.Unlock() | |
| 32 | - | require.Equal(t, runtimeRestoring, entry.state) | |
| 33 | - | require.NotEqual(t, runtimeDegraded, entry.state) | |
| 33 | + | require.Equal(t, runtimeDegraded, entry.state) | |
| 34 | + | require.Equal(t, persisted, entry.record) | |
| 35 | + | require.Equal(t, ports.SessionDegraded, listSessions(t, d).Sessions[0].State) | |
| 34 | 36 | select { | |
| 35 | 37 | case <-entry.restoreDone: | |
| 36 | 38 | default: | |
| @@ -11,7 +11,7 @@ import ( | |||||
| 11 | "github.com/bnema/vev/internal/ports" | 11 | "github.com/bnema/vev/internal/ports" | ||
| 12 | ) | 12 | ) | ||
| 13 | 13 | ||||
| 14 | - | func TestRestoreTransientLoadFailurePreservesHealthyAuthority(t *testing.T) { | 14 | + | func TestRestoreTransientLoadFailureBecomesDiscardable(t *testing.T) { |
| 15 | record := durableRecoveryRecord(0) | 15 | record := durableRecoveryRecord(0) | ||
| 16 | repository := &durableRecoveryRepository{ | 16 | repository := &durableRecoveryRepository{ | ||
| 17 | errors: map[string]error{record.Name: ports.ErrBudgetExhausted}, | 17 | errors: map[string]error{record.Name: ports.ErrBudgetExhausted}, | ||
| @@ -25,12 +25,14 @@ func TestRestoreTransientLoadFailurePreservesHealthyAuthority(t *testing.T) { | |||||
| 25 | persisted, ok, err := catalogue.Record(record.Name) | 25 | persisted, ok, err := catalogue.Record(record.Name) | ||
| 26 | require.NoError(t, err) | 26 | require.NoError(t, err) | ||
| 27 | require.True(t, ok) | 27 | require.True(t, ok) | ||
| 28 | - | require.Equal(t, domain.RecoveryHealthy, persisted.RecoveryState) | 28 | + | require.Equal(t, domain.RecoveryDegraded, persisted.RecoveryState) |
| 29 | + | require.Equal(t, "checkpoint load failed", persisted.DegradedReason) | |||
| 29 | d.mu.Lock() | 30 | d.mu.Lock() | ||
| 30 | entry := d.stopped[record.Name] | 31 | entry := d.stopped[record.Name] | ||
| 31 | d.mu.Unlock() | 32 | d.mu.Unlock() | ||
| 32 | - | require.Equal(t, runtimeRestoring, entry.state) | 33 | + | require.Equal(t, runtimeDegraded, entry.state) |
| 33 | - | require.NotEqual(t, runtimeDegraded, entry.state) | 34 | + | require.Equal(t, persisted, entry.record) |
| 35 | + | require.Equal(t, ports.SessionDegraded, listSessions(t, d).Sessions[0].State) | |||
| 34 | select { | 36 | select { | ||
| 35 | case <-entry.restoreDone: | 37 | case <-entry.restoreDone: | ||
| 36 | default: | 38 | default: | ||
M internal/usecase/daemon/durable_recovery_test.go +0 -1
| @@ -360,7 +360,6 @@ func (s explicitRecoveryStub) Discard(context.Context, string, string) (domain.C | |||
| 360 | 360 | } | |
| 361 | 361 | ||
| 362 | 362 | type contextRecoveryStub struct { | |
| 363 | - | explicitRecoveryStub | |
| 364 | 363 | wantKey any | |
| 365 | 364 | want any | |
| 366 | 365 | seen bool | |
| @@ -360,7 +360,6 @@ func (s explicitRecoveryStub) Discard(context.Context, string, string) (domain.C | |||||
| 360 | } | 360 | } | ||
| 361 | 361 | ||||
| 362 | type contextRecoveryStub struct { | 362 | type contextRecoveryStub struct { | ||
| 363 | - | explicitRecoveryStub | |||
| 364 | wantKey any | 363 | wantKey any | ||
| 365 | want any | 364 | want any | ||
| 366 | seen bool | 365 | seen bool | ||
M internal/usecase/daemon/snapshot_restore_incremental.go +9 -10
| @@ -135,13 +135,9 @@ func (d *Daemon) setStoppedRecovery(record domain.CatalogueRecord, state runtime | |||
| 135 | 135 | } | |
| 136 | 136 | ||
| 137 | 137 | func (d *Daemon) finishRecordRestore(record domain.CatalogueRecord, restoreErr error, done chan struct{}) { | |
| 138 | - | d.finishRecordRestoreAttempt(record, restoreErr, done, false) | |
| 139 | - | } | |
| 140 | - | ||
| 141 | - | func (d *Daemon) finishRecordRestoreAttempt(record domain.CatalogueRecord, restoreErr error, done chan struct{}, resetRetryable bool) { | |
| 142 | 138 | retryable := errors.Is(restoreErr, errRetryableRestoreLoad) | |
| 143 | 139 | degraded := false | |
| 144 | - | if restoreErr != nil && !retryable && d.catalogue != nil { | |
| 140 | + | if restoreErr != nil && d.catalogue != nil { | |
| 145 | 141 | current, ok, readErr := d.catalogue.Record(record.Name) | |
| 146 | 142 | switch { | |
| 147 | 143 | case readErr != nil: | |
| @@ -150,7 +146,10 @@ func (d *Daemon) finishRecordRestoreAttempt(record domain.CatalogueRecord, resto | |||
| 150 | 146 | record = current | |
| 151 | 147 | record.RecoveryState = domain.RecoveryDegraded | |
| 152 | 148 | record.DegradedReason = "checkpoint validation failed" | |
| 153 | - | if errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded) { | |
| 149 | + | switch { | |
| 150 | + | case retryable: | |
| 151 | + | record.DegradedReason = "checkpoint load failed" | |
| 152 | + | case errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded): | |
| 154 | 153 | record.DegradedReason = "restore interrupted" | |
| 155 | 154 | } | |
| 156 | 155 | if err := d.catalogue.Replace(record.Name, record); err != nil { | |
| @@ -173,16 +172,16 @@ func (d *Daemon) finishRecordRestoreAttempt(record domain.CatalogueRecord, resto | |||
| 173 | 172 | entry.lastUsedSeq = record.LastUsedSeq | |
| 174 | 173 | entry.tabNames = append([]string(nil), record.TabNames...) | |
| 175 | 174 | d.stopped[record.Name] = entry | |
| 176 | - | case retryable && resetRetryable && entry.state == runtimeRestoring && entry.restoreDone == done: | |
| 177 | - | entry.state = runtimeDegraded | |
| 178 | - | d.stopped[record.Name] = entry | |
| 179 | 175 | } | |
| 180 | 176 | } | |
| 181 | 177 | closeRuntimeRestoreDoneLocked(done) | |
| 182 | 178 | d.mu.Unlock() | |
| 183 | 179 | if degraded { | |
| 184 | 180 | reasonCode := "checkpoint-invalid" | |
| 185 | - | if errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded) { | |
| 181 | + | switch { | |
| 182 | + | case retryable: | |
| 183 | + | reasonCode = "checkpoint-load-failed" | |
| 184 | + | case errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded): | |
| 186 | 185 | reasonCode = "restore-interrupted" | |
| 187 | 186 | } | |
| 188 | 187 | d.logSessionDegraded(record, reasonCode) | |
| @@ -135,13 +135,9 @@ func (d *Daemon) setStoppedRecovery(record domain.CatalogueRecord, state runtime | |||||
| 135 | } | 135 | } | ||
| 136 | 136 | ||||
| 137 | func (d *Daemon) finishRecordRestore(record domain.CatalogueRecord, restoreErr error, done chan struct{}) { | 137 | func (d *Daemon) finishRecordRestore(record domain.CatalogueRecord, restoreErr error, done chan struct{}) { | ||
| 138 | - | d.finishRecordRestoreAttempt(record, restoreErr, done, false) | |||
| 139 | - | } | |||
| 140 | - | ||||
| 141 | - | func (d *Daemon) finishRecordRestoreAttempt(record domain.CatalogueRecord, restoreErr error, done chan struct{}, resetRetryable bool) { | |||
| 142 | retryable := errors.Is(restoreErr, errRetryableRestoreLoad) | 138 | retryable := errors.Is(restoreErr, errRetryableRestoreLoad) | ||
| 143 | degraded := false | 139 | degraded := false | ||
| 144 | - | if restoreErr != nil && !retryable && d.catalogue != nil { | 140 | + | if restoreErr != nil && d.catalogue != nil { |
| 145 | current, ok, readErr := d.catalogue.Record(record.Name) | 141 | current, ok, readErr := d.catalogue.Record(record.Name) | ||
| 146 | switch { | 142 | switch { | ||
| 147 | case readErr != nil: | 143 | case readErr != nil: | ||
| @@ -150,7 +146,10 @@ func (d *Daemon) finishRecordRestoreAttempt(record domain.CatalogueRecord, resto | |||||
| 150 | record = current | 146 | record = current | ||
| 151 | record.RecoveryState = domain.RecoveryDegraded | 147 | record.RecoveryState = domain.RecoveryDegraded | ||
| 152 | record.DegradedReason = "checkpoint validation failed" | 148 | record.DegradedReason = "checkpoint validation failed" | ||
| 153 | - | if errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded) { | 149 | + | switch { |
| 150 | + | case retryable: | |||
| 151 | + | record.DegradedReason = "checkpoint load failed" | |||
| 152 | + | case errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded): | |||
| 154 | record.DegradedReason = "restore interrupted" | 153 | record.DegradedReason = "restore interrupted" | ||
| 155 | } | 154 | } | ||
| 156 | if err := d.catalogue.Replace(record.Name, record); err != nil { | 155 | if err := d.catalogue.Replace(record.Name, record); err != nil { | ||
| @@ -173,16 +172,16 @@ func (d *Daemon) finishRecordRestoreAttempt(record domain.CatalogueRecord, resto | |||||
| 173 | entry.lastUsedSeq = record.LastUsedSeq | 172 | entry.lastUsedSeq = record.LastUsedSeq | ||
| 174 | entry.tabNames = append([]string(nil), record.TabNames...) | 173 | entry.tabNames = append([]string(nil), record.TabNames...) | ||
| 175 | d.stopped[record.Name] = entry | 174 | d.stopped[record.Name] = entry | ||
| 176 | - | case retryable && resetRetryable && entry.state == runtimeRestoring && entry.restoreDone == done: | |||
| 177 | - | entry.state = runtimeDegraded | |||
| 178 | - | d.stopped[record.Name] = entry | |||
| 179 | } | 175 | } | ||
| 180 | } | 176 | } | ||
| 181 | closeRuntimeRestoreDoneLocked(done) | 177 | closeRuntimeRestoreDoneLocked(done) | ||
| 182 | d.mu.Unlock() | 178 | d.mu.Unlock() | ||
| 183 | if degraded { | 179 | if degraded { | ||
| 184 | reasonCode := "checkpoint-invalid" | 180 | reasonCode := "checkpoint-invalid" | ||
| 185 | - | if errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded) { | 181 | + | switch { |
| 182 | + | case retryable: | |||
| 183 | + | reasonCode = "checkpoint-load-failed" | |||
| 184 | + | case errors.Is(restoreErr, context.Canceled) || errors.Is(restoreErr, context.DeadlineExceeded): | |||
| 186 | reasonCode = "restore-interrupted" | 185 | reasonCode = "restore-interrupted" | ||
| 187 | } | 186 | } | ||
| 188 | d.logSessionDegraded(record, reasonCode) | 187 | d.logSessionDegraded(record, reasonCode) | ||