amadan
Merge pull request #298 from carlosframework/snapshot-fingerprint-churn
registry: SnapshotFingerprint ignores the box's own scheduling stamps (#207)
2 files changed,
+80
−1
internal/registry/registry.go+26 −1internal/registry/registry_test.go+54 −0
diff --git a/internal/registry/registry.go b/internal/registry/registry.go| index 0b79bd5b5..0c9a5fafa 100644 |
| --- a/internal/registry/registry.go |
| +++ b/internal/registry/registry.go |
| @@ -1794,6 +1794,20 @@ FROM fleet_boxes ORDER BY fleet, label`) |
| // values are scanned generically and written in column order, which is why |
| // this needs no per-column typing the way Fingerprint does. |
| // |
| +// "Every column" has exactly two exceptions, snapshotChurnCols: the |
| +// scheduling stamps the box rewrites on its own clock with no operator |
| +// change behind them. Hashing those made the gate never hold (#207 — both |
| +// production boxes wrote a generation on essentially every 5-minute tick, |
| +// so the 8-generation history was ~40 minutes deep, not 8 changes deep). |
| +// They are still IN the snapshot; only the decision to take one ignores |
| +// them. A restore therefore loses up to one tick of those stamps, which the |
| +// activator and DNS sweeps re-derive on their first pass anyway. |
| +// |
| +// route_aliases and wildcard_domains are not hashed either: both are |
| +// pass-owned materializations of domain claims (domains.Reconcile rebuilds |
| +// every row from the claim records), so a restore recovers them by |
| +// running. |
| +// |
| // Hashing the VACUUM INTO output instead was rejected: SQLite's page image |
| // is not byte-deterministic across runs of identical content (freelist and |
| // page-ordering effects), so it would report a change on every pass and |
| @@ -1802,6 +1816,14 @@ FROM fleet_boxes ORDER BY fleet, label`) |
| // Fresh connection, same reason as Fingerprint and List: a long-lived |
| // modernc connection can serve a stale WAL snapshot of another process's |
| // commits. |
| +// snapshotChurnCols are the routes columns SnapshotFingerprint skips: stamps |
| +// the box's own sweeps rewrite with nothing behind them an operator did. |
| +// Keep this to columns that are re-derived within one sweep of a restore. |
| +var snapshotChurnCols = map[string]bool{ |
| + "next_due": true, // activator scheduling (SetNextDue, every pass) |
| + "dns_checked_at": true, // DNS sweep stamp (SetDNSStatus, every sweep) |
| +} |
| + |
| func (r *Registry) SnapshotFingerprint() (string, error) { |
| db, err := sql.Open("sqlite", r.path) |
| if err != nil { |
| @@ -1832,7 +1854,10 @@ func (r *Registry) SnapshotFingerprint() (string, error) { |
| if err := rows.Scan(dest...); err != nil { |
| return "", err |
| } |
| - for _, v := range vals { |
| + for i, v := range vals { |
| + if snapshotChurnCols[cols[i]] { |
| + continue |
| + } |
| // NULL and "" must not hash alike: a column that has never been |
| // written is a different restore than one explicitly cleared. |
| if v.Valid { |
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go| index 8a48955e3..196a1ad45 100644 |
| --- a/internal/registry/registry_test.go |
| +++ b/internal/registry/registry_test.go |
| @@ -2470,3 +2470,57 @@ func TestWildcardDomains(t *testing.T) { |
| t.Fatalf("after delete: %+v", got) |
| } |
| } |
| + |
| +// #207: the snapshot gate must hold across the stamps the box rewrites on |
| +// its own clock — otherwise every 5-minute tick is a "change" and the |
| +// 8-generation history is 40 minutes deep instead of 8 changes deep. A |
| +// real DNS status flip, by contrast, still moves it. |
| +func TestSnapshotFingerprintIgnoresSchedulingChurn(t *testing.T) { |
| + reg := openTestRegistry(t) |
| + if err := reg.Upsert(Route{ |
| + Host: "app.example.com", Kind: "instance", App: "app", |
| + Addr: "127.0.0.1:8091", Status: "ready", |
| + }); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.SetDNSStatus("app.example.com", "ok", 1000); err != nil { |
| + t.Fatal(err) |
| + } |
| + before, err := reg.SnapshotFingerprint() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.SetNextDue("app.example.com", 4242); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := reg.SetDNSStatus("app.example.com", "ok", 2000); err != nil { |
| + t.Fatal(err) |
| + } |
| + after, err := reg.SnapshotFingerprint() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if after != before { |
| + t.Fatalf("SnapshotFingerprint moved on next_due/dns_checked_at churn (%s -> %s) — the snapshot pass would write every tick", before, after) |
| + } |
| + if err := reg.SetDNSStatus("app.example.com", "mismatch", 3000); err != nil { |
| + t.Fatal(err) |
| + } |
| + flipped, err := reg.SnapshotFingerprint() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if flipped == before { |
| + t.Fatal("a dns_status change is invisible to SnapshotFingerprint") |
| + } |
| +} |
| + |
| +// The skip list is by column NAME, so a churn column must actually exist in |
| +// routeCols — a rename would silently stop excluding it. |
| +func TestSnapshotChurnColsExist(t *testing.T) { |
| + for col := range snapshotChurnCols { |
| + if !strings.Contains(routeCols, col) { |
| + t.Errorf("snapshotChurnCols names %q, which is not in routeCols", col) |
| + } |
| + } |
| +} |