fix(landscape): guard redelivery, detach terminal writes, bound workers
F2: Execute had no status guard, so a redelivered or stale message reran a finished run. Runs that are neither queued nor running are logged and acked without work; "running" still executes so a worker that crashed mid-run recovers. F4: failRun, finishNodeFailed and MarkRunFailed wrote the terminal status on the caller's context, which is exactly the context that is already cancelled when the run failed because of a shutdown or a sibling node — the write was refused and the run stayed stuck in "running". They now write on a context.WithoutCancel copy with a 10s timeout; the consumer's best-effort MarkRunFailed does the same. F5: peak memory scaled with the host's core count. LP_MAX_WORKERS (default 8) is now the total window-worker budget of a run and LP_MAX_NODES (default 2) the node fan-out; each node gets max(1, workers/nodes) through the new landscape.ComputeWithWorkers. Compute and service.ComputeFunc keep their signatures: Compute delegates with MaxWorkersPerNode, and a legacy ComputeFunc handed to NewLandscapeService is wrapped. F6: the stage loop used a plain errgroup, so a failing node let its siblings run to completion. It now uses errgroup.WithContext and passes the group context into runNode, cancelling siblings mid-compute. F7: wrap every float32 product in an explicit conversion so the compiler cannot fuse it into an FMA, keeping the kernels bit-identical to numpy's per-op rounding. F8: README's builder package list now matches the Dockerfile (adds g++), and the Landscape section documents the memory model and the two new knobs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -65,12 +65,63 @@ func (r *lockedDatasetRepo) LatestInGroup(ctx context.Context, groupID uuid.UUID
|
||||
}
|
||||
|
||||
// stubLPRepo is an in-memory LandscapeRepository for tests.
|
||||
// lpStatusWrite records one SetRunStatus call and whether the context it was
|
||||
// handed was still live.
|
||||
type lpStatusWrite struct {
|
||||
status domain.LPRunStatus
|
||||
ctxErr error
|
||||
}
|
||||
|
||||
// lpNodeWrite records one SetNodeFinished call the same way.
|
||||
type lpNodeWrite struct {
|
||||
code string
|
||||
status domain.LPNodeStatus
|
||||
ctxErr error
|
||||
}
|
||||
|
||||
type stubLPRepo struct {
|
||||
mu sync.Mutex
|
||||
runs map[uuid.UUID]*domain.LPRun
|
||||
order []uuid.UUID
|
||||
reuseLookups []string
|
||||
clock int64
|
||||
statusWrites []lpStatusWrite
|
||||
nodeWrites []lpNodeWrite
|
||||
}
|
||||
|
||||
// statusesWritten returns the run statuses written so far, in order.
|
||||
func (r *stubLPRepo) statusesWritten() []domain.LPRunStatus {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
out := make([]domain.LPRunStatus, 0, len(r.statusWrites))
|
||||
for _, w := range r.statusWrites {
|
||||
out = append(out, w.status)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// statusWrite returns the last write recording the given status.
|
||||
func (r *stubLPRepo) statusWrite(status domain.LPRunStatus) (lpStatusWrite, bool) {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
for i := len(r.statusWrites) - 1; i >= 0; i-- {
|
||||
if r.statusWrites[i].status == status {
|
||||
return r.statusWrites[i], true
|
||||
}
|
||||
}
|
||||
return lpStatusWrite{}, false
|
||||
}
|
||||
|
||||
// nodeWrite returns the last SetNodeFinished write recording the given status.
|
||||
func (r *stubLPRepo) nodeWrite(status domain.LPNodeStatus) (lpNodeWrite, bool) {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
for i := len(r.nodeWrites) - 1; i >= 0; i-- {
|
||||
if r.nodeWrites[i].status == status {
|
||||
return r.nodeWrites[i], true
|
||||
}
|
||||
}
|
||||
return lpNodeWrite{}, false
|
||||
}
|
||||
|
||||
func newStubLPRepo() *stubLPRepo {
|
||||
@@ -170,9 +221,10 @@ func (r *stubLPRepo) CountRuns(_ context.Context) (int, error) {
|
||||
return len(r.runs), nil
|
||||
}
|
||||
|
||||
func (r *stubLPRepo) SetRunStatus(_ context.Context, id uuid.UUID, status domain.LPRunStatus, errMsg *string) error {
|
||||
func (r *stubLPRepo) SetRunStatus(ctx context.Context, id uuid.UUID, status domain.LPRunStatus, errMsg *string) error {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
r.statusWrites = append(r.statusWrites, lpStatusWrite{status: status, ctxErr: ctx.Err()})
|
||||
run, ok := r.runs[id]
|
||||
if !ok {
|
||||
return domain.ErrNotFound
|
||||
@@ -219,9 +271,10 @@ func (r *stubLPRepo) SetNodeStarted(_ context.Context, runID uuid.UUID, code, fi
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *stubLPRepo) SetNodeFinished(_ context.Context, runID uuid.UUID, code string, status domain.LPNodeStatus, outputs []domain.LPDatasetRef, errMsg *string) error {
|
||||
func (r *stubLPRepo) SetNodeFinished(ctx context.Context, runID uuid.UUID, code string, status domain.LPNodeStatus, outputs []domain.LPDatasetRef, errMsg *string) error {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
r.nodeWrites = append(r.nodeWrites, lpNodeWrite{code: code, status: status, ctxErr: ctx.Err()})
|
||||
n, err := r.node(runID, code)
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -354,8 +407,11 @@ func (j *stubLPJobs) EnqueueRun(_ context.Context, runID uuid.UUID) error {
|
||||
|
||||
// stubComputeCall records one invocation of the compute stub.
|
||||
type stubComputeCall struct {
|
||||
node string
|
||||
inputs map[string]string
|
||||
node string
|
||||
// workers is the per-node worker cap the service passed, or -1 when the
|
||||
// call came through the legacy worker-agnostic ComputeFunc.
|
||||
workers int
|
||||
inputs map[string]string
|
||||
// bodies holds each input file's content at call time; seedDataset stores
|
||||
// each object under its own storage key as its body, so a test can tell
|
||||
// which dataset a node actually downloaded.
|
||||
@@ -369,11 +425,47 @@ type stubCompute struct {
|
||||
mu sync.Mutex
|
||||
calls []stubComputeCall
|
||||
errs map[string]error
|
||||
// waitForCancel names nodes whose compute blocks until its context is
|
||||
// cancelled (or the deadline below elapses), so a test can observe whether
|
||||
// a sibling's failure cancelled them.
|
||||
waitForCancel map[string]bool
|
||||
sawCancel map[string]bool
|
||||
}
|
||||
|
||||
func newStubCompute() *stubCompute { return &stubCompute{errs: map[string]error{}} }
|
||||
func newStubCompute() *stubCompute {
|
||||
return &stubCompute{errs: map[string]error{}, waitForCancel: map[string]bool{}, sawCancel: map[string]bool{}}
|
||||
}
|
||||
|
||||
// fn is a legacy service.ComputeFunc: it sees no worker cap.
|
||||
func (c *stubCompute) fn(ctx context.Context, rio raster.IO, node landscape.Node, inputs map[string]string, outDir string, p landscape.Params) (map[string]string, error) {
|
||||
return c.fnWithWorkers(ctx, rio, node, inputs, outDir, p, -1)
|
||||
}
|
||||
|
||||
// observedCancel reports whether the node's compute saw a cancelled context.
|
||||
func (c *stubCompute) observedCancel(code string) bool {
|
||||
c.mu.Lock()
|
||||
defer c.mu.Unlock()
|
||||
return c.sawCancel[code]
|
||||
}
|
||||
|
||||
func (c *stubCompute) fnWithWorkers(ctx context.Context, _ raster.IO, node landscape.Node, inputs map[string]string, outDir string, p landscape.Params, workers int) (map[string]string, error) {
|
||||
c.mu.Lock()
|
||||
wait := c.waitForCancel[node.Code]
|
||||
c.mu.Unlock()
|
||||
if wait {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
case <-time.After(5 * time.Second):
|
||||
}
|
||||
c.mu.Lock()
|
||||
c.sawCancel[node.Code] = ctx.Err() != nil
|
||||
c.mu.Unlock()
|
||||
}
|
||||
// The real Compute honours cancellation, so the stub must too.
|
||||
if err := ctx.Err(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
func (c *stubCompute) fn(_ context.Context, _ raster.IO, node landscape.Node, inputs map[string]string, outDir string, p landscape.Params) (map[string]string, error) {
|
||||
in := map[string]string{}
|
||||
bodies := map[string]string{}
|
||||
for k, v := range inputs {
|
||||
@@ -386,7 +478,7 @@ func (c *stubCompute) fn(_ context.Context, _ raster.IO, node landscape.Node, in
|
||||
}
|
||||
|
||||
c.mu.Lock()
|
||||
c.calls = append(c.calls, stubComputeCall{node: node.Code, inputs: in, bodies: bodies, params: p})
|
||||
c.calls = append(c.calls, stubComputeCall{node: node.Code, workers: workers, inputs: in, bodies: bodies, params: p})
|
||||
err := c.errs[node.Code]
|
||||
c.mu.Unlock()
|
||||
if err != nil {
|
||||
@@ -453,6 +545,12 @@ type lpFixture struct {
|
||||
}
|
||||
|
||||
func newLPFixture(t *testing.T) *lpFixture {
|
||||
t.Helper()
|
||||
return newLPFixtureWith(t, LandscapeOptions{})
|
||||
}
|
||||
|
||||
// newLPFixtureWith builds the fixture with explicit parallelism options.
|
||||
func newLPFixtureWith(t *testing.T, opts LandscapeOptions) *lpFixture {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
|
||||
@@ -482,7 +580,11 @@ func newLPFixture(t *testing.T) *lpFixture {
|
||||
comp := newStubCompute()
|
||||
log := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
|
||||
svc := NewLandscapeService(runs, datasets, groups, cats, pub, store, jobs, nil, comp.fn, log)
|
||||
svc := NewLandscapeService(runs, datasets, groups, cats, pub, store, jobs, nil, nil, log, opts)
|
||||
// Substitute the workers-aware stub so tests can see the per-node cap the
|
||||
// service computes; NewLandscapeService's legacy-ComputeFunc wrapping is
|
||||
// covered separately.
|
||||
svc.computeWithWorkers = comp.fnWithWorkers
|
||||
return &lpFixture{
|
||||
svc: svc, runs: runs, datasets: datasets, groups: groups, cats: cats,
|
||||
pub: pub, store: store, jobs: jobs, compute: comp, category: cat,
|
||||
@@ -1216,3 +1318,203 @@ func TestLandscapeService_Execute_OverridePinsInputOverFresherDataset(t *testing
|
||||
t.Fatalf("node 01 input for %s = %v, want the freshest %v", landscape.SlotUVeg, got, newest.ID)
|
||||
}
|
||||
}
|
||||
|
||||
// F2: a redelivered message for a finished run must be acked without rerunning.
|
||||
func TestLandscapeService_Execute_IgnoresTerminalRun(t *testing.T) {
|
||||
f := newLPFixture(t)
|
||||
ctx := context.Background()
|
||||
f.seedAllSlots(t)
|
||||
|
||||
run, err := f.svc.CreateRun(ctx, RunInput{})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRun: %v", err)
|
||||
}
|
||||
if err := f.runs.SetRunStatus(ctx, run.ID, domain.LPRunSucceeded, nil); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
f.runs.mu.Lock()
|
||||
f.runs.statusWrites = nil
|
||||
f.runs.mu.Unlock()
|
||||
|
||||
if err := f.svc.Execute(ctx, run.ID); err != nil {
|
||||
t.Fatalf("Execute on a terminal run must ack, got %v", err)
|
||||
}
|
||||
if n := f.compute.count(); n != 0 {
|
||||
t.Fatalf("compute called %d times for a terminal run", n)
|
||||
}
|
||||
if got := f.runs.statusesWritten(); len(got) != 0 {
|
||||
t.Fatalf("no status write expected, got %v", got)
|
||||
}
|
||||
got, err := f.svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.Status != domain.LPRunSucceeded {
|
||||
t.Fatalf("run status changed to %s", got.Status)
|
||||
}
|
||||
}
|
||||
|
||||
// F2: a "running" run is still executed, so a worker that crashed mid-run
|
||||
// recovers on redelivery.
|
||||
func TestLandscapeService_Execute_RerunsRunningRun(t *testing.T) {
|
||||
f := newLPFixture(t)
|
||||
ctx := context.Background()
|
||||
f.seedAllSlots(t)
|
||||
|
||||
run, err := f.svc.CreateRun(ctx, RunInput{})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRun: %v", err)
|
||||
}
|
||||
if err := f.runs.SetRunStatus(ctx, run.ID, domain.LPRunRunning, nil); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := f.svc.Execute(ctx, run.ID); err != nil {
|
||||
t.Fatalf("Execute: %v", err)
|
||||
}
|
||||
if f.compute.count() == 0 {
|
||||
t.Fatal("a running run must be executed on redelivery")
|
||||
}
|
||||
}
|
||||
|
||||
// F4: terminal status writes must land even when the caller's context is dead.
|
||||
func TestLandscapeService_Execute_TerminalWritesSurviveCancelledContext(t *testing.T) {
|
||||
f := newLPFixture(t)
|
||||
f.seedAllSlots(t)
|
||||
|
||||
run, err := f.svc.CreateRun(context.Background(), RunInput{})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRun: %v", err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
if err := f.svc.Execute(ctx, run.ID); err == nil {
|
||||
t.Fatal("expected Execute to fail on a cancelled context")
|
||||
}
|
||||
|
||||
got, err := f.svc.GetRun(context.Background(), run.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got.Status != domain.LPRunFailed {
|
||||
t.Fatalf("run status = %s, want failed", got.Status)
|
||||
}
|
||||
w, ok := f.runs.statusWrite(domain.LPRunFailed)
|
||||
if !ok {
|
||||
t.Fatal("no failed status write recorded")
|
||||
}
|
||||
if w.ctxErr != nil {
|
||||
t.Fatalf("terminal run status write got a cancelled context: %v", w.ctxErr)
|
||||
}
|
||||
nw, ok := f.runs.nodeWrite(domain.LPNodeFailed)
|
||||
if !ok {
|
||||
t.Fatal("no failed node write recorded")
|
||||
}
|
||||
if nw.ctxErr != nil {
|
||||
t.Fatalf("terminal node write got a cancelled context: %v", nw.ctxErr)
|
||||
}
|
||||
}
|
||||
|
||||
// F4: MarkRunFailed is the consumer's last resort and must not need a live ctx.
|
||||
func TestLandscapeService_MarkRunFailed_IgnoresCancelledContext(t *testing.T) {
|
||||
f := newLPFixture(t)
|
||||
run, err := f.svc.CreateRun(context.Background(), RunInput{})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRun: %v", err)
|
||||
}
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
if err := f.svc.MarkRunFailed(ctx, run.ID, "boom"); err != nil {
|
||||
t.Fatalf("MarkRunFailed: %v", err)
|
||||
}
|
||||
w, ok := f.runs.statusWrite(domain.LPRunFailed)
|
||||
if !ok {
|
||||
t.Fatal("no failed status write recorded")
|
||||
}
|
||||
if w.ctxErr != nil {
|
||||
t.Fatalf("MarkRunFailed wrote on a cancelled context: %v", w.ctxErr)
|
||||
}
|
||||
}
|
||||
|
||||
// F5: the per-node worker cap handed to Compute is the run's budget split over
|
||||
// the concurrently running nodes.
|
||||
func TestLandscapeService_Execute_SplitsWorkerBudgetOverNodes(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
opts LandscapeOptions
|
||||
want int
|
||||
}{
|
||||
{"defaults", LandscapeOptions{}, DefaultMaxWorkers / DefaultMaxConcurrentNodes},
|
||||
{"explicit", LandscapeOptions{MaxWorkers: 12, MaxConcurrentNodes: 3}, 4},
|
||||
{"single node keeps the whole budget", LandscapeOptions{MaxWorkers: 6, MaxConcurrentNodes: 1}, 6},
|
||||
{"never below one", LandscapeOptions{MaxWorkers: 1, MaxConcurrentNodes: 4}, 1},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
f := newLPFixtureWith(t, c.opts)
|
||||
f.seedAllSlots(t)
|
||||
got := f.runOnce(t, RunInput{})
|
||||
if got.Status != domain.LPRunSucceeded {
|
||||
t.Fatalf("run status = %s: %v", got.Status, got.Error)
|
||||
}
|
||||
f.compute.mu.Lock()
|
||||
defer f.compute.mu.Unlock()
|
||||
if len(f.compute.calls) == 0 {
|
||||
t.Fatal("compute was never called")
|
||||
}
|
||||
for _, call := range f.compute.calls {
|
||||
if call.workers != c.want {
|
||||
t.Fatalf("node %s got workers=%d, want %d", call.node, call.workers, c.want)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// F6: a failing node cancels its siblings instead of letting them run on.
|
||||
func TestLandscapeService_Execute_FailingNodeCancelsSiblings(t *testing.T) {
|
||||
// Four stage-0 nodes, all in flight at once; "00" fails immediately while
|
||||
// "01" blocks waiting for its context.
|
||||
f := newLPFixtureWith(t, LandscapeOptions{MaxWorkers: 8, MaxConcurrentNodes: 4})
|
||||
f.seedAllSlots(t)
|
||||
f.compute.errs["00"] = errors.New("boom")
|
||||
f.compute.waitForCancel["01"] = true
|
||||
|
||||
got := f.runOnce(t, RunInput{})
|
||||
if got.Status != domain.LPRunFailed {
|
||||
t.Fatalf("run status = %s, want failed", got.Status)
|
||||
}
|
||||
if !f.compute.observedCancel("01") {
|
||||
t.Fatal("sibling node 01 did not observe the cancellation of its context")
|
||||
}
|
||||
for _, n := range got.Nodes {
|
||||
if n.NodeCode == "01" && n.Status == domain.LPNodeSucceeded {
|
||||
t.Fatal("cancelled sibling must not be recorded as succeeded")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// F5: a legacy ComputeFunc still wires up through NewLandscapeService.
|
||||
func TestNewLandscapeService_WrapsLegacyComputeFunc(t *testing.T) {
|
||||
f := newLPFixture(t)
|
||||
f.seedAllSlots(t)
|
||||
// Rebuild the service the production-adjacent way: a legacy ComputeFunc,
|
||||
// no direct field substitution.
|
||||
log := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
svc := NewLandscapeService(f.runs, f.datasets, f.groups, f.cats, f.pub, f.store, f.jobs, nil, f.compute.fn, log, LandscapeOptions{})
|
||||
|
||||
run, err := svc.CreateRun(context.Background(), RunInput{})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateRun: %v", err)
|
||||
}
|
||||
if err := svc.Execute(context.Background(), run.ID); err != nil {
|
||||
t.Fatalf("Execute: %v", err)
|
||||
}
|
||||
call, ok := f.compute.callFor("00")
|
||||
if !ok {
|
||||
t.Fatal("legacy ComputeFunc was never called")
|
||||
}
|
||||
if call.workers != -1 {
|
||||
t.Fatalf("legacy ComputeFunc should not see a worker cap, got %d", call.workers)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user