diff --git a/CHANGELOG.md b/CHANGELOG.md index ff77657..9218e0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ ## Changelog +### v0.193.1 — the refusal's size estimate is rendered in bytes, not `0.00 GiB` (2026-08-03, R-181 follow-on) + +**Found by the live proof run for v0.193.0, not by review.** The refusal message printed the estimate +fixed to two decimal GiB, so **every app under ~10 MB rendered as `estimated 0.00 GiB write`** — which +reads as *"no estimate was available"* and is the exact opposite of what happened. Observed live on +demo-hp at 08:59:46: opengist's real **178 KB** estimate printed as `0.00 GiB`. + +Shipped in the same session it was found because it is the same defect class the whole of R-181 is +about — a message that an operator cannot rely on is worse than no message. **The arithmetic is +unchanged and still in GiB** (the reserve's own unit, so the comparison against `FloorFreeGiB` reads +directly); only the rendering moved to `humanizeBytes`. `estimatedWriteGiB` → `estimatedWriteBytes`, +with the GiB conversion done once at the point of comparison. + +Verified live after redeploy: the same refusal now reads `estimated 178.0 KB write`. + ### v0.193.0 — the reserve guards the write that fills the disk, and its promise is true (2026-08-03, R-181) — MinAgent: none **The defect, found on live hardware and not by review.** v0.192.0's capture floor (B2) shipped as the diff --git a/REUSE.md b/REUSE.md index 6535380..b8d40ca 100644 --- a/REUSE.md +++ b/REUSE.md @@ -174,7 +174,7 @@ | `api.GracefulSelfRestart` | controller/internal/api/selfrestart.go | `(logger)` | Controller self-restart | Detached exit; bootstrap unit re-runs the image | | `Settings.AddPendingEvent/DrainPendingEvents` | controller/internal/settings/settings.go | offline event queue | Events while hub unreachable | — | | `Manager.SetUnitNotify` + `UnitSpace` (R-158/R-167, v0.191.0) | controller/internal/backup/recovery_unit.go | `(func(stack string, err error, *UnitSpace))` | THE per-app Tier-1 recovery-unit capture failure alert — fires PER APP from `captureAllRecoveryUnits`, loop continues | **OPERATOR-TIER** (`recovery_unit_capture_failed`, in the hub's `operatorOnlyEvents`). **NEVER route it to `backup_failed`** — that type is in `DefaultEnabledEvents` and carries Hungarian copy, so it emails the CUSTOMER about a failure they cannot act on (D-c; R-158's own proposal said `backup_failed` and D-c overrides it). `UnitSpace` is **nil when the target filesystem is unreadable** and renders as *"unavailable"*, never as zeros — "0 GB free" and "we could not look" are opposite diagnoses. No controller-side cooldown: the hub owns it | -| `Manager.admitApp` / `beginAdmissionRun` / `decideAdmission` / `estimatedWriteGiB` (R-181, v0.193.0) | controller/internal/backup/admission.go | `(stackName) bool` / `() func()` | **THE reserve gate. Call it before ANY per-app backup write** — one verdict per app per run, covering the DB dump, the volume dump and the unit capture (all three write under one per-app root) | **Decided LAZILY at the app's first write, never once at run start** — app A's dump can put app B under the reserve, so a run-start verdict reads a disk that no longer exists. **Never re-decided between an app's own legs**: that is exactly the split R-181 closed (bulk written, capture refused). **Reset per run** via the closer `beginAdmissionRun` returns. **Must sit ahead of `DumpAppVolumesSafe`**, which stops the stack as its first act — a refusal decided inside it has already bounced the app. Fires **exactly one** `unitNotify` per refused app per run. Nil admission set (periodic status refresh) → decides fresh, which is still once per app per sweep. Wiring pinned by an **AST walk** in `TestAdmission_IsWiredIntoEveryProductionWriteLeg`, not `strings.Contains` | +| `Manager.admitApp` / `beginAdmissionRun` / `decideAdmission` / `estimatedWriteBytes` (R-181, v0.193.0) | controller/internal/backup/admission.go | `(stackName) bool` / `() func()` | **THE reserve gate. Call it before ANY per-app backup write** — one verdict per app per run, covering the DB dump, the volume dump and the unit capture (all three write under one per-app root) | **Decided LAZILY at the app's first write, never once at run start** — app A's dump can put app B under the reserve, so a run-start verdict reads a disk that no longer exists. **Never re-decided between an app's own legs**: that is exactly the split R-181 closed (bulk written, capture refused). **Reset per run** via the closer `beginAdmissionRun` returns. **Must sit ahead of `DumpAppVolumesSafe`**, which stops the stack as its first act — a refusal decided inside it has already bounced the app. Fires **exactly one** `unitNotify` per refused app per run. Nil admission set (periodic status refresh) → decides fresh, which is still once per app per sweep. Wiring pinned by an **AST walk** in `TestAdmission_IsWiredIntoEveryProductionWriteLeg`, not `strings.Contains` | | `Manager.floorVerdict` + `FloorUsedPercent`/`FloorFreeGiB` / `ErrCaptureFloor` / `floorReason` (R-165 B2 v0.192.0, size term R-181 v0.193.0) | controller/internal/backup/recovery_unit.go | `(*UnitSpace, estGiB float64) (*UnitSpace, floorReason)` | The pure two-question predicate behind `admitApp`: is the filesystem already below the reserve (`floorHeadroom`), and would THIS app's write take it below (`floorSize`)? | **Headroom is about the FILESYSTEM, never a per-unit cap** — a size cap is R-163 rebuilt inside one volume; the size term bounds the *delta*, not the unit. **REFUSES, never deletes:** nothing here is generational (a unit is one fixed path per app, a DB dump one fixed name), so pruning could only destroy a DIFFERENT app's only local copy — **never repurpose `pruneStalePrimaryDirs`**, which removes ORPHANED dirs from an app that moved drives and has no notion of age. Two terms (97% / 1 GiB) in `fillwatch`'s shape, deliberately BEYOND its critical band (95% / 2 GiB) so the customer is always warned first — pinned by `TestFloorSitsBelowTheCriticalWarningBand`. **`estGiB == 0` degrades to headroom-only on purpose** — refusing an app with no history makes the FIRST backup the one that can never happen. A nil reading neither refuses nor warns (§8.4). Inject `unitSpaceFn` in tests rather than manufacturing occupancy on a real disk | | `fillwatch.Watcher` (`New`/`SetNotify`/`Check`) (R-167, v0.191.0) | controller/internal/fillwatch/fillwatch.go | `(statePath, logger, targetsFn, usageFn)` → `Check() error` | THE customer fill warning — warns BEFORE a filesystem fills, per FILESYSTEM (never per app: one full disk holding ten apps would fire ten times) | Emits the **pre-existing** `disk_warning`/`disk_critical` pair, which was allowlisted + copy'd + default-enabled with **no producer in any repo** until now — do NOT mint a new type beside it. **Two threshold terms, whichever trips first** (85% / 5 GiB; critical 95% / 2 GiB) because a percentage alone lies at both ends of this fleet's size range. **Edge-triggered on ESCALATION ONLY**, state persisted; de-escalation is silent and re-arms. Hysteresis dead zone between clear (75% / 7 GiB) and warn — pinned by `TestThresholdsKeepTheirHysteresisGap`. **A nil usage read is NEVER a warning** (§8.4). The hub has **no `customerMessages` entry** for either type on purpose — an entry would override the dynamic message and discard the drive label + free space | diff --git a/controller/internal/backup/admission.go b/controller/internal/backup/admission.go index 5a1a0ac..b81b2ad 100644 --- a/controller/internal/backup/admission.go +++ b/controller/internal/backup/admission.go @@ -65,7 +65,8 @@ type admissionVerdict struct { admitted bool reason floorReason usage *UnitSpace - estGiB float64 // the estimated write, 0 when the app has no previous dump on disk + estGiB float64 // the estimated write in GiB — the arithmetic unit, matching the reserve's terms + estBytes int64 // the same estimate in bytes — the RENDERING unit; see floorRefusal hasEst bool // whether an estimate was available at all (§8.2: distinct from "estimated 0") err error // the refusal, nil when admitted } @@ -136,17 +137,19 @@ func (m *Manager) admitApp(stackName string) bool { // decideAdmission applies the floor to a fresh reading plus this app's estimated write. func (m *Manager) decideAdmission(stackName string) admissionVerdict { - estGiB, hasEst := m.estimatedWriteGiB(stackName) + estBytes, hasEst := m.estimatedWriteBytes(stackName) + estGiB := float64(estBytes) / (1024 * 1024 * 1024) usage, reason := m.floorVerdict(m.readUnitSpace(stackName), estGiB) v := admissionVerdict{ admitted: reason == floorAdmit, reason: reason, usage: usage, estGiB: estGiB, + estBytes: estBytes, hasEst: hasEst, } if !v.admitted { - v.err = floorRefusal(reason, usage, estGiB, hasEst) + v.err = floorRefusal(reason, usage, estBytes, hasEst) } return v } @@ -154,14 +157,20 @@ func (m *Manager) decideAdmission(stackName string) admissionVerdict { // floorRefusal renders the refusal an operator reads. It names the reserve (not an I/O error — this // is a deliberate hold, not broken machinery), says WHICH term bound, and states plainly when the // decision was headroom-only because the app has no previous backup to estimate from (§8.2). -func floorRefusal(reason floorReason, usage *UnitSpace, estGiB float64, hasEst bool) error { +// +// THE ESTIMATE IS RENDERED IN BYTES-HUMANIZED, NOT GiB, and that is not cosmetic. Fixed to two +// decimal GiB, every app under ~10 MB prints `0.00 GiB` — which reads as "no estimate was available" +// and is the opposite of what happened. Observed on the live proof run: opengist's real 178 KB +// estimate rendered as `estimated 0.00 GiB write`. The arithmetic stays in GiB (the reserve's own +// unit); only the rendering changes. +func floorRefusal(reason floorReason, usage *UnitSpace, estBytes int64, hasEst bool) error { var b strings.Builder fmt.Fprintf(&b, "%%w (reserve: %.0f%%%% used or %.1f GiB free", FloorUsedPercent, FloorFreeGiB) switch { case reason == floorSize: - fmt.Fprintf(&b, "; this app's last backup was %.2f GiB and writing it again would cross the reserve", estGiB) + fmt.Fprintf(&b, "; this app's last backup was %s and writing it again would cross the reserve", humanizeBytes(estBytes)) case hasEst: - fmt.Fprintf(&b, "; the filesystem is already below it, before this app's estimated %.2f GiB write", estGiB) + fmt.Fprintf(&b, "; the filesystem is already below it, before this app's estimated %s write", humanizeBytes(estBytes)) default: b.WriteString("; this app has no previous backup on disk, so only current headroom was considered") } @@ -169,7 +178,7 @@ func floorRefusal(reason floorReason, usage *UnitSpace, estGiB float64, hasEst b return fmt.Errorf(b.String(), ErrCaptureFloor, usage) } -// estimatedWriteGiB estimates what this app's three legs are about to write, from what the PREVIOUS +// estimatedWriteBytes estimates what this app's three legs are about to write, from what the PREVIOUS // run left in its unit: the `.sql` dumps and the `.tar` volume archives already on disk for this app. // // WHY THIS ESTIMATOR. It is free — two ReadDirs of a directory the caller is about to write into — and @@ -182,7 +191,7 @@ func floorRefusal(reason floorReason, usage *UnitSpace, estGiB float64, hasEst b // It reads the app's CURRENT unit root, so an app that moved drives estimates from its new (probably // empty) location and is treated as history-less — conservative in the admitting direction, which is // the right way round for an estimate that only ever tightens a threshold. -func (m *Manager) estimatedWriteGiB(stackName string) (float64, bool) { +func (m *Manager) estimatedWriteBytes(stackName string) (int64, bool) { drivePath := m.GetAppDrivePath(stackName) if drivePath == "" { return 0, false @@ -204,7 +213,7 @@ func (m *Manager) estimatedWriteGiB(stackName string) (float64, bool) { if !found { return 0, false } - return float64(total) / (1024 * 1024 * 1024), true + return total, true } // sumFileSizes totals the sizes of files with the given suffix in dir. The bool reports whether ANY diff --git a/controller/internal/backup/admission_test.go b/controller/internal/backup/admission_test.go index f6dbbf6..dd26b98 100644 --- a/controller/internal/backup/admission_test.go +++ b/controller/internal/backup/admission_test.go @@ -426,7 +426,7 @@ func TestAdmission_SizeTermRefusesAnAppWhoseOwnWriteWouldCrossTheReserve(t *test if !strings.Contains(h.logs.String(), "(size)") { t.Fatalf("the refusal was not attributed to the SIZE term:\n%s", h.logs.String()) } - if !strings.Contains(h.events[0].err, "last backup was 2.00 GiB") { + if !strings.Contains(h.events[0].err, "last backup was 2.0 GB") { t.Fatalf("the alert does not carry the estimate that produced the refusal: %q", h.events[0].err) } if len(h.volDumped) != 0 { @@ -443,8 +443,8 @@ func TestAdmission_FirstEverBackupIsAdmitted(t *testing.T) { h := newAdmissionHarness(t, "brandnew") h.setSpace("brandnew", 40, 600, 1000) // ample room, and NO previous unit on disk - if est, ok := h.m.estimatedWriteGiB("brandnew"); ok || est != 0 { - t.Fatalf("estimatedWriteGiB = (%v, %v) for an app with no history, want (0, false)", est, ok) + if est, ok := h.m.estimatedWriteBytes("brandnew"); ok || est != 0 { + t.Fatalf("estimatedWriteBytes = (%v, %v) for an app with no history, want (0, false)", est, ok) } h.runOneBackupRun() @@ -558,16 +558,16 @@ func TestAdmission_UnreadableFilesystemAdmitsEveryLegAndDoesNotWarn(t *testing.T // ── The estimator, through the production path (no seam) ───────────────────────────────────────── -func TestEstimatedWriteGiB_SumsTheAppsPreviousDumpsFromRealFiles(t *testing.T) { +func TestEstimatedWriteBytes_SumsTheAppsPreviousDumpsFromRealFiles(t *testing.T) { h := newAdmissionHarness(t, "opengist") h.seedUnit(t, "opengist", 3<<30) // 3 GiB sparse tar + a small .sql - est, ok := h.m.estimatedWriteGiB("opengist") + est, ok := h.m.estimatedWriteBytes("opengist") if !ok { t.Fatal("history on disk was not recognised as history") } - if est < 3.0 || est > 3.001 { - t.Fatalf("estimate = %.4f GiB, want ~3.0 (the .tar plus the small .sql)", est) + if est < 3<<30 || est > (3<<30)+4096 { + t.Fatalf("estimate = %d B, want ~%d (the .tar plus the small .sql)", est, int64(3)<<30) } // An app whose unit exists but holds no dumps yet is history-LESS, not a zero-byte estimate. @@ -575,7 +575,7 @@ func TestEstimatedWriteGiB_SumsTheAppsPreviousDumpsFromRealFiles(t *testing.T) { if err := os.MkdirAll(other, 0o755); err != nil { t.Fatal(err) } - if est, ok := h.m.estimatedWriteGiB("empty"); ok || est != 0 { + if est, ok := h.m.estimatedWriteBytes("empty"); ok || est != 0 { t.Fatalf("an empty unit reported history (%v, %v) — an absent dump is not a 0-byte one", est, ok) } }