From e901769cb2c2bf88cc32e044fcf2b9a9c99c8776 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 00:43:53 +0100 Subject: [PATCH] Read the newest steps first: sort the node's keys by id (start order) before the cut, and say older steps were not read The review found a cut read took an arbitrary 1000 keys, so the newest steps could be missing. A step id is base-36 milliseconds plus random digits, so its string order is its start order; the newest max(limit*4, 100) keys are read now, 1000 at most. The toolchains read logs when it drops keys past its cap. --- .../mesh-build-go/cmd/mesh-build-go/read.go | 17 ++++++++++------- .../cmd/mesh-build-go/read_test.go | 19 +++++++++++-------- .../cmd/mesh-build-image/read.go | 12 +++++++----- .../cmd/mesh-build-image/read_test.go | 19 +++++++++++-------- .../cmd/mesh-build-node/read.go | 17 ++++++++++------- .../cmd/mesh-build-node/read_test.go | 19 +++++++++++-------- 6 files changed, 60 insertions(+), 43 deletions(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/read.go b/modules/mesh-build-go/cmd/mesh-build-go/read.go index fd2706b3..2ab40bbb 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/read.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/read.go @@ -131,8 +131,9 @@ type StepLine struct { } // RecentSteps are this node's steps, the latest started first, at most limit (ten when limit is not above zero). Only -// the node's own keys (.) are read, at most MaxStateReads of them; cut says more were stored than were read, so -// the newest may be missing. A step's id says nothing of when it started, so the values are what orders them. +// the node's own keys (.) are listed and the newest max(limit*4, 100) of them read (at most MaxStateReads): a step's +// id is base-36 milliseconds and random digits, so its string order is its start order. cut says older steps were not +// read. The values still order the lines, since an ask step's id carries the ask time. func (m *Module) RecentSteps(limit int) (lines []StepLine, cut bool, err error) { if limit <= 0 { limit = 10 @@ -153,8 +154,9 @@ func (m *Module) RecentSteps(limit int) (lines []StepLine, cut bool, err error) } } } - if len(own) > MaxStateReads { - own, cut = own[:MaxStateReads], true + sort.Sort(sort.Reverse(sort.StringSlice(own))) + if n := min(max(limit*4, 100), MaxStateReads); len(own) > n { + own, cut = own[:n], true } raws, err := readValues(own, m.ReadSteps.Get) log.Printf("steps read: %d of %d keys listed read in %v (cut %v)", len(own), len(keys), time.Since(start).Round(time.Millisecond), cut) @@ -197,11 +199,12 @@ func (m *Module) ReadToolchains() (map[string]delivery.Toolchain, error) { if err != nil { return nil, err } - if len(keys) > MaxStateReads { + cut := len(keys) > MaxStateReads + if cut { keys = keys[:MaxStateReads] // a node has a key per language: far fewer in practice } raws, err := readValues(keys, m.ReadToolchainsState.Get) - log.Printf("toolchains read: %d keys read in %v", len(keys), time.Since(start).Round(time.Millisecond)) + log.Printf("toolchains read: %d keys read in %v (cut %v: keys past %d were not read)", len(keys), time.Since(start).Round(time.Millisecond), cut, MaxStateReads) if err != nil { return nil, err } @@ -242,7 +245,7 @@ func (m *Module) ReadTools() []stdio.Tool { ans := map[string]any{"node": m.Node, "steps": steps} if cut { ans["cut"] = true - ans["note"] = fmt.Sprintf("this node has more than %d steps stored; %d were read, so the newest may be missing", MaxStateReads, MaxStateReads) + ans["note"] = fmt.Sprintf("this node has more steps stored than one call reads; older steps were not read (ask for a larger limit, at most %d reads)", MaxStateReads) } return ans, err }}, diff --git a/modules/mesh-build-go/cmd/mesh-build-go/read_test.go b/modules/mesh-build-go/cmd/mesh-build-go/read_test.go index 7880c4af..04049540 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/read_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/read_test.go @@ -153,14 +153,14 @@ func TestReadToolsAreInTheManifestAndNeedNoToolchain(t *testing.T) { func TestRecentStepsReadsOnlyTheNodesKeysInParallelAndBounded(t *testing.T) { t0 := time.Date(2026, 10, 25, 9, 0, 0, 0, time.UTC) for _, row := range []struct { - perNode int - reads int64 - steps int - cut bool + perNode, limit int + reads int64 + steps int + cut bool }{ - {20, 20, 10, false}, - {600, 600, 10, false}, - {1500, MaxStateReads, 10, true}, + {20, 0, 20, 10, false}, + {600, 0, 100, 10, true}, + {1500, 300, MaxStateReads, 300, true}, } { var reads, inFlight, maxFlight atomic.Int64 stored := map[string][]byte{} @@ -193,10 +193,13 @@ func TestRecentStepsReadsOnlyTheNodesKeysInParallelAndBounded(t *testing.T) { }, } start := time.Now() - lines, cut, err := m.RecentSteps(0) + lines, cut, err := m.RecentSteps(row.limit) if err != nil || len(lines) != row.steps || cut != row.cut || reads.Load() != row.reads { t.Errorf("%d a node: %d steps, cut %v, %d reads, err %v; want %d, %v, %d", row.perNode, len(lines), cut, reads.Load(), err, row.steps, row.cut, row.reads) } + if row.cut && len(lines) > 0 && lines[0].ID != fmt.Sprintf("s%04d", row.perNode-1) { + t.Errorf("%d a node: newest answered %s", row.perNode, lines[0].ID) + } if maxFlight.Load() < 2 { t.Errorf("%d a node: values read one at a time", row.perNode) } diff --git a/modules/mesh-build-image/cmd/mesh-build-image/read.go b/modules/mesh-build-image/cmd/mesh-build-image/read.go index fc62afc8..b97ddccc 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/read.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/read.go @@ -130,8 +130,9 @@ type StepLine struct { } // RecentSteps are this node's steps, the latest started first, at most limit (ten when limit is not above zero). Only -// the node's own keys (.) are read, at most MaxStateReads of them; cut says more were stored than were read, so -// the newest may be missing. A step's id says nothing of when it started, so the values are what orders them. +// the node's own keys (.) are listed and the newest max(limit*4, 100) of them read (at most MaxStateReads): a step's +// id is base-36 milliseconds and random digits, so its string order is its start order. cut says older steps were not +// read. The values still order the lines, since an ask step's id carries the ask time. func (m *Module) RecentSteps(limit int) (lines []StepLine, cut bool, err error) { if limit <= 0 { limit = 10 @@ -152,8 +153,9 @@ func (m *Module) RecentSteps(limit int) (lines []StepLine, cut bool, err error) } } } - if len(own) > MaxStateReads { - own, cut = own[:MaxStateReads], true + sort.Sort(sort.Reverse(sort.StringSlice(own))) + if n := min(max(limit*4, 100), MaxStateReads); len(own) > n { + own, cut = own[:n], true } raws, err := readValues(own, m.ReadSteps.Get) log.Printf("steps read: %d of %d keys listed read in %v (cut %v)", len(own), len(keys), time.Since(start).Round(time.Millisecond), cut) @@ -209,7 +211,7 @@ func (m *Module) ReadTools() []stdio.Tool { ans := map[string]any{"node": m.Node, "steps": steps} if cut { ans["cut"] = true - ans["note"] = fmt.Sprintf("this node has more than %d steps stored; %d were read, so the newest may be missing", MaxStateReads, MaxStateReads) + ans["note"] = fmt.Sprintf("this node has more steps stored than one call reads; older steps were not read (ask for a larger limit, at most %d reads)", MaxStateReads) } return ans, err }}, diff --git a/modules/mesh-build-image/cmd/mesh-build-image/read_test.go b/modules/mesh-build-image/cmd/mesh-build-image/read_test.go index d7b8214b..61c6fc95 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/read_test.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/read_test.go @@ -54,14 +54,14 @@ func TestTheReadToolsAnswerAStepsResult(t *testing.T) { func TestRecentStepsReadsOnlyTheNodesKeysInParallelAndBounded(t *testing.T) { t0 := time.Date(2026, 10, 25, 9, 0, 0, 0, time.UTC) for _, row := range []struct { - perNode int - reads int64 - steps int - cut bool + perNode, limit int + reads int64 + steps int + cut bool }{ - {20, 20, 10, false}, - {600, 600, 10, false}, - {1500, MaxStateReads, 10, true}, + {20, 0, 20, 10, false}, + {600, 0, 100, 10, true}, + {1500, 300, MaxStateReads, 300, true}, } { var reads, inFlight, maxFlight atomic.Int64 stored := map[string][]byte{} @@ -94,10 +94,13 @@ func TestRecentStepsReadsOnlyTheNodesKeysInParallelAndBounded(t *testing.T) { }, } start := time.Now() - lines, cut, err := m.RecentSteps(0) + lines, cut, err := m.RecentSteps(row.limit) if err != nil || len(lines) != row.steps || cut != row.cut || reads.Load() != row.reads { t.Errorf("%d a node: %d steps, cut %v, %d reads, err %v; want %d, %v, %d", row.perNode, len(lines), cut, reads.Load(), err, row.steps, row.cut, row.reads) } + if row.cut && len(lines) > 0 && lines[0].ID != fmt.Sprintf("s%04d", row.perNode-1) { + t.Errorf("%d a node: newest answered %s", row.perNode, lines[0].ID) + } if maxFlight.Load() < 2 { t.Errorf("%d a node: values read one at a time", row.perNode) } diff --git a/modules/mesh-build-node/cmd/mesh-build-node/read.go b/modules/mesh-build-node/cmd/mesh-build-node/read.go index c89dba94..4b290eaf 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/read.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/read.go @@ -131,8 +131,9 @@ type StepLine struct { } // RecentSteps are this node's steps, the latest started first, at most limit (ten when limit is not above zero). Only -// the node's own keys (.) are read, at most MaxStateReads of them; cut says more were stored than were read, so -// the newest may be missing. A step's id says nothing of when it started, so the values are what orders them. +// the node's own keys (.) are listed and the newest max(limit*4, 100) of them read (at most MaxStateReads): a step's +// id is base-36 milliseconds and random digits, so its string order is its start order. cut says older steps were not +// read. The values still order the lines, since an ask step's id carries the ask time. func (m *Module) RecentSteps(limit int) (lines []StepLine, cut bool, err error) { if limit <= 0 { limit = 10 @@ -153,8 +154,9 @@ func (m *Module) RecentSteps(limit int) (lines []StepLine, cut bool, err error) } } } - if len(own) > MaxStateReads { - own, cut = own[:MaxStateReads], true + sort.Sort(sort.Reverse(sort.StringSlice(own))) + if n := min(max(limit*4, 100), MaxStateReads); len(own) > n { + own, cut = own[:n], true } raws, err := readValues(own, m.ReadSteps.Get) log.Printf("steps read: %d of %d keys listed read in %v (cut %v)", len(own), len(keys), time.Since(start).Round(time.Millisecond), cut) @@ -197,11 +199,12 @@ func (m *Module) ReadToolchains() (map[string]delivery.Toolchain, error) { if err != nil { return nil, err } - if len(keys) > MaxStateReads { + cut := len(keys) > MaxStateReads + if cut { keys = keys[:MaxStateReads] // a node has a key per language: far fewer in practice } raws, err := readValues(keys, m.ReadToolchainsState.Get) - log.Printf("toolchains read: %d keys read in %v", len(keys), time.Since(start).Round(time.Millisecond)) + log.Printf("toolchains read: %d keys read in %v (cut %v: keys past %d were not read)", len(keys), time.Since(start).Round(time.Millisecond), cut, MaxStateReads) if err != nil { return nil, err } @@ -242,7 +245,7 @@ func (m *Module) ReadTools() []stdio.Tool { ans := map[string]any{"node": m.Node, "steps": steps} if cut { ans["cut"] = true - ans["note"] = fmt.Sprintf("this node has more than %d steps stored; %d were read, so the newest may be missing", MaxStateReads, MaxStateReads) + ans["note"] = fmt.Sprintf("this node has more steps stored than one call reads; older steps were not read (ask for a larger limit, at most %d reads)", MaxStateReads) } return ans, err }}, diff --git a/modules/mesh-build-node/cmd/mesh-build-node/read_test.go b/modules/mesh-build-node/cmd/mesh-build-node/read_test.go index d0b68425..cc0d85ef 100644 --- a/modules/mesh-build-node/cmd/mesh-build-node/read_test.go +++ b/modules/mesh-build-node/cmd/mesh-build-node/read_test.go @@ -153,14 +153,14 @@ func TestReadToolsAreInTheManifestAndNeedNoToolchain(t *testing.T) { func TestRecentStepsReadsOnlyTheNodesKeysInParallelAndBounded(t *testing.T) { t0 := time.Date(2026, 10, 25, 9, 0, 0, 0, time.UTC) for _, row := range []struct { - perNode int - reads int64 - steps int - cut bool + perNode, limit int + reads int64 + steps int + cut bool }{ - {20, 20, 10, false}, - {600, 600, 10, false}, - {1500, MaxStateReads, 10, true}, + {20, 0, 20, 10, false}, + {600, 0, 100, 10, true}, + {1500, 300, MaxStateReads, 300, true}, } { var reads, inFlight, maxFlight atomic.Int64 stored := map[string][]byte{} @@ -193,10 +193,13 @@ func TestRecentStepsReadsOnlyTheNodesKeysInParallelAndBounded(t *testing.T) { }, } start := time.Now() - lines, cut, err := m.RecentSteps(0) + lines, cut, err := m.RecentSteps(row.limit) if err != nil || len(lines) != row.steps || cut != row.cut || reads.Load() != row.reads { t.Errorf("%d a node: %d steps, cut %v, %d reads, err %v; want %d, %v, %d", row.perNode, len(lines), cut, reads.Load(), err, row.steps, row.cut, row.reads) } + if row.cut && len(lines) > 0 && lines[0].ID != fmt.Sprintf("s%04d", row.perNode-1) { + t.Errorf("%d a node: newest answered %s", row.perNode, lines[0].ID) + } if maxFlight.Load() < 2 { t.Errorf("%d a node: values read one at a time", row.perNode) }