amadan
provision: a snapshot answers keys under an account whose apps the walk could not list with that failure, not absence (#336)
Found by the RecordView invariant simulation: after a backstop walk whose ListDirs(apps/<a>/) failed, Get of any manifest or record under apps/<a>/ and List of its instances/ and sidecars/ answered absent and empty, though the view never read them. Both now answer the listing failure until a refresh or walk reads the app.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+35
−2
internal/provision/recordview_store.go+10 −2internal/provision/recordview_test.go+25 −0
diff --git a/internal/provision/recordview_store.go b/internal/provision/recordview_store.go| index 26c59d710..913edf4e8 100644 |
| --- a/internal/provision/recordview_store.go |
| +++ b/internal/provision/recordview_store.go |
| @@ -164,7 +164,9 @@ func appKey(key string) (a, b, rest string, ok bool) { |
| // lookup answers Get of a key in a cached family; handled is false for every |
| // other key. A key the view does not hold is absent: its family's listing is |
| -// complete. A record under a listing that failed answers that failure. |
| +// complete. A record under a listing that failed answers that failure, and |
| +// so does any key of an app the view never read because the walk could not |
| +// list its account's apps. |
| func (sn *ViewSnapshot) lookup(key string) (o object, handled bool) { |
| if sn == nil { |
| return object{}, false |
| @@ -193,6 +195,12 @@ func (sn *ViewSnapshot) lookup(key string) (o object, handled bool) { |
| if listErr != nil { |
| return object{err: listErr}, true |
| } |
| + if err := sn.dirErrs[a]; err != nil && sn.app(a, b) == nil { |
| + // The last walk could not list this account's apps, and no refresh |
| + // has read this one since: the view does not know, so it must not |
| + // answer absent. |
| + return object{err: err}, true |
| + } |
| if p := sn.app(a, b).object(key); p != nil { |
| return *p, true |
| } |
| @@ -245,7 +253,7 @@ func (sn *ViewSnapshot) list(prefix string) (keys []string, handled bool, err er |
| } |
| e := sn.app(a, b) |
| if e == nil { |
| - return nil, true, nil |
| + return nil, true, sn.dirErrs[a] // unknown, not empty, when the walk could not list the account's apps |
| } |
| if rest == "instances/" { |
| return keysOf(e.inst), true, e.instErr |
diff --git a/internal/provision/recordview_test.go b/internal/provision/recordview_test.go| index 110a33e6b..30592441b 100644 |
| --- a/internal/provision/recordview_test.go |
| +++ b/internal/provision/recordview_test.go |
| @@ -421,6 +421,31 @@ func TestViewCompleteMirrorsTheWalk(t *testing.T) { |
| assertMatchesWalk(t, f, sn) |
| } |
| +// A walk that cannot list one account's apps does not know them: Get and List |
| +// of their records and manifests over the snapshot answer that failure, never |
| +// "absent" or "empty", until a refresh or walk reads them. Found by the |
| +// invariant simulation (a snapshot answered absent for keys no one deleted). |
| +func TestViewUnlistedAccountIsNotAbsent(t *testing.T) { |
| + t.Parallel() |
| + f := newViewFixture(t, true, true) |
| + b := f.acct[1] |
| + f.live.setFailList("apps/" + b + "/") |
| + st := mustRebuild(t, f.view()).Store(f.live) |
| + for _, key := range []string{ManifestKey(b, "docs"), RecordKey(b, "docs", "one.docs.example.com"), SidecarRecordKey(b, "wiki", "bot")} { |
| + if _, _, err := st.Get(ctx, key); err == nil || errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("Get %s over a walk that could not list %s's apps = %v, want the listing failure", key, b, err) |
| + } |
| + } |
| + for _, p := range []string{RecordsPrefix(b, "docs"), SidecarsPrefix(b, "wiki")} { |
| + if keys, err := st.List(ctx, p); err == nil { |
| + t.Fatalf("List %s over a walk that could not list %s's apps = %v, nil; want the listing failure", p, b, keys) |
| + } |
| + } |
| + if _, err := GetManifest(ctx, st, f.acct[0], "shop"); err != nil { |
| + t.Fatalf("an account whose apps listed: %v", err) |
| + } |
| +} |
| + |
| // A refresh whose caller goes away mid-read publishes what it read, with the |
| // reads that died cached as failures: the view knows it is behind, so it must |
| // not leave the previous snapshot standing as Complete. |