amadan

carlos / platform Public

edge: one registry listing per watch tick builds both the table and the holds; every table-and-holds reader reads the paired snapshot (Task 2 review 2 round)

- Item 1: watchHolds listed the rows after the fingerprint's own read
  and re-based the holds on them against the older table, so a held row
  turning ready plus an alias stamp between the two reads could answer
  neither. Reload is now listRegistry (rows, paths, aliases, with a
  listingGen generation) then reloadFrom; the watch lists once and
  rebaseHolds rebuilds the holds alone only from a listing of the
  published table's generation, else runs reloadFrom on that listing.
  The snapshot records rowsGen and tableGen.
  TestWatchBuildsTableAndHoldsFromOneListing flips the row and the stamp
  between the fingerprint read and the rebuild (afterFingerprint seam).
- Item 2: currentHosts and warmSet (through certNamesOf) read the table
  and the holds from servingPair; localHoldsFor is gone.
  TestCertificateReadersReadThePairedSnapshot pauses a ready-to-held
  Reload.
- Item 3: TestHoldingAliasFollowsDNSStamp asserts the holding resolution
  and Allowed separately.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
claude b2c76eff4044bdf6da52283ff9a697e0fb1ccda1 parent 5f8a63f
4 files changed, +243 −61
  • internal/edge/certwarm.go +5 −2
  • internal/edge/decision_test.go +89 −2
  • internal/edge/edge.go +49 −26
  • internal/edge/holding.go +100 −31
diff --git a/internal/edge/certwarm.go b/internal/edge/certwarm.go
index 16d56ff31..56d16fb4b 100644
--- a/internal/edge/certwarm.go
+++ b/internal/edge/certwarm.go
@@ -367,7 +367,10 @@ func (e *Edge) warmSet(appsDomain string, parents func() []string) []string {
seen[h] = true
out = append(out, h)
}
- for h := range e.current() {
+ // The table and the holds from one read (servingPair): a Reload turning
+ // a ready row held is seen whole, so the name never drops out between.
+ table, holds := e.servingPair()
+ for h := range table {
consider(h)
}
for _, h := range e.homes.Load().ServedHosts() {
@@ -376,7 +379,7 @@ func (e *Edge) warmSet(appsDomain string, parents func() []string) []string {
// The names this box answers with the holding page (self-serve moves
// plan 3): its own holds and the remote live holds' winners, under the
// same exclusions. Never served evidence; only a certificate to keep.
- for _, h := range e.certNames() {
+ for _, h := range e.certNamesOf(holds) {
consider(h)
}
sort.Strings(out)
diff --git a/internal/edge/decision_test.go b/internal/edge/decision_test.go
index 3de0f4e26..60db37329 100644
--- a/internal/edge/decision_test.go
+++ b/internal/edge/decision_test.go
@@ -723,8 +723,13 @@ func TestHoldingAliasFollowsDNSStamp(t *testing.T) {
t.Fatal("test bug: the fingerprint must not see a DNS stamp")
}
sc.e.watchTick(&st)
- if aliasHeld() != step.want {
- t.Fatalf("after the stamp turned %s: held %v, want %v", step.dns, aliasHeld(), step.want)
+ // The holding resolution and the TLS admission, each on its own: a
+ // revocation must take both away.
+ if held := sc.e.resolve("shop.example.net", sc.now).kind == decHolding; held != step.want {
+ t.Fatalf("after the stamp turned %s: holding resolution %v, want %v", step.dns, held, step.want)
+ }
+ if allowed := sc.e.Allowed("shop.example.net"); allowed != step.want {
+ t.Fatalf("after the stamp turned %s: Allowed %v, want %v", step.dns, allowed, step.want)
}
if step.want {
wantPage(t, sc.serve("shop.example.net", "/", ordinary), "the alias once pointing")
@@ -797,3 +802,85 @@ func TestReloadPublishesReadyAndHoldsAtomically(t *testing.T) {
})
wantBody(t, sc.serve("shop.example.com", "/", ordinary), "shop", "ready, after the reload")
}
+
+// One listing per watch tick (final review 2, item 1): a held row turning
+// ready and its alias's DNS stamp both landing between the tick's
+// fingerprint read and its hold rebuild must not pair the newer rows with
+// the older table. The tick's one listing decides: rows whose table inputs
+// differ from the published table's take the full Reload path from that
+// listing, so the table and the holds come from one listing generation.
+func TestWatchBuildsTableAndHoldsFromOneListing(t *testing.T) {
+ sc := newScene(t, "annie")
+ sc.heldRow("shop.example.com", "bab", "shop", "mv_r", 1, 1)
+ sc.alias("shop.example.net", "shop.example.com", "pending", true)
+ sc.view(map[string]HoldState{"shop.example.com": moveR})
+ var st watchState
+ sc.e.watchTick(&st)
+ wantPage(t, sc.serve("shop.example.com", "/", ordinary), "held, before")
+ sc.e.afterFingerprint = func() {
+ sc.e.afterFingerprint = nil
+ if _, err := sc.reg.ReleaseHold("shop.example.com", "mv_r"); err != nil {
+ t.Error(err)
+ }
+ if err := sc.reg.SetStatus("shop.example.com", "ready"); err != nil {
+ t.Error(err)
+ }
+ if err := sc.reg.SetAliasDNSStatus("shop.example.net", "pointing", time.Now().Unix()); err != nil {
+ t.Error(err)
+ }
+ }
+ sc.e.watchTick(&st)
+ for _, h := range []string{"shop.example.com", "shop.example.net"} {
+ w := sc.serve(h, "/", ordinary)
+ if !isPage(w) && !(w.Code == http.StatusOK && w.Body.String() == "the held row's backend") {
+ t.Fatalf("%s after the tick = %d %q: the ready answer or the page, never neither", h, w.Code, w.Body.String())
+ }
+ }
+ hs := sc.e.holds.Load()
+ if hs == nil || hs.rowsGen == "" || hs.rowsGen != hs.tableGen {
+ t.Fatalf("published holds from listing %q over a table from listing %q: one listing generation", hs.rowsGen, hs.tableGen)
+ }
+}
+
+// Every reader of the table and the holds reads them as one pair (final
+// review 2, item 2): mid-reload, a ready row turning held keeps its wildcard
+// parent input (currentHosts) and its certificate eligibility (warmSet).
+func TestCertificateReadersReadThePairedSnapshot(t *testing.T) {
+ sc := newScene(t, "annie")
+ sc.route("cart.bab.apps.example.com", "cart", "bab", "shop", func(r *registry.Route) { r.Platform = true })
+ sc.route("shop.example.org", "shop", "bab", "shop")
+ sc.view(map[string]HoldState{"cart.bab.apps.example.com": moveR, "shop.example.org": {MoveID: "mv_s"}})
+ for host, op := range map[string]string{"cart.bab.apps.example.com": "mv_r", "shop.example.org": "mv_s"} {
+ if err := sc.reg.SetHold(host, "moving", op); err != nil {
+ t.Fatal(err)
+ }
+ }
+ check := func(when string) {
+ t.Helper()
+ if !slices.Contains(sc.e.currentHosts(), "cart.bab.apps.example.com") {
+ t.Fatalf("%s: currentHosts dropped the platform host", when)
+ }
+ if !slices.Contains(sc.e.warmSet("apps.example.com", nil), "shop.example.org") {
+ t.Fatalf("%s: warmSet dropped the custom domain", when)
+ }
+ }
+ paused, release := make(chan struct{}), make(chan struct{})
+ var once sync.Once
+ sc.e.holdsRead = func(*HoldSnapshot) {
+ once.Do(func() {
+ close(paused)
+ <-release
+ })
+ }
+ done := make(chan error, 1)
+ go func() { done <- sc.e.Reload() }()
+ <-paused
+ check("mid-reload")
+ close(release)
+ if err := <-done; err != nil {
+ t.Fatal(err)
+ }
+ sc.e.holdsRead = nil
+ check("after the reload")
+ wantPage(t, sc.serve("shop.example.org", "/", ordinary), "the held custom domain")
+}
diff --git a/internal/edge/edge.go b/internal/edge/edge.go
index c8b740eed..7f8af5cf1 100644
--- a/internal/edge/edge.go
+++ b/internal/edge/edge.go
@@ -307,11 +307,13 @@ type Edge struct {
holdInputs atomic.Pointer[holdInputs]
holdsMu sync.Mutex
holds atomic.Pointer[holdSnapshot]
- // holdsRead and afterResolve are test seams: run inside a hold rebuild
- // after its inputs are read, and in ServeHTTP right after the request's
- // decision. nil in production.
- holdsRead func(*HoldSnapshot)
- afterResolve func()
+ // holdsRead, afterResolve and afterFingerprint are test seams: run
+ // inside a hold rebuild after its inputs are read, in ServeHTTP right
+ // after the request's decision, and in the watch right after its
+ // fingerprint read. nil in production.
+ holdsRead func(*HoldSnapshot)
+ afterResolve func()
+ afterFingerprint func()
}
// RunOption configures the Edge that Run/RunWithActivator* builds, before it
@@ -638,10 +640,21 @@ func (e *Edge) statusHandlerFor(rt registry.Route) http.Handler {
// Reload rebuilds the host→backend map from the registry and swaps it in
// atomically. Safe to call while serving.
func (e *Edge) Reload() error {
- routes, err := e.reg.List()
+ l, err := e.listRegistry()
if err != nil {
return err
}
+ return e.reloadFrom(l)
+}
+
+// reloadFrom is Reload over one listing: the table, the fleet tables and
+// the holds are built from l and nothing re-reads the rows, the paths or
+// the aliases, so the table and the holds share l's generation (the watch's
+// rebaseHolds runs it from its own listing). Its reads of the registry's
+// rows, path routes and alias rows are listRegistry's, all done before
+// anything is built, under the same all-or-nothing error contract.
+func (e *Edge) reloadFrom(l *registryListing) error {
+ routes := l.routes
m := make(map[string]upstream, len(routes))
// The hosts the activator should manage after this reload — feeds both the
// per-upstream gate and the retire pass below.
@@ -920,10 +933,7 @@ func (e *Edge) Reload() error {
// already in m by this point, but resolving now would freeze a copy of
// the target's entry, and the target's own row may still be rewritten by
// the alias fold below.
- paths, err := e.reg.ListPaths()
- if err != nil {
- return err
- }
+ paths := l.paths
for _, p := range paths {
up, ok := m[p.Host]
if !ok {
@@ -944,10 +954,7 @@ func (e *Edge) Reload() error {
// local) is skipped — never a 502 route to nowhere. Errors abort the
// reload like every other registry read here: a transient failure must
// leave the old table serving.
- aliases, err := e.reg.ListAliases()
- if err != nil {
- return err
- }
+ aliases := l.aliases
for _, a := range aliases {
if _, taken := m[a.Alias]; taken {
log.Printf("edge: alias %s shadowed by a real route — skipped", a.Alias)
@@ -1018,7 +1025,7 @@ func (e *Edge) Reload() error {
// land here with the view unchanged, and the request path never sees
// the new table without its holds. Before the reload hook, so the
// wildcard manager's re-derivation sees them.
- e.installTable(routes, aliases, m)
+ e.installTable(l, m)
// Reconcile the activator with the new route set: a host un-flagged (or
// removed) since the last reload may still have a live child holding its
// socket and lease — retire it so a systemd unit can take over cleanly.
@@ -1101,14 +1108,16 @@ func (e *Edge) current() map[string]upstream {
// plan 3): a held row has left the table, and its account's parent must stay
// loaded through the move.
func (e *Edge) currentHosts() []string {
- m := e.current()
+ // The table and the holds from one read (servingPair), so a Reload that
+ // turns a platform row held never drops it between the two.
+ m, holds := e.servingPair()
out := make([]string, 0, len(m))
for h, up := range m {
if (up.route.Platform || up.route.Provisioned) && !up.alias {
out = append(out, h)
}
}
- for _, h := range e.holds.Load().platformNames() {
+ for _, h := range holds.platformNames() {
if _, ok := m[h]; !ok {
out = append(out, h)
}
@@ -2479,10 +2488,15 @@ type watchState struct {
// beat, and a reload when the fingerprint moved. While the box holds a row,
// it also watches the one input of the local holds the fingerprint leaves
// out on purpose (an explicit alias's DNS stamp, holdAliasDigest): when only
-// that moved, it re-lists the rows here, outside the view's lock, and
-// re-bases the holds on them (refreshHoldRows) without a Reload.
+// that moved, it lists the registry once, here, outside the view's lock,
+// and builds from that one listing (watchHolds, rebaseHolds): the holds
+// alone when the listing is of the published table's generation, the full
+// reload path otherwise.
func (e *Edge) watchTick(st *watchState) {
fp, err := e.reg.Fingerprint()
+ if e.afterFingerprint != nil {
+ e.afterFingerprint()
+ }
if err != nil {
st.errs++
if st.errs == 1 || st.errs%300 == 0 {
@@ -2499,9 +2513,9 @@ func (e *Edge) watchTick(st *watchState) {
}
// Read before any reload: a stamp landing after it is caught on the next
// tick, never baselined away.
- holds, aliases, armed, herr := e.holdAliasDigest()
+ holds, armed, herr := e.holdAliasDigest()
if fp == st.last {
- e.watchHolds(st, holds, aliases, armed, herr)
+ e.watchHolds(st, holds, armed, herr)
return
}
// last is baselined only on a SUCCESSFUL reload: a fleet detach is a
@@ -2531,7 +2545,14 @@ func (e *Edge) watchTick(st *watchState) {
// watchHolds is watchTick's holds half, on a tick whose fingerprint did not
// move: a changed digest re-lists the rows and re-bases the holds. A failed
// read leaves the baseline, so the next tick retries.
-func (e *Edge) watchHolds(st *watchState, holds string, aliases []registry.RouteAlias, armed bool, herr error) {
+//
+// What it installs comes from ONE listing (review 2): the digest only says
+// whether to list; the rows, paths and aliases are then read once, the
+// baseline is that listing's own alias digest, and rebaseHolds either
+// rebuilds the holds from it (the published table's generation) or runs the
+// full reload path from it. A row that changed between the fingerprint read
+// and this listing therefore reaches the table and the holds together.
+func (e *Edge) watchHolds(st *watchState, holds string, armed bool, herr error) {
if !armed {
st.lastHolds = ""
return
@@ -2539,9 +2560,12 @@ func (e *Edge) watchHolds(st *watchState, holds string, aliases []registry.Route
if herr == nil && holds == st.lastHolds {
return
}
- var routes []registry.Route
+ var l *registryListing
+ if herr == nil {
+ l, herr = e.listRegistry()
+ }
if herr == nil {
- routes, herr = e.reg.List()
+ _, herr = e.rebaseHolds(l)
}
if herr != nil {
st.holdErrs++
@@ -2551,8 +2575,7 @@ func (e *Edge) watchHolds(st *watchState, holds string, aliases []registry.Route
return
}
st.holdErrs = 0
- e.refreshHoldRows(routes, aliases)
- st.lastHolds = holds
+ st.lastHolds = aliasDigest(l.aliases)
}
// versionHeader wraps next so every response also carries
diff --git a/internal/edge/holding.go b/internal/edge/holding.go
index 54c09ad0e..aca896a60 100644
--- a/internal/edge/holding.go
+++ b/internal/edge/holding.go
@@ -4,6 +4,7 @@ import (
"bytes"
"crypto/sha256"
"encoding/hex"
+ "encoding/json"
"fmt"
"html/template"
"net/http"
@@ -191,6 +192,11 @@ func (e *Edge) holdingAnswer(w http.ResponseWriter, r *http.Request, h *holdingV
type holdSnapshot struct {
cands map[string][]candidate
table map[string]upstream
+ // rowsGen and tableGen are the listing generations (listingGen) of the
+ // rows the holds were built from and of the rows the table was built
+ // from. They are always equal: a holds rebuild never pairs rows with a
+ // table another listing built.
+ rowsGen, tableGen string
}
// holdInputs is the registry as the latest Reload read it: every row, every
@@ -201,6 +207,56 @@ type holdInputs struct {
routes []registry.Route
aliases []registry.RouteAlias
table map[string]upstream
+ // rowsGen is the generation of the listing routes and aliases came
+ // from, tableGen that of the listing table was built from; equal by
+ // construction (installTable sets both, rebaseHolds only re-bases a
+ // listing of the table's own generation).
+ rowsGen, tableGen string
+}
+
+// registryListing is one read of everything the table is built from: the
+// rows, the path routes and the alias rows (Reload's three reads), and its
+// generation (listingGen).
+type registryListing struct {
+ routes []registry.Route
+ paths []registry.RoutePath
+ aliases []registry.RouteAlias
+ gen string
+}
+
+// listRegistry reads the table's inputs once.
+func (e *Edge) listRegistry() (*registryListing, error) {
+ routes, err := e.reg.List()
+ if err != nil {
+ return nil, err
+ }
+ paths, err := e.reg.ListPaths()
+ if err != nil {
+ return nil, err
+ }
+ aliases, err := e.reg.ListAliases()
+ if err != nil {
+ return nil, err
+ }
+ return &registryListing{routes: routes, paths: paths, aliases: aliases, gen: listingGen(routes, paths, aliases)}, nil
+}
+
+// listingGen is a listing's table generation: a digest of every field of
+// every row and path route, and of every alias row but its DNS and
+// certificate stamps, which no table entry reads. Two listings of one
+// generation build the same table, so holds built from either may be paired
+// with it; any other difference, a row's status included, is a new
+// generation.
+func listingGen(routes []registry.Route, paths []registry.RoutePath, aliases []registry.RouteAlias) string {
+ h := sha256.New()
+ enc := json.NewEncoder(h)
+ _ = enc.Encode(routes)
+ _ = enc.Encode(paths)
+ for _, a := range aliases {
+ a.DNSStatus, a.DNSCheckedAt, a.CertExpiresAt = "", 0, 0
+ _ = enc.Encode(a)
+ }
+ return hex.EncodeToString(h.Sum(nil))
}
// SetHoldView is the offline view's onChange (internal/host's
@@ -220,7 +276,7 @@ func (e *Edge) SetHoldView(s HoldSnapshot) {
}
// RefreshHolds rebuilds the local holds from the latest view and the rows
-// the latest Reload (or the watch's holds refresh, refreshHoldRows) listed,
+// the latest Reload (or the watch's holds re-base, rebaseHolds) listed,
// and installs them. The inputs are read under holdsMu, the
// lock that installs, so rebuilds install in the order they read and none
// installs older inputs than one before it; no revision is compared (a
@@ -253,7 +309,7 @@ func (e *Edge) rebuildHoldsLocked() bool {
}
next := localHolds(view, in)
if in != nil {
- next.table = in.table
+ next.table, next.rowsGen, next.tableGen = in.table, in.rowsGen, in.tableGen
}
prev := e.holds.Load()
e.holds.Store(next)
@@ -265,33 +321,38 @@ func (e *Edge) rebuildHoldsLocked() bool {
// holdSnapshot (the request path's pair), with the table also stored for
// every other reader of current(). It reports whether the platform hold
// names changed.
-func (e *Edge) installTable(routes []registry.Route, aliases []registry.RouteAlias, m map[string]upstream) bool {
+func (e *Edge) installTable(l *registryListing, m map[string]upstream) bool {
e.holdsMu.Lock()
defer e.holdsMu.Unlock()
e.upstreams.Store(&m)
- e.holdInputs.Store(&holdInputs{routes: routes, aliases: aliases, table: m})
+ e.holdInputs.Store(&holdInputs{routes: l.routes, aliases: l.aliases, table: m, rowsGen: l.gen, tableGen: l.gen})
return e.rebuildHoldsLocked()
}
-// refreshHoldRows re-bases the holds on freshly listed rows and alias rows
-// against the table already published (final review F1): the watch calls it,
-// outside the view's lock, when the only registry change is one the
-// fingerprint leaves out (an alias's DNS stamp). The table is not rebuilt.
-// Before the first Reload there is nothing to re-base.
-func (e *Edge) refreshHoldRows(routes []registry.Route, aliases []registry.RouteAlias) {
+// rebaseHolds is the watch's holds rebuild from its one listing l (final
+// review F1, and review 2's one-listing rule): when l is of the published
+// table's generation (only what no table entry reads moved, an alias's DNS
+// stamp), the holds are rebuilt from l against that table, outside the
+// view's lock and without a Reload; otherwise the full reload path runs
+// from l, so the table and the holds are installed together from it. Rows
+// are never paired with a table another listing built. reloaded reports
+// which path ran.
+func (e *Edge) rebaseHolds(l *registryListing) (reloaded bool, err error) {
e.holdsMu.Lock()
in := e.holdInputs.Load()
- changed := false
- if in != nil {
- e.holdInputs.Store(&holdInputs{routes: routes, aliases: aliases, table: in.table})
- changed = e.rebuildHoldsLocked()
+ if in == nil || in.tableGen != l.gen {
+ e.holdsMu.Unlock()
+ return true, e.reloadFrom(l)
}
+ e.holdInputs.Store(&holdInputs{routes: l.routes, aliases: l.aliases, table: in.table, rowsGen: l.gen, tableGen: in.tableGen})
+ changed := e.rebuildHoldsLocked()
e.holdsMu.Unlock()
if changed {
if fn := e.reloadHook.Load(); fn != nil {
go (*fn)()
}
}
+ return false, nil
}
// servingPair is the request path's table and local holds, from one read
@@ -304,28 +365,34 @@ func (e *Edge) servingPair() (map[string]upstream, *holdSnapshot) {
return e.current(), e.holds.Load()
}
-// holdAliasDigest is the watch's second input (final review F1): a digest
-// of every explicit alias row's (Alias, Host, DNSStatus, Explicit), which a
-// held row's alias holds depend on and the registry fingerprint leaves out.
-// armed is false, and nothing is read, while the rows the holds were last
-// built from hold nothing: then no alias can be a local hold, and a row
-// turning held moves the fingerprint.
-func (e *Edge) holdAliasDigest() (digest string, aliases []registry.RouteAlias, armed bool, err error) {
+// holdAliasDigest is the watch's trigger for the holds (final review F1): a
+// digest of every explicit alias row's (Alias, Host, DNSStatus, Explicit),
+// which a held row's alias holds depend on and the registry fingerprint
+// leaves out. armed is false, and nothing is read, while the rows the holds
+// were last built from hold nothing: then no alias can be a local hold, and
+// a row turning held moves the fingerprint. It only decides whether to
+// list: what is installed comes from the tick's one listing (rebaseHolds).
+func (e *Edge) holdAliasDigest() (digest string, armed bool, err error) {
in := e.holdInputs.Load()
if in == nil || !slices.ContainsFunc(in.routes, func(r registry.Route) bool { return registry.IsHold(r.Status) }) {
- return "", nil, false, nil
+ return "", false, nil
}
- aliases, err = e.reg.ListAliases()
+ aliases, err := e.reg.ListAliases()
if err != nil {
- return "", nil, true, err
+ return "", true, err
}
+ return aliasDigest(aliases), true, nil
+}
+
+// aliasDigest is holdAliasDigest's digest over aliases.
+func aliasDigest(aliases []registry.RouteAlias) string {
h := sha256.New()
for _, a := range aliases {
if a.Explicit {
fmt.Fprintf(h, "%s\x1f%s\x1f%s\x1f%t\x1e", a.Alias, a.Host, a.DNSStatus, a.Explicit)
}
}
- return hex.EncodeToString(h.Sum(nil)), aliases, true, nil
+ return hex.EncodeToString(h.Sum(nil))
}
// localHolds builds the box's holding candidates: none without a view, or
@@ -405,11 +472,6 @@ func (h *holdSnapshot) platformNames() []string {
return out
}
-// localHoldsFor is name's local holding candidates.
-func (e *Edge) localHoldsFor(name string) []candidate {
- return e.holds.Load().candsFor(name)
-}
-
// candsFor is name's candidates in h, nil-receiver-safe.
func (h *holdSnapshot) candsFor(name string) []candidate {
if h == nil {
@@ -424,7 +486,14 @@ func (h *holdSnapshot) candsFor(name string) []candidate {
// cert warm considers them after the served names (warmSet); they are never
// served evidence.
func (e *Edge) certNames() []string {
- out := e.holds.Load().names()
+ _, holds := e.servingPair()
+ return e.certNamesOf(holds)
+}
+
+// certNamesOf is certNames over holds, the paired snapshot's holds of a
+// caller that also reads its table (warmSet).
+func (e *Edge) certNamesOf(holds *holdSnapshot) []string {
+ out := holds.names()
hm := e.homes.Load()
for _, n := range hm.HoldingHosts(hm.clock()) {
if !slices.Contains(out, n) {