amadan

carlos / platform Public

provision: prefetch each app's manifest and trash marker in reconcile

Reconcile also read, for every app with records, its instances manifest
and its trash marker, lazily and one at a time inside the record loop:
two Gets for each of ~90 apps on every serial bump. Both are now read
eight apps at a time before the loop. The lookups judge exactly what
they judged before from those results, and read for themselves only for
an app not prefetched (a route whose app has no record, at removal).
Agent notes
TestReconcileReadsAppStateConcurrentlyAndOnce fails with the manifest prefetch unused (each manifest read twice).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude f41b1248d9ccdaef5c85eaa3d95ee5b20236fe6a parent 20c5ffd
2 files changed, +112 −2
  • internal/provision/parallelreads_test.go +60 −0
  • internal/provision/reconcile.go +52 −2

CI failed — run details

diff --git a/internal/provision/parallelreads_test.go b/internal/provision/parallelreads_test.go
index e0e049e90..07b98022f 100644
--- a/internal/provision/parallelreads_test.go
+++ b/internal/provision/parallelreads_test.go
@@ -99,3 +99,63 @@ func TestReconcileStillJudgesAFailedReadPerRecord(t *testing.T) {
t.Fatal("ListAllRecords returned a set with one record unread")
}
}
+
+// appOverlapStore holds each app-level read (an app's instances manifest and
+// its trash marker) open briefly and records the most in flight at once.
+type appOverlapStore struct {
+ release.Store
+ mu sync.Mutex
+ inFlight int
+ maxFlight int
+ reads map[string]int
+}
+
+func (s *appOverlapStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ if strings.HasSuffix(key, "/instances.json") || strings.HasSuffix(key, "/deleted.json") {
+ s.mu.Lock()
+ s.inFlight++
+ if s.inFlight > s.maxFlight {
+ s.maxFlight = s.inFlight
+ }
+ if s.reads == nil {
+ s.reads = map[string]int{}
+ }
+ s.reads[key]++
+ s.mu.Unlock()
+ time.Sleep(20 * time.Millisecond)
+ s.mu.Lock()
+ s.inFlight--
+ s.mu.Unlock()
+ }
+ return s.Store.Get(ctx, key)
+}
+
+// Reconcile reads each app's manifest and trash marker several apps at a
+// time, and each one once.
+func TestReconcileReadsAppStateConcurrentlyAndOnce(t *testing.T) {
+ inner, sqid := setup(t)
+ for i := 0; i < 6; i++ {
+ app := "app" + string(rune('a'+i))
+ if err := PutManifest(ctx, inner, sqid, app, Manifest{Schema: 1, App: app, Domain: app + ".example", Pool: "tito"}); err != nil {
+ t.Fatal(err)
+ }
+ rec := acmeRecord(sqid)
+ rec.App, rec.Host, rec.Slug = app, "x."+app+".example", "x"
+ if err := CreateRecord(ctx, inner, rec); err != nil {
+ t.Fatal(err)
+ }
+ }
+ s := &appOverlapStore{Store: inner}
+ cfg := ReconcileConfig{Region: "ie", Pools: []string{"tito"}, DataDir: "/data"}
+ if err := Reconcile(ctx, s, regWith(t), cfg); err != nil {
+ t.Fatal(err)
+ }
+ if s.maxFlight < 2 {
+ t.Fatalf("app state read at most %d at a time, want several", s.maxFlight)
+ }
+ for key, n := range s.reads {
+ if n != 1 {
+ t.Fatalf("%s read %d times in one pass, want once", key, n)
+ }
+ }
+}
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go
index aaef5f92e..c54ceca19 100644
--- a/internal/provision/reconcile.go
+++ b/internal/provision/reconcile.go
@@ -841,6 +841,44 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg
// never safe to feed the converge branch below — SetSeed-ing an existing
// route to "" on a transient blip would erase a working seed for a
// reason that has nothing to do with the manifest actually changing.
+ // Each app with records costs two reads below (its manifest and its trash
+ // marker), made lazily, one at a time, inside the record loop. Both are
+ // read here first, eight at a time, for every app the records name; the
+ // lookups below then judge exactly what they judged before, from these
+ // results, and read for themselves only for an app not prefetched (a
+ // route's app with no record, at removal time).
+ type manifestRead struct {
+ m *Manifest
+ err error
+ }
+ type tombRead struct {
+ tomb *release.AppTombstone
+ err error
+ }
+ var recApps []string
+ appOf := map[string]acctApp{}
+ for _, rec := range recs {
+ k := acctApp{rec.Account, rec.App}
+ id := rec.Account + "/" + rec.App
+ if _, seen := appOf[id]; !seen {
+ appOf[id] = k
+ recApps = append(recApps, id)
+ }
+ }
+ slices.Sort(recApps)
+ manifests := map[acctApp]manifestRead{}
+ tombs := map[acctApp]tombRead{}
+ var prefetchMu sync.Mutex
+ forEach(recApps, func(id string) {
+ k := appOf[id]
+ m, merr := GetManifest(ctx, s, k.acct, k.app)
+ tomb, terr := release.ReadAppTombstone(ctx, s, k.acct, k.app)
+ prefetchMu.Lock()
+ manifests[k] = manifestRead{m, merr}
+ tombs[k] = tombRead{tomb, terr}
+ prefetchMu.Unlock()
+ })
+
seedCache := map[acctApp]string{}
seedKnown := map[acctApp]bool{}
seenManifest := map[acctApp]bool{}
@@ -850,7 +888,13 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg
return seedCache[k], seedKnown[k]
}
seenManifest[k] = true
- m, err := GetManifest(ctx, s, acct, app)
+ var m *Manifest
+ var err error
+ if r, ok := manifests[k]; ok {
+ m, err = r.m, r.err
+ } else {
+ m, err = GetManifest(ctx, s, acct, app)
+ }
if err != nil {
if errors.Is(err, os.ErrNotExist) {
seedCache[k], seedKnown[k] = "", true // absent manifest: confirmed no seed
@@ -900,7 +944,13 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg
return trashCache[k]
}
trashSeen[k] = true
- tomb, err := release.ReadAppTombstone(ctx, s, acct, app)
+ var tomb *release.AppTombstone
+ var err error
+ if r, ok := tombs[k]; ok {
+ tomb, err = r.tomb, r.err
+ } else {
+ tomb, err = release.ReadAppTombstone(ctx, s, acct, app)
+ }
switch {
case err != nil:
log.Printf("provision: trash marker for %s/%s unreadable: %v — not minting for this app", acct, app, err)