diff --git a/cmd/mesh-controller/bucket_log_fill.go b/cmd/mesh-controller/bucket_log_fill.go index 4fd8fa88..619b8ebc 100644 --- a/cmd/mesh-controller/bucket_log_fill.go +++ b/cmd/mesh-controller/bucket_log_fill.go @@ -103,19 +103,12 @@ func unreadObservation(s bucketOrLog, why error) conditions.Observation { } // keptFilling is an open fill condition said again for a bucket or log that could not be read this -// time: not knowing how full it is now is no reason to say it has room. +// time: not knowing how full it is now is no reason to say it has room. Only ever made from a condition +// read back as open — its own summary and severity, never words made up for it. func keptFilling(s bucketOrLog, open conditions.Condition, why error) conditions.Observation { - severity := open.Severity - if severity == "" { - severity = conditions.Warning - } - summary := open.Summary - if summary == "" { - summary = fmt.Sprintf("%s's %s %s was filling when it was last read", s.Module, s.Kind, s.Name) - } return conditions.Observation{ Scope: conditions.ScopeBus, ID: s.Stream, Token: "filling", Kind: kindBucketOrLogFilling, - Severity: severity, Summary: summary, + Severity: open.Severity, Summary: open.Summary, Said: fmt.Sprintf("module %s, %s %s (stream %s): not read this time (%v); kept open as it was", s.Module, s.Kind, s.Name, s.Stream, why), } @@ -129,18 +122,21 @@ func mibWords(b uint64) string { // readFill is how full one bucket or log is on the bus; found false when it is not on the bus. type readFill func(ctx context.Context, s bucketOrLog) (f filled, found bool, err error) -// openFill is the fill condition open under a key, if one is. -type openFill func(ctx context.Context, key string) (conditions.Condition, bool) +// openFill is the fill condition open under a key, if one is, or why it could not be read. +type openFill func(ctx context.Context, key string) (conditions.Condition, bool, error) // judgeFills judges every bucket and log on its own: one that cannot be read is said as unread, with -// its open fill condition kept, and every other is judged by its fill. +// its fill condition kept only when that condition was read back and is open — when it cannot be read +// either, nothing is said of its fill, which the unread finding already covers. Every other is judged +// by its fill; for one of those, a condition that cannot be read is taken as open, so an unknown never +// clears a fill measured between fillClearBelow and fillRaiseAt. func judgeFills(ctx context.Context, all []bucketOrLog, read readFill, open openFill) []conditions.Observation { var out []conditions.Observation for _, s := range all { f, found, err := read(ctx, s) if err != nil { out = append(out, unreadObservation(s, err)) - if c, isOpen := open(ctx, fillKey(s)); isOpen { + if c, isOpen, cerr := open(ctx, fillKey(s)); cerr == nil && isOpen { out = append(out, keptFilling(s, c, err)) } continue @@ -148,7 +144,8 @@ func judgeFills(ctx context.Context, all []bucketOrLog, read readFill, open open if !found { continue // not on the bus: created on the controller's next raise, and nothing there can fill } - _, isOpen := open(ctx, fillKey(s)) + _, isOpen, cerr := open(ctx, fillKey(s)) + isOpen = isOpen || cerr != nil if o, said := fillObservation(s, f, isOpen); said { out = append(out, o) } @@ -200,13 +197,11 @@ func probeFill(ctx context.Context, d *doctor) ([]conditions.Observation, error) } return filled{Bytes: info.State.Bytes, Max: uint64(info.Config.MaxBytes)}, true, nil } - open := func(ctx context.Context, key string) (conditions.Condition, bool) { + open := func(ctx context.Context, key string) (conditions.Condition, bool, error) { if d.keeper == nil { - return conditions.Condition{}, false + return conditions.Condition{}, false, nil } - // One that cannot be read is taken as open, as confirm.go takes it: unknown is no reason to clear. - c, isOpen, err := d.keeper.Get(ctx, key) - return c, isOpen || err != nil + return d.keeper.Get(ctx, key) } return judgeFills(ctx, all, read, open), nil } diff --git a/cmd/mesh-controller/bucket_log_fill_test.go b/cmd/mesh-controller/bucket_log_fill_test.go index e9acd8f2..01f98aa6 100644 --- a/cmd/mesh-controller/bucket_log_fill_test.go +++ b/cmd/mesh-controller/bucket_log_fill_test.go @@ -84,12 +84,12 @@ func TestAnUnreadableBucketOrLogIsSaidAndTheRestAreStillJudged(t *testing.T) { } return filled{}, false, nil } - open := func(_ context.Context, key string) (conditions.Condition, bool) { + open := func(_ context.Context, key string) (conditions.Condition, bool, error) { if key == fillKey(brokenOpen) { return conditions.Condition{Key: key, Severity: conditions.Warning, - Summary: "a's log was-filling holds 800.0 MiB of 1024.0 MiB, 78%"}, true + Summary: "a's log was-filling holds 800.0 MiB of 1024.0 MiB, 78%"}, true, nil } - return conditions.Condition{}, false + return conditions.Condition{}, false, nil } got := map[string]conditions.Observation{} for _, o := range judgeFills(context.Background(), []bucketOrLog{broken, brokenOpen, full, empty, absent}, read, open) { @@ -128,3 +128,38 @@ func TestAFillingOrUnreadBucketOrLogIsSaidInPlainWords(t *testing.T) { } } } + +// **When neither the bucket or log nor its condition can be read, nothing is said of its fill** (second +// review of #231): the unread finding covers it, and no fill is invented for it. For one that is read and +// measured between seven tenths and three quarters, a condition that cannot be read is taken as open, so +// an unknown never clears it. +func TestNothingIsSaidOfAFillWhenNeitherItNorItsConditionCanBeRead(t *testing.T) { + const mib = 1024 * 1024 + broken := bucketOrLog{Module: "a", Kind: "log", Name: "changes", Stream: "LOG_a_changes"} + between := bucketOrLog{Module: "b", Kind: "bucket", Name: "issues", Stream: "KV_b_issues"} + read := func(_ context.Context, s bucketOrLog) (filled, bool, error) { + if s == broken { + return filled{}, false, errors.New("the bus did not answer") + } + return filled{Bytes: 72 * mib, Max: 100 * mib}, true, nil + } + open := func(context.Context, string) (conditions.Condition, bool, error) { + return conditions.Condition{}, false, errors.New("the conditions bucket did not answer") + } + got := map[string]conditions.Observation{} + for _, o := range judgeFills(context.Background(), []bucketOrLog{broken, between}, read, open) { + got[o.Key()] = o + } + if o, said := got[fillKey(broken)]; said { + t.Fatalf("a fill was said for a log whose fill and condition could not be read: %+v", o) + } + if _, said := got[conditions.Key(conditions.ScopeBus, broken.Stream, "unread")]; !said { + t.Error("the log that could not be read was not said as unread") + } + if _, kept := got[fillKey(between)]; !kept { + t.Error("a bucket at 72% whose condition could not be read was cleared") + } + if len(got) != 2 { + t.Errorf("said %d findings, want 2: %v", len(got), got) + } +}