v0.193.1 — the refusal's size estimate is rendered in bytes, not "0.00 GiB" (R-181 follow-on)
gates / gates (push) Successful in 9s
gates / gates (push) Successful in 9s
Found by v0.193.0's own live proof run. The estimate was printed 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 opposite of what happened. Observed live on demo-hp 08:59:46: opengist's real 178 KB estimate printed as 0.00 GiB. Shipped in the same session because it is the same defect class R-181 is about: a message 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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user