amadan

carlos / platform Public

geoip: verdict knows unknown/off/live pointer states; off refuses during a publish; bounded pointer reads; spec rev 10 (Astra round 9)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude f83276b72d1d8a76342513881f64e8b3c5bd3afd parent 242de22
7 files changed, +234 −57
  • docs/superpowers/specs/2026-10-06-platform-geoip-design.md +44 −10
  • docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R9.md +34 −0
  • internal/console/apisystemgeoip.go +60 −24
  • internal/console/apisystemgeoip_test.go +67 −8
  • internal/console/geoippublish.go +6 −1
  • internal/console/geoippublish_test.go +6 −8
  • internal/edge/geoipmirror.go +17 −6

CI cancelled — superseded by a newer push — run details

diff --git a/docs/superpowers/specs/2026-10-06-platform-geoip-design.md b/docs/superpowers/specs/2026-10-06-platform-geoip-design.md
index eb757b276..8a24c5d3e 100644
--- a/docs/superpowers/specs/2026-10-06-platform-geoip-design.md
+++ b/docs/superpowers/specs/2026-10-06-platform-geoip-design.md
@@ -1,7 +1,7 @@
# Platform GeoIP for apps: headers, a mirrored database, a lookup endpoint
**Date:** 2026-10-06
-**Revision:** 9
+**Revision:** 10
**Discussion:** amadan carlos/platform #241 (quoted in Appendix A)
**Branch:** `geoip-for-apps`
@@ -67,6 +67,13 @@
clock runs from the start of each pass;
- §3 says digest, not size;
- only expected boxes can block the verdict.
+- Rev 10 answers Astra's round 9 (`CODEX-FINDINGS-GEOIP-R9.md`), marked
+ "(codex R9-N)":
+ - the verdict knows four pointer states;
+ - `off` answers 409 during a publish instead of waiting;
+ - pointer reads are bounded at 30 seconds, and the off bound includes
+ them;
+ - an unreadable heartbeat blocks only through the expected box it hides.
## 0. Rulings and decisions
@@ -400,7 +407,9 @@ Under `carlos system`, through the console API:
- the feed's configuration and its last result;
- the per-box report from §5.4.
- `carlos system geoip off` deletes the pointer, then sweeps the blobs
- (§3). Every edge drops its reader within one poll (at most 11 minutes,
+ (§3). While a publish holds the slot (an upload streaming in, or its
+ job), `off` answers **409 straight away** rather than waiting behind it
+ (codex R9-2). The operator runs it again once the publish finishes. Every edge drops its reader within one poll (at most 11 minutes,
with jitter). The console follows its own box's edge within a further
minute, so it takes at most 12 minutes (§11.3, codex R2-9). If
`CONSOLE_GEOIP_FEED` is set, `off` warns that the feed will publish
@@ -473,9 +482,14 @@ old generation, or the new `current` with the new one.
publish that landed mid-download takes effect at the end of that pass.
- If the re-read fails, nothing is installed until the next pass.
- Each pass is due 10 minutes plus jitter after the previous one
- **started**, and a pass is bounded at 10 minutes. A change to the
- pointer is therefore acted on within 11 minutes even with a long
- download in flight. That is the bound §4.2 quotes.
+ **started**, and a pass is bounded at 10 minutes.
+ - Every pointer read has its own 30-second timeout (codex R9 on R8-2).
+ A change to the pointer is therefore acted on within 11 minutes plus
+ one pointer read (at most 11.5 minutes), even with a long download in
+ flight. That is the bound §4.2 quotes.
+ - While pointer reads keep failing, the edge keeps what it has loaded,
+ and its heartbeat reports the error, so the verdict shows the box as
+ not caught up.
- **Every 10 minutes,** with up to 1 minute of jitter, read the pointer
(bounded read, at most 64 KiB). Each pass has a 10-minute deadline,
covering the pointer read and any blob download. A pass that misses it
@@ -594,6 +608,14 @@ blocking line uses the pair. Lines name a fleet box as `<fleet>/<label>`.
`systemBoxes`'s strict mode is used, because both skip unreadable
records silently.
+**What the pointer asks of every mirror** (codex R9-1):
+
+| Pointer | Each expected mirroring box must report |
+|---|---|
+| live | the pointer's sha |
+| absent (off), or expired | no generation loaded |
+| unreadable | nothing satisfies this: the verdict is never ready while the pointer can't be read |
+
**Which boxes are listed, and which can block:**
- Every expected box is listed whatever its age, and **only expected
boxes can block** the verdict (codex R8 on R1-16).
@@ -607,8 +629,10 @@ It reports:
- **stale,** by the shared rule `status.BoxStale` (3 heartbeat periods,
15 minutes), not a threshold of its own (codex R3-4). The verdict can't
vouch for a box it can't currently see.
-- **record unreadable**, or an incomplete read of any prefix:
- "unreadable". An incomplete read is never "ready".
+- **record unreadable:** the box is missing from what was read, so if it
+ is expected it blocks as "no heartbeat". An unreadable record for a box
+ nobody expects blocks nothing (codex R9 on R1-16). If the list of
+ expected fleet boxes itself can't be read, that always blocks.
- **expected box with no heartbeat at all:** "no heartbeat".
- **no block:** "old binary". The headers can't be trusted while any such
box is serving.
@@ -630,9 +654,10 @@ retirement.
GeoIP. Plan 1 files it as a discussion rather than working around its
absence.
-It ends with a one-line verdict. **"Ready"** means every listed box is
-fresh, readable and GeoIP-aware, and every mirroring box has loaded the
-current sha. Anything else names the boxes in the way (codex R2 on R1-16).
+It ends with a one-line verdict. **"Ready"** means the pointer was read,
+the expected-box list was complete, and every **expected** box is fresh,
+readable and GeoIP-aware, with every expected mirroring box holding what
+the pointer asks for (table above). Anything else names the boxes in the way (codex R2 on R1-16).
A test sets a box's heartbeat just inside the next jittered tick and
checks it still reads as fresh.
@@ -1340,6 +1365,15 @@ Round 8 (`CODEX-FINDINGS-GEOIP-R8.md`): every finding accepted.
| 2 | P2 | Accepted. A re-read of the pointer before install, one immediate re-run, and pass-start timing, so the 11-minute bound holds (§5.3) |
| 3 | P3 | Accepted. §3 step 2 now requires digest and size |
+Round 9 (`CODEX-FINDINGS-GEOIP-R9.md`): every finding accepted.
+
+| # | Sev | Disposition |
+|---|---|---|
+| R1-16 (partly) | | Unreadable records block only through expected identities; the final requirement says "every expected box" (§5.5) |
+| R8-2 (partly) | | 30-second bound on each pointer read; the off bound states it; read failures keep the loaded database and surface in the heartbeat (§5.3) |
+| 1 | P2 | Accepted. Four pointer states: unknown never ready, off/expired requires empty mirrors, live requires the sha (§5.5) |
+| 2 | P2 | Accepted. `off` answers 409 while a publish holds the slot (§4.2) |
+
## Appendix A. The request (#241, verbatim)
> **Expose the platform's GeoIP lookup to apps (headers and/or lookup endpoint)**
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R9.md b/docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R9.md
new file mode 100644
index 000000000..6395e0774
--- /dev/null
+++ b/docs/superpowers/specs/CODEX-FINDINGS-GEOIP-R9.md
@@ -0,0 +1,34 @@
+| Previous finding | Disposition | Evidence from revision 9 at `ca3f3536` |
+|---|---|---|
+| R3-1. Expiry/install races | Resolved | §5.3 retains expiry checks before download and installation, with `genMu` serializing selection, expiry and cleanup. |
+| R3-2. Unprotected console `Locate` | Resolved | §5.4 retains locked `Swappable.Locate` and assigns the wrapper to `Config.GeoIP`. |
+| R3-3. Coordinate-only results | Resolved | §8.1a accepts coordinate-only hits; §9 counts their country as `_unknown`. |
+| R3-4. Heartbeat freshness | Resolved | §5.5 retains `status.BoxStale` and the jittered-heartbeat test. |
+| R1-4, carried forward: orphan destruction | Resolved | §3 retains startup and daily orphan sweeps, followed by §15’s noncurrent-version expiration. |
+| R1-16, carried forward: rollout verification | Partly resolved | §5.5 now excludes retired rows from blocking, but its blanket incomplete-read rule still lets an unreadable retired heartbeat block readiness. Scope record-level blockers to expected identities; also change the final “every listed box” requirement to “every expected box.” |
+| R4-1. Duplicate tunnel analytics | Resolved | §§8.4 and 9 retain zero remote observations and exactly one at the hub. |
+| R4-2. Publication deadline across backhaul | Resolved | §4.2 retains staging followed by a 202 response and a polled publication job. |
+| R5-1. Concurrent upload staging | Resolved | §4.2 retains mutex reservation before reading the body and ownership through job cleanup. |
+| R6-1. Plaintext tunnel cache mismatch | Resolved | D2 and §§8–9, 16 consistently require the transport-independent tunnel mark. |
+| R6-2. Abandoned upload staging | Resolved | §4.2 retains the dedicated staging directory, startup cleanup and restart tests. |
+| R7-1. Publication of expired candidates | Resolved | §4.1 rejects expired candidates, rechecks before pointer replacement and forbids a force override. |
+| R7-2. Empty sanitized source | Resolved | §6 retains the valid fallback identifier `mmdb`. |
+| R7-3. Attribution lost through overflow | Resolved | §9 exempts attributed sources from folding into `_other`. |
+| R7-4. Interface discrepancies | Resolved | §§4.1 and 6 retain the corrected signatures and Unix-seconds conversion. |
+| R8-1. Box identity namespaces | Resolved | §5.5 requires `(namespace, label)` throughout inventory, heartbeat matching and blockers. |
+| R8-2. Shutdown propagation bound | Partly resolved | §5.3 adds pointer revalidation and pass-start scheduling, but its eleven-minute guarantee still excludes pointer-read time. An eleven-minute polling wait followed by a successful nine-minute read takes twenty minutes. Include read duration and qualify behavior during read failures. |
+| R8-3. Existing-blob verification | Resolved | §3 now requires both SHA-256 and size to match before replacing the pointer. |
+
+1. **P2 — §§4.2, 5.5: status can report `ready` without establishing database convergence.** The readiness contract does not distinguish an unreadable pointer from confirmed absence. The implementation records `pointer_error` but passes a nil pointer to the verdict, disabling the hash comparison. Healthy heartbeats reporting different generations can therefore produce `ready: true`. Confirmed absence or expiry also disables that comparison, allowing readiness while mirrors still report a loaded database.
+
+ **Evidence:** [apisystemgeoip.go:137](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/apisystemgeoip.go:137) leaves `ptr` nil after a read failure; [apisystemgeoip.go:157](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/apisystemgeoip.go:157) still computes readiness. At [apisystemgeoip.go:299](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/apisystemgeoip.go:299), mismatches block only when the desired SHA is nonempty.
+
+ **Fix:** specify distinct unknown, off, live and expired pointer states. Unknown must block readiness. Off and expired must require expected mirrors to report no loaded generation. Test pointer-read failure with otherwise healthy heartbeats, and deletion while a mirror still reports the previous SHA.
+
+2. **P2 — §§3, 4.2: `off` can time out waiting for the publication mutex.** An upload reserves the mutex before reading its body and retains it through the publication job. A concurrent synchronous `off` therefore waits behind an operation allowed to take minutes. The caller or backhaul times out before the pointer is deleted, leaving the operator without a completed shutdown. The spec defines serialization but omits admission and response behavior for this case.
+
+ **Evidence:** [geoippublish.go:290](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/geoippublish.go:290) uses blocking `Lock`; [apisystemgeoip.go:106](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/console/apisystemgeoip.go:106) waits synchronously for `Off`. The ordinary CLI client has a thirty-second timeout at [auth.go:404](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/auth.go:404), and backhaul has the same response-header limit at [edge.go:2710](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/geoip-for-apps/internal/edge/edge.go:2710).
+
+ **Fix:** require immediate 409 when the publication slot is occupied, or make `off` a polled job with an independent lifetime. Preserve serialization through cleanup. Test `off` during both a stalled upload and an active publication job, including through backhaul.
+
+Verdict: revise.
\ No newline at end of file
diff --git a/internal/console/apisystemgeoip.go b/internal/console/apisystemgeoip.go
index 22dc83d31..7c5598546 100644
--- a/internal/console/apisystemgeoip.go
+++ b/internal/console/apisystemgeoip.go
@@ -103,7 +103,10 @@ func (s *Server) handleAPISystemGeoipDelete(w http.ResponseWriter, r *http.Reque
if !s.systemBucket(w) {
return
}
- if err := s.geoip.Off(r.Context()); err != nil {
+ if err := s.geoip.Off(r.Context()); errors.Is(err, errGeoipBusy) {
+ jsonError(w, http.StatusConflict, "a geoip publish is in progress. run off again when it finishes")
+ return
+ } else if err != nil {
log.Printf("console: geoip off: %v", err)
jsonError(w, http.StatusBadGateway, "could not turn geoip off")
return
@@ -133,11 +136,17 @@ func (s *Server) handleAPISystemGeoipGet(w http.ResponseWriter, r *http.Request)
ctx := r.Context()
cur, _, present, perr := s.geoip.Current(ctx)
out := map[string]any{"pointer": nil}
- var ptr *geoip.Pointer
- if perr != nil {
+ want := geoipWant{state: geoipOff}
+ switch {
+ case perr != nil:
out["pointer_error"] = "the published pointer is unreadable"
- } else if present {
- ptr = &cur
+ want.state = geoipUnknown
+ case present && cur.Expired:
+ want.state = geoipOff // expired: every edge must have dropped it
+ case present:
+ want = geoipWant{state: geoipLive, sha: cur.SHA256}
+ }
+ if perr == nil && present {
p := map[string]any{"sha256": cur.SHA256, "database_type": cur.DatabaseType, "build": cur.Build(),
"published": cur.Published, "origin": cur.Origin, "edition": cur.Edition, "size": cur.Size, "expired": cur.Expired}
if at, ok := geoip.ExpiresAt(cur.DatabaseType, cur.Build()); ok {
@@ -154,7 +163,7 @@ func (s *Server) handleAPISystemGeoipGet(w http.ResponseWriter, r *http.Request)
jsonError(w, http.StatusBadGateway, "box heartbeats unreadable")
return
}
- ready, rows, blocking := geoipVerdict(ptr, inv, time.Now())
+ ready, rows, blocking := geoipVerdict(want, inv, time.Now())
out["boxes"], out["ready"], out["blocking"] = rows, ready, blocking
out["feed_set"] = os.Getenv("CONSOLE_GEOIP_FEED") != ""
writeJSON(w, http.StatusOK, out)
@@ -173,17 +182,38 @@ func (k geoipBoxKey) String() string {
}
// geoipInventoryRead is everything the verdict reconciles (spec §5.5).
+// expectedComplete is false when the list of expected boxes itself could
+// not be read (a fleet registry or fleet listing failed): that always
+// blocks. heartbeatsComplete is false when some heartbeat record could not
+// be read: informational only, because an expected box whose record was
+// unreadable is missing from boxes and blocks as "no heartbeat" on its own
+// (codex R9 on R1-16).
type geoipInventoryRead struct {
- expected map[geoipBoxKey]bool
- boxes map[geoipBoxKey]status.Box
- complete bool
+ expected map[geoipBoxKey]bool
+ boxes map[geoipBoxKey]status.Box
+ expectedComplete bool
+ heartbeatsComplete bool
+}
+
+// geoipWantState is what the published pointer asks of every mirror.
+type geoipWantState int
+
+const (
+ geoipUnknown geoipWantState = iota // the pointer could not be read: never ready
+ geoipOff // no pointer, or an expired one: mirrors must hold nothing
+ geoipLive // mirrors must hold sha
+)
+
+type geoipWant struct {
+ state geoipWantState
+ sha string
}
// geoipInventory reads the expected boxes (platform edges records, active
// fleet registrations) and every heartbeat prefix with LoadBoxesComplete,
// never LoadBoxes, which skips unreadable records silently.
func (s *Server) geoipInventory(ctx context.Context) (geoipInventoryRead, error) {
- inv := geoipInventoryRead{expected: map[geoipBoxKey]bool{}, boxes: map[geoipBoxKey]status.Box{}, complete: true}
+ inv := geoipInventoryRead{expected: map[geoipBoxKey]bool{}, boxes: map[geoipBoxKey]status.Box{}, expectedComplete: true, heartbeatsComplete: true}
keys, err := s.cfg.Releases.List(ctx, "control/edges/")
if err != nil {
return inv, fmt.Errorf("edges records: %w", err)
@@ -200,7 +230,7 @@ func (s *Server) geoipInventory(ctx context.Context) (geoipInventoryRead, error)
return err
}
if !complete {
- inv.complete = false
+ inv.heartbeatsComplete = false
}
for _, b := range boxes {
inv.boxes[geoipBoxKey{fleet, b.Label}] = b
@@ -218,18 +248,18 @@ func (s *Server) geoipInventory(ctx context.Context) (geoipInventoryRead, error)
case errors.Is(err, os.ErrNotExist):
return inv, nil
case err != nil:
- inv.complete = false
+ inv.expectedComplete = false
return inv, nil
}
fleets, err := reg.ListFleets()
if err != nil {
- inv.complete = false
+ inv.expectedComplete = false
return inv, nil
}
for _, f := range fleets {
fbs, err := reg.ListFleetBoxes(f.Name)
if err != nil {
- inv.complete = false
+ inv.expectedComplete = false
continue
}
for _, fb := range fbs {
@@ -238,7 +268,9 @@ func (s *Server) geoipInventory(ctx context.Context) (geoipInventoryRead, error)
}
}
if err := add(release.NewPrefixStore(s.cfg.Releases, "fleets/"+f.Name+"/"), f.Name); err != nil {
- inv.complete = false
+ // That fleet's heartbeats are unknown: its expected boxes are
+ // missing from boxes and block as "no heartbeat".
+ inv.heartbeatsComplete = false
}
}
return inv, nil
@@ -247,7 +279,7 @@ func (s *Server) geoipInventory(ctx context.Context) (geoipInventoryRead, error)
// geoipVerdict is spec §5.5's reconciliation. ready only when every listed
// box is fresh, readable and GeoIP-aware, every mirroring box holds the
// live pointer's sha, and every read was complete.
-func geoipVerdict(ptr *geoip.Pointer, inv geoipInventoryRead, now time.Time) (bool, []geoipBoxRow, []string) {
+func geoipVerdict(want geoipWant, inv geoipInventoryRead, now time.Time) (bool, []geoipBoxRow, []string) {
var rows []geoipBoxRow
var blocking []string
keys := map[geoipBoxKey]bool{}
@@ -264,10 +296,6 @@ func geoipVerdict(ptr *geoip.Pointer, inv geoipInventoryRead, now time.Time) (bo
sorted = append(sorted, k)
}
sort.Slice(sorted, func(i, j int) bool { return sorted[i].String() < sorted[j].String() })
- liveSHA := ""
- if ptr != nil && !ptr.Expired {
- liveSHA = ptr.SHA256
- }
for _, k := range sorted {
name, expected := k.String(), inv.expected[k]
row := geoipBoxRow{Label: k.Label, Fleet: k.Fleet, Expected: expected}
@@ -296,9 +324,14 @@ func geoipVerdict(ptr *geoip.Pointer, inv geoipInventoryRead, now time.Time) (bo
switch g := b.Metrics.GeoIP; {
case g.Mode == "tunnel", g.Mode == "none":
row.State = g.Mode
- case liveSHA != "" && g.SHA256 != liveSHA:
+ case want.state == geoipLive && g.SHA256 != want.sha:
row.State = "other sha"
- row.Blocking = name + ": has " + short8(g.SHA256) + ", not the published " + short8(liveSHA)
+ row.Blocking = name + ": has " + short8(g.SHA256) + ", not the published " + short8(want.sha)
+ case want.state == geoipOff && g.SHA256 != "":
+ // Off or expired: a mirror still serving a generation has
+ // not caught up (codex R9-1).
+ row.State = "still loaded"
+ row.Blocking = name + ": still has " + short8(g.SHA256) + " loaded"
default:
row.State = "ok"
}
@@ -315,8 +348,11 @@ func geoipVerdict(ptr *geoip.Pointer, inv geoipInventoryRead, now time.Time) (bo
}
rows = append(rows, row)
}
- if !inv.complete {
- blocking = append(blocking, "heartbeats: some records could not be read")
+ if want.state == geoipUnknown {
+ blocking = append(blocking, "database: the published pointer could not be read")
+ }
+ if !inv.expectedComplete {
+ blocking = append(blocking, "boxes: the list of fleet boxes could not be read")
}
if rows == nil {
rows = []geoipBoxRow{}
diff --git a/internal/console/apisystemgeoip_test.go b/internal/console/apisystemgeoip_test.go
index 73b8ea0a0..62a9789b3 100644
--- a/internal/console/apisystemgeoip_test.go
+++ b/internal/console/apisystemgeoip_test.go
@@ -227,7 +227,7 @@ func TestGeoipStatusVerdict(t *testing.T) {
}
func TestGeoipVerdictIncompleteReadIsNeverReady(t *testing.T) {
- ready, _, blocking := geoipVerdict(nil, geoipInventoryRead{expected: map[geoipBoxKey]bool{}, boxes: map[geoipBoxKey]status.Box{}, complete: false}, time.Now())
+ ready, _, blocking := geoipVerdict(geoipWant{state: geoipOff}, geoipInventoryRead{expected: map[geoipBoxKey]bool{}, boxes: map[geoipBoxKey]status.Box{}, expectedComplete: false, heartbeatsComplete: true}, time.Now())
if ready || len(blocking) == 0 {
t.Fatalf("incomplete read = ready %v, blocking %v", ready, blocking)
}
@@ -267,16 +267,16 @@ func TestGeoipVerdictKeysBoxesByNamespace(t *testing.T) {
old := status.CollectBox(nil, status.Probes{Release: "v0"}, now)
old.Label = "node-1"
inv := geoipInventoryRead{
- expected: map[geoipBoxKey]bool{{"", "node-1"}: true, {"tito", "node-1"}: true, {"bae", "node-1"}: true},
- boxes: map[geoipBoxKey]status.Box{{"", "node-1"}: fresh("node-1"), {"tito", "node-1"}: fresh("node-1"), {"bae", "node-1"}: old},
- complete: true,
+ expected: map[geoipBoxKey]bool{{"", "node-1"}: true, {"tito", "node-1"}: true, {"bae", "node-1"}: true},
+ boxes: map[geoipBoxKey]status.Box{{"", "node-1"}: fresh("node-1"), {"tito", "node-1"}: fresh("node-1"), {"bae", "node-1"}: old},
+ expectedComplete: true, heartbeatsComplete: true,
}
- ready, _, blocking := geoipVerdict(&geoip.Pointer{SHA256: "s"}, inv, now)
+ ready, _, blocking := geoipVerdict(geoipWant{state: geoipLive, sha: "s"}, inv, now)
if ready || len(blocking) != 1 || !strings.HasPrefix(blocking[0], "bae/node-1: runs a binary") {
t.Fatalf("ready=%v blocking=%v, want only bae/node-1 blocking", ready, blocking)
}
delete(inv.boxes, geoipBoxKey{"tito", "node-1"})
- _, _, blocking = geoipVerdict(&geoip.Pointer{SHA256: "s"}, inv, now)
+ _, _, blocking = geoipVerdict(geoipWant{state: geoipLive, sha: "s"}, inv, now)
found := false
for _, b := range blocking {
found = found || strings.HasPrefix(b, "tito/node-1: no heartbeat")
@@ -292,9 +292,68 @@ func TestGeoipVerdictRetiredBoxDoesNotBlock(t *testing.T) {
now := time.Now()
b := status.CollectBox(nil, status.Probes{Release: "v0"}, now.Add(-time.Hour))
b.Label = "gone-1"
- inv := geoipInventoryRead{expected: map[geoipBoxKey]bool{}, boxes: map[geoipBoxKey]status.Box{{"", "gone-1"}: b}, complete: true}
- ready, rows, _ := geoipVerdict(nil, inv, now)
+ inv := geoipInventoryRead{expected: map[geoipBoxKey]bool{}, boxes: map[geoipBoxKey]status.Box{{"", "gone-1"}: b}, expectedComplete: true, heartbeatsComplete: false}
+ ready, rows, _ := geoipVerdict(geoipWant{state: geoipOff}, inv, now)
if !ready || len(rows) != 1 {
t.Fatalf("ready=%v rows=%v, want ready with the retired box listed", ready, rows)
}
}
+
+// Codex R9-1: an unreadable pointer is never ready, and off requires every
+// mirror to have dropped its generation.
+func TestGeoipVerdictPointerStates(t *testing.T) {
+ now := time.Now()
+ mirror := func(sha string) status.Box {
+ b := status.CollectBox(nil, status.Probes{Release: "v1", GeoIP: &status.GeoIPState{Mode: "mirror", SHA256: sha}}, now)
+ b.Label = "ie-1"
+ return b
+ }
+ inv := func(sha string) geoipInventoryRead {
+ return geoipInventoryRead{expected: map[geoipBoxKey]bool{{"", "ie-1"}: true},
+ boxes: map[geoipBoxKey]status.Box{{"", "ie-1"}: mirror(sha)}, expectedComplete: true, heartbeatsComplete: true}
+ }
+ if ready, _, _ := geoipVerdict(geoipWant{state: geoipUnknown}, inv("a"), now); ready {
+ t.Fatal("an unreadable pointer reported ready")
+ }
+ if ready, _, b := geoipVerdict(geoipWant{state: geoipOff}, inv("a"), now); ready || len(b) != 1 || !strings.Contains(b[0], "still has") {
+ t.Fatalf("off with a mirror still loaded = %v %v", ready, b)
+ }
+ if ready, _, _ := geoipVerdict(geoipWant{state: geoipOff}, inv(""), now); !ready {
+ t.Fatal("off with every mirror empty must be ready")
+ }
+ if ready, _, _ := geoipVerdict(geoipWant{state: geoipLive, sha: "b"}, inv("a"), now); ready {
+ t.Fatal("a mirror on another sha reported ready")
+ }
+}
+
+// Codex R9-2: off never waits behind a publish.
+func TestGeoipOffDuringAPublishIs409(t *testing.T) {
+ withFixtureProbes(t)
+ fx := newConsoleFixture(t)
+ owner := cliToken(t, fx, "paul@keymail.dev", "code-geoip-offbusy")
+ st, err := fx.srv.geoip.Stage(context.Background(), bytes.NewReader(fixture(t, "GeoIP2-City-Test.mmdb")))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer fx.srv.geoip.Abandon(st)
+ req, _ := http.NewRequest(http.MethodDelete, fx.ts.URL+"/api/cli/system/geoip", nil)
+ req.Header.Set("Authorization", "Bearer "+owner)
+ done := make(chan int, 1)
+ go func() {
+ res, err := http.DefaultClient.Do(req)
+ if err != nil {
+ done <- 0
+ return
+ }
+ res.Body.Close()
+ done <- res.StatusCode
+ }()
+ select {
+ case code := <-done:
+ if code != http.StatusConflict {
+ t.Fatalf("off during a publish = %d, want 409", code)
+ }
+ case <-time.After(5 * time.Second):
+ t.Fatal("off waited behind the publish")
+ }
+}
diff --git a/internal/console/geoippublish.go b/internal/console/geoippublish.go
index ab564df6e..e56c83e5c 100644
--- a/internal/console/geoippublish.go
+++ b/internal/console/geoippublish.go
@@ -287,7 +287,12 @@ func hasCountryData(db *geo.MMDB) bool {
// Off deletes the pointer, then every blob (spec §3). Unconditional delete:
// safe under the single-writer mutex (codex R2-6).
func (p *geoipPublisher) Off(ctx context.Context) error {
- p.mu.Lock()
+ // Never waits behind a publish (codex R9-2): a slow upload or job can
+ // hold the slot for minutes, longer than any caller's timeout. The
+ // operator gets a 409 and runs off again when the publish finishes.
+ if !p.mu.TryLock() {
+ return errGeoipBusy
+ }
defer p.mu.Unlock()
if err := p.store.Delete(ctx, geoip.PointerKey); err != nil && !errors.Is(err, os.ErrNotExist) {
return err
diff --git a/internal/console/geoippublish_test.go b/internal/console/geoippublish_test.go
index 428f84bbc..6d93cf327 100644
--- a/internal/console/geoippublish_test.go
+++ b/internal/console/geoippublish_test.go
@@ -324,7 +324,7 @@ func TestCommitRefusesAForeignPointerWrite(t *testing.T) {
p.mu.Unlock()
}
-func TestOffWaitsForAnInFlightPublish(t *testing.T) {
+func TestOffRefusesDuringAPublishThenSucceeds(t *testing.T) {
withFixtureProbes(t)
ctx := context.Background()
p, s := newTestPublisher(t)
@@ -332,17 +332,15 @@ func TestOffWaitsForAnInFlightPublish(t *testing.T) {
if err != nil {
t.Fatal(err)
}
- offDone := make(chan error, 1)
- go func() { offDone <- p.Off(ctx) }()
- select {
- case <-offDone:
- t.Fatal("off ran while a publish held the slot")
- case <-time.After(50 * time.Millisecond):
+ // Off refuses rather than waits while the slot is held (codex R9-2)...
+ if err := p.Off(ctx); !errors.Is(err, errGeoipBusy) {
+ t.Fatalf("off during a publish = %v, want errGeoipBusy", err)
}
if _, err := p.Commit(ctx, st, geoipPublishOpts{Origin: "manual"}); err != nil {
t.Fatal(err)
}
- if err := <-offDone; err != nil {
+ // ...and succeeds once the publish has finished.
+ if err := p.Off(ctx); err != nil {
t.Fatal(err)
}
if _, _, err := s.Get(ctx, geoip.PointerKey); !errors.Is(err, os.ErrNotExist) {
diff --git a/internal/edge/geoipmirror.go b/internal/edge/geoipmirror.go
index 1735af904..419645b3e 100644
--- a/internal/edge/geoipmirror.go
+++ b/internal/edge/geoipmirror.go
@@ -27,10 +27,14 @@ import (
// expires or deletes a generation (codex R3-1); network I/O stays outside it.
var (
- geoMirrorInterval = 10 * time.Minute
- geoMirrorJitter = time.Minute
- geoMirrorDeadline = 10 * time.Minute
- geoExpiryInterval = time.Minute
+ // geoPointerReadTimeout bounds every pointer read on its own, so a
+ // slow store cannot stretch the off bound past interval+jitter+this
+ // (codex R9 on R8-2).
+ geoPointerReadTimeout = 30 * time.Second
+ geoMirrorInterval = 10 * time.Minute
+ geoMirrorJitter = time.Minute
+ geoMirrorDeadline = 10 * time.Minute
+ geoExpiryInterval = time.Minute
)
type geoCurrent struct {
@@ -220,7 +224,7 @@ func (m *geoMirror) poll(ctx context.Context, recheck bool) error {
if err := os.MkdirAll(m.dir, 0o755); err != nil {
return m.fail("local directory unusable", err)
}
- raw, err := readCapped(ctx, m.store, geoip.PointerKey, geoip.MaxPointerBytes)
+ raw, err := m.readPointer(ctx)
var p geoip.Pointer
if err == nil {
p, err = geoip.ParsePointer(raw)
@@ -304,7 +308,7 @@ var downloadHook func()
// failure answers false, which defers the install to the next pass rather
// than installing something nobody has confirmed is still wanted.
func (m *geoMirror) pointerStillNames(ctx context.Context, sha string) bool {
- raw, err := readCapped(ctx, m.store, geoip.PointerKey, geoip.MaxPointerBytes)
+ raw, err := m.readPointer(ctx)
if err != nil {
return false
}
@@ -434,6 +438,13 @@ func (m *geoMirror) failLocked(category string, err error) error {
return fmt.Errorf("%s: %w", category, err)
}
+// readPointer is one bounded pointer read.
+func (m *geoMirror) readPointer(ctx context.Context) ([]byte, error) {
+ ctx, cancel := context.WithTimeout(ctx, geoPointerReadTimeout)
+ defer cancel()
+ return readCapped(ctx, m.store, geoip.PointerKey, geoip.MaxPointerBytes)
+}
+
// readCapped reads key through OpenObject, refusing more than max bytes.
func readCapped(ctx context.Context, s release.Store, key string, max int) ([]byte, error) {
rc, _, err := release.OpenObject(ctx, s, key)