diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/holder.go b/modules/artifact-store-tools/cmd/artifact-store-tools/holder.go index e6e626cb9..309d4248a 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/holder.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/holder.go @@ -93,12 +93,10 @@ type Holder struct { Now func() time.Time Logf func(string, ...any) - sweep sync.Mutex // one sweep at a time - mu sync.Mutex - fmu sync.Mutex // the findings said and the sweep's refusal - said map[string]meshconditions.Finding // S25's findings as last written, by key - refused *meshconditions.Finding // the sweep's finding while a seat's refusal stands - ended map[string]time.Time // a done key's end, read once from its history + sweep sync.Mutex // one sweep at a time + mu sync.Mutex + fmu sync.Mutex // one pass of Observe at a time + ended map[string]time.Time // a done key's end, read once from its history } // Build is one done build of a module, as its key holds it, with when it ended. @@ -856,7 +854,8 @@ func inStoreAt(image, address string) string { } // Observe says S25's findings (image-missing): each want whose build names an image the store does not hold, on the -// want's machine, written once and cleared once it no longer holds. Nothing is said without a listing. +// want's machine, written once and cleared once it no longer holds. Nothing is said without a listing. What stands is what +// the held writer holds (design 47 G A-32, A-38), so a restart neither drops nor rewrites it. func (h *Holder) Observe(ctx context.Context) error { now, err := h.observed(ctx) if err != nil { @@ -867,52 +866,36 @@ func (h *Holder) Observe(ctx context.Context) error { } h.fmu.Lock() defer h.fmu.Unlock() - if h.said == nil { - h.said = map[string]meshconditions.Finding{} + said := map[string]meshconditions.Finding{} + for _, f := range h.Findings.Held() { + if f.Kind == ImageMissing { + said[views.FindingKey(f.Scope, f.ID, f.Kind)] = f + } } - for k, was := range h.said { + for k, was := range said { if _, still := now[k]; !still { if err := h.Findings.Clear(ctx, was.Scope, was.ID, was.Kind, "the store holds the image again"); err != nil { return err } - delete(h.said, k) } } for k, f := range now { - if was, ok := h.said[k]; ok && was.Summary == f.Summary { + if was, ok := said[k]; ok && was.Summary == f.Summary { continue // written only when what it says changes } if err := h.Findings.Write(ctx, f); err != nil { return err } - h.said[k] = f } return nil } -// Findings is the module's finding writer (views.Writer): the holder writes and clears through it (design 47 G §689). +// Findings is the module's held finding writer (views.HeldWriter): the holder writes and clears through it and reads what it +// holds (design 47 G §689, A-29, A-38). type Findings interface { Write(ctx context.Context, finding meshconditions.Finding) error Clear(ctx context.Context, scope, id, kind, why string) error -} - -// FindingsObserved answers the findings this module observes now, for its writer's statement (views.WriterConfig.Observe): -// S25's findings and the sweep's, when a seat refused it and no clean sweep cleared it. -func (h *Holder) FindingsObserved(ctx context.Context) (views.Observed[meshconditions.Finding], error) { - now, err := h.observed(ctx) - if err != nil { - return views.Observed[meshconditions.Finding]{}, err - } - var out views.Observed[meshconditions.Finding] - for _, f := range now { - out.Findings = append(out.Findings, f) - } - h.fmu.Lock() - defer h.fmu.Unlock() - if h.refused != nil { - out.Findings = append(out.Findings, *h.refused) - } - return out, nil + Held() []meshconditions.Finding } // observed is S25's findings now by key, nil without a listing. diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go b/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go index a5df807cd..0c1a05d98 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/holder_test.go @@ -418,6 +418,8 @@ func TestAWantedImageTheStoreLacksIsAFindingAndImagesNamesWantsAndHaves(t *testi t.Fatalf("the finding is %+v (%v, %v)", f, found, err) } view.L.Repos[repo].Revisions[digest] = true + // A restart: a holder made afresh holds no finding of its own, and clears what the writer holds (design 47 G A-32, A-38). + h = &Holder{Bus: h.Bus, Findings: h.Findings, Door: h.Door, Listed: h.Listed, Seats: h.Seats, Now: h.Now, Acts: h.Acts, Logf: h.Logf} if err := h.Observe(context.Background()); err != nil { t.Fatal(err) } @@ -459,6 +461,24 @@ func (f findingsThrough) Write(_ context.Context, finding meshconditions.Finding return f.bus.Put(FindingsState, views.FindingKey(finding.Scope, finding.ID, finding.Kind), finding) } +// Held is what the writer holds: the findings in the module's state. +func (f findingsThrough) Held() []meshconditions.Finding { + var out []meshconditions.Finding + keys, _ := f.bus.(interface { + Keys(string) ([]string, error) + }).Keys(FindingsState) + for _, k := range keys { + raw, ok, _ := f.bus.(interface { + Get(string, string) ([]byte, bool, error) + }).Get(FindingsState, k) + var m meshconditions.Finding + if ok && json.Unmarshal(raw, &m) == nil { + out = append(out, m) + } + } + return out +} + func (f findingsThrough) Clear(_ context.Context, scope, id, kind, _ string) error { return f.bus.Delete(FindingsState, views.FindingKey(scope, id, kind)) } diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/main.go b/modules/artifact-store-tools/cmd/artifact-store-tools/main.go index 14394b18b..acb1d80d5 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/main.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/main.go @@ -30,9 +30,9 @@ func main() { h := &Holder{Bus: runtimeBus{views: v}, Door: DoorAt(os.Getenv("MESH_ARTIFACT_STORE_ADDRESS_FILE"), s), Listed: s.View, Seats: stdio.Ask, Now: time.Now, Logf: logf} h.Acts = meshhandactsclient.Recorder{Ask: stdio.Ask} - // The module's one writer (design 47 G §689, A-16): the holder's findings and the reader's go through it. - w, err := views.NewStdioWriter(views.WriterConfig[meshconditions.Finding, meshconditions.FindingCleared, meshconditions.FindingsStated, meshconditions.FindingsAskedEvent]{ - Asked: meshconditions.SubjectFindingsAsked, Module: Module, State: stdio.State("findings"), Kinds: []string{ImageMissing, SweepRefused}, Observe: h.FindingsObserved}) + // The module's one held writer (design 47 G §689, A-16, A-29, A-32): the holder's findings and the reader's go through it. + w, err := views.NewStdioHeldWriter(views.WriterConfig[meshconditions.Finding, meshconditions.FindingCleared, meshconditions.FindingsStated, meshconditions.FindingsAskedEvent]{ + Asked: meshconditions.SubjectFindingsAsked, Module: Module, State: stdio.State("findings"), Kinds: []string{ImageMissing, SweepRefused}}) if err != nil { logf("%v", err) os.Exit(1) diff --git a/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go b/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go index 34c85d483..b106f1cf7 100644 --- a/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go +++ b/modules/artifact-store-tools/cmd/artifact-store-tools/sweepseats.go @@ -123,25 +123,20 @@ func (h *Holder) sweepFinding(what string, err error) { Needs: "read the seat holder's journal for the refusal; the next sweep asks again.", Evidence: []meshconditions.Evidence{{At: at.UTC(), Said: fmt.Sprintf("%s: %v", what, err)}}, Source: "CAT-AS1", Observer: Module, At: at} - h.fmu.Lock() - h.refused = &f - h.fmu.Unlock() if werr := h.Findings.Write(context.Background(), f); werr != nil { h.logf("the sweep's finding (%s: %v) could not be written: %v", what, err, werr) } } -// sweepClean clears the sweep's finding after a sweep no seat refused. +// sweepClean clears the sweep's finding, when the held writer holds it, after a sweep no seat refused. func (h *Holder) sweepClean() { - h.fmu.Lock() - was := h.refused - h.refused = nil - h.fmu.Unlock() - if was == nil { - return - } - if err := h.Findings.Clear(context.Background(), was.Scope, was.ID, was.Kind, "a sweep ran through without a refusal"); err != nil { - h.logf("the sweep's finding could not be cleared: %v", err) + for _, was := range h.Findings.Held() { + if was.Kind != SweepRefused { + continue + } + if err := h.Findings.Clear(context.Background(), was.Scope, was.ID, was.Kind, "a sweep ran through without a refusal"); err != nil { + h.logf("the sweep's finding could not be cleared: %v", err) + } } } diff --git a/modules/mesh-config/cmd/mesh-config/config.go b/modules/mesh-config/cmd/mesh-config/config.go index 0b62393dd..1708f57a8 100644 --- a/modules/mesh-config/cmd/mesh-config/config.go +++ b/modules/mesh-config/cmd/mesh-config/config.go @@ -86,10 +86,9 @@ type Config struct { Logf func(format string, a ...any) mu sync.Mutex - // Findings is the module's finding writer (design 47 G §689); nil, no finding is said. findMu guards the findings open. + // Findings is the module's finding writer (design 47 G §689); nil, no finding is said. findMu serializes a pass over what it holds. Findings FindingsWriter findMu sync.Mutex - open map[string]openFinding // withheld is why the newest list of predefined keys was not published, empty when it was (publish). withheld string diff --git a/modules/mesh-config/cmd/mesh-config/findings.go b/modules/mesh-config/cmd/mesh-config/findings.go index 42c4d7fd5..09531271c 100644 --- a/modules/mesh-config/cmd/mesh-config/findings.go +++ b/modules/mesh-config/cmd/mesh-config/findings.go @@ -72,20 +72,15 @@ func windowHeld(r Rename, node string, at time.Time) meshconditions.Finding { fmt.Sprintf("the window opened %s and %s has not reported a build reading %s", r.Nodes[node].Since.UTC().Format(time.RFC3339), node, r.To), at) } -// FindingsWriter is the module's finding writer (views.Writer), through which mesh-config writes and clears its findings -// (design 47 G §689). +// FindingsWriter is the module's held finding writer (views.HeldWriter), through which mesh-config writes and clears its +// findings and reads what it holds (design 47 G §689, A-29, A-38): the module keeps no set of its own. type FindingsWriter interface { Write(ctx context.Context, finding meshconditions.Finding) error Clear(ctx context.Context, scope, id, kind, why string) error + Held() []meshconditions.Finding } -// openFinding is a finding written and not yet cleared, with the parts of its id. -type openFinding struct { - f meshconditions.Finding - module, ref string -} - -// setFinding writes a finding when it differs from the one written; the module and ref are the parts of its id. +// setFinding writes a finding when it differs from the one held; the module and ref are the parts of its id. func (c *Config) setFinding(module, ref string, f meshconditions.Finding) { if c.Findings == nil { return @@ -97,17 +92,14 @@ func (c *Config) setFinding(module, ref string, f meshconditions.Finding) { key := views.FindingKey(f.Scope, f.ID, f.Kind) c.findMu.Lock() defer c.findMu.Unlock() - if old, ok := c.open[key]; ok && old.f.Summary == f.Summary { - return // what it says has not changed + for _, old := range c.Findings.Held() { + if views.FindingKey(old.Scope, old.ID, old.Kind) == key && old.Summary == f.Summary { + return // what it says has not changed + } } if err := c.Findings.Write(context.Background(), f); err != nil { c.logf("[mesh-config] the finding %s is not written: %v", key, err) - return } - if c.open == nil { - c.open = map[string]openFinding{} - } - c.open[key] = openFinding{f: f, module: module, ref: ref} } // clearFinding clears the findings of a kind and ref about a module, on whatever node but those in keep. @@ -117,30 +109,17 @@ func (c *Config) clearFinding(kind, module, ref string, keep ...string) { } c.findMu.Lock() defer c.findMu.Unlock() - for k, o := range c.open { - if o.f.Kind != kind || o.ref != ref || o.module != module || - (o.f.Subject.Machine != "" && slices.Contains(keep, o.f.Subject.Machine)) { + for _, f := range c.Findings.Held() { + if f.Kind != kind || f.ID != findingID(module, f.Subject.Machine, ref) || + (f.Subject.Machine != "" && slices.Contains(keep, f.Subject.Machine)) { continue } - if err := c.Findings.Clear(context.Background(), o.f.Scope, o.f.ID, o.f.Kind, "the fault is gone"); err != nil { - c.logf("[mesh-config] the finding %s is not cleared: %v", k, err) - continue + if err := c.Findings.Clear(context.Background(), f.Scope, f.ID, f.Kind, "the fault is gone"); err != nil { + c.logf("[mesh-config] the finding %s is not cleared: %v", views.FindingKey(f.Scope, f.ID, f.Kind), err) } - delete(c.open, k) } } -// FindingsObserved is the findings open now, for the writer's statement (views.WriterConfig.Observe). -func (c *Config) FindingsObserved(context.Context) (views.Observed[meshconditions.Finding], error) { - c.findMu.Lock() - defer c.findMu.Unlock() - var out views.Observed[meshconditions.Finding] - for _, o := range c.open { - out.Findings = append(out.Findings, o.f) - } - return out, nil -} - // retirementHeld is the finding of a retirement that waits seven days for nodes that still run a build reading the value. func retirementHeld(e Retiring, at time.Time) meshconditions.Finding { node := "" diff --git a/modules/mesh-config/cmd/mesh-config/main.go b/modules/mesh-config/cmd/mesh-config/main.go index 44d1b28d0..d094be9b8 100644 --- a/modules/mesh-config/cmd/mesh-config/main.go +++ b/modules/mesh-config/cmd/mesh-config/main.go @@ -121,9 +121,9 @@ func (runtimeAsks) Warranted(ask string) error { func main() { c := &Config{Store: runtimeStore{}, Logf: logf, Asks: runtimeAsks{}, Forge: toolForge{ask: stdio.Ask}, Delivery: toolDelivery{ask: stdio.Ask}, Vault: toolVault{ask: stdio.Ask, recorder: recorder}} - // The module's one writer (design 47 G §689, A-16): the renames' and retirements' findings and the views reader's go through it. - w, err := views.NewStdioWriter(views.WriterConfig[meshconditions.Finding, meshconditions.FindingCleared, meshconditions.FindingsStated, meshconditions.FindingsAskedEvent]{ - Asked: meshconditions.SubjectFindingsAsked, Module: Module, State: stdio.State(FindingsState), Observe: c.FindingsObserved, + // The module's one held writer (design 47 G §689, A-16, A-29, A-32): the renames' and retirements' findings and the views reader's go through it. + w, err := views.NewStdioHeldWriter(views.WriterConfig[meshconditions.Finding, meshconditions.FindingCleared, meshconditions.FindingsStated, meshconditions.FindingsAskedEvent]{ + Asked: meshconditions.SubjectFindingsAsked, Module: Module, State: stdio.State(FindingsState), Kinds: []string{FindingRenameRefused, FindingWindowHeld, FindingRetireUnanswered, FindingRetireHeld}}) if err != nil { logf("%v", err) diff --git a/modules/mesh-config/cmd/mesh-config/renames_test.go b/modules/mesh-config/cmd/mesh-config/renames_test.go index 7f7c6557d..4655fa358 100644 --- a/modules/mesh-config/cmd/mesh-config/renames_test.go +++ b/modules/mesh-config/cmd/mesh-config/renames_test.go @@ -274,6 +274,8 @@ func TestAWindowHeldSevenDaysIsAFindingAndAHandCloseIsAHandAct(t *testing.T) { t.Error("a node holding no open window was dropped") } m.handActs = nil + // A restart in between: the module holds no finding of its own, the writer holds it (design 47 G A-32, A-38). + c = &Config{Store: c.Store, Now: c.Now, Logf: c.Logf, Delivery: c.Delivery, Findings: c.Findings} if _, err := c.RenameClose("player", "level", "novox", "novox is retired for good"); err != nil { t.Fatal(err) } @@ -443,6 +445,19 @@ func (m *memStore) Write(_ context.Context, f meshconditions.Finding) error { return m.Put(FindingsState, views.FindingKey(f.Scope, f.ID, f.Kind), f) } +// Held is what the writer holds: the findings in the module's state. +func (m *memStore) Held() []meshconditions.Finding { + var out []meshconditions.Finding + for _, k := range m.keysOf(FindingsState) { + e, _ := m.Get(FindingsState, k) + var f meshconditions.Finding + if e != nil && json.Unmarshal(e.Value, &f) == nil { + out = append(out, f) + } + } + return out +} + func (m *memStore) Clear(_ context.Context, scope, id, kind, _ string) error { return m.Delete(FindingsState, views.FindingKey(scope, id, kind)) }