amadan
host: a run of this boot with its result but still picked is marked done on the next pass (final review M1)
A done CAS that failed past runFinishBound left the run picked with no handle, holding the instance until the sweep's deadline plus an hour. consider now retries runFinishOurs for such a run when its result.json exists; a run with no result is still left for the lost sweep.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+54
−0
internal/host/runs.go+24 −0internal/host/runs_test.go+30 −0
diff --git a/internal/host/runs.go b/internal/host/runs.go| index e98ed4098..cab2cd423 100644 |
| --- a/internal/host/runs.go |
| +++ b/internal/host/runs.go |
| @@ -164,10 +164,34 @@ func (w *runsWorker) consider(ctx context.Context, rec provision.Record) { |
| h.requestCancel() |
| case h == nil && r.BootID != w.boot: |
| w.closeInterrupted(ctx, rec) |
| + case h == nil: |
| + w.finishStranded(ctx, rec) |
| } |
| } |
| } |
| +// finishStranded is the done CAS again for a run this box picked under this |
| +// boot and is no longer executing, whose result landed but whose done CAS |
| +// did not within runFinishBound. Left picked, it would hold the instance |
| +// until the sweep's deadline plus an hour. A run with no result is left |
| +// alone: its result could not be written, and the sweep ends it as lost. |
| +func (w *runsWorker) finishStranded(ctx context.Context, rec provision.Record) { |
| + id := rec.ActiveRun.ID |
| + rctx, cancel := context.WithTimeout(ctx, runReadBound) |
| + defer cancel() |
| + res, err := runs.ReadResult(rctx, w.store, rec.Account, rec.App, id) |
| + if err != nil || res == nil { |
| + return |
| + } |
| + if _, err := provision.SetRecordRun(rctx, w.store, rec.Account, rec.App, rec.Host, runFinishOurs(id, w.box, w.boot)); err != nil { |
| + if !errors.Is(err, provision.ErrRunConflict) && !errors.Is(err, os.ErrNotExist) { |
| + log.Printf("agent: runs: %s %s: done, again: %v", rec.Host, id, err) |
| + } |
| + return |
| + } |
| + log.Printf("agent: runs: %s: run %s had its result but was still picked; marked done", rec.Host, id) |
| +} |
| + |
| // pickup takes a queued run with one CAS from queued to picked. Only the |
| // box whose CAS lands launches; any other outcome (another box, a cancel, |
| // an expiry) leaves the run alone. A request.json that is not the run the |
diff --git a/internal/host/runs_test.go b/internal/host/runs_test.go| index 2bd35d3ec..aa8cce854 100644 |
| --- a/internal/host/runs_test.go |
| +++ b/internal/host/runs_test.go |
| @@ -280,6 +280,36 @@ func TestRunsWorkerLeavesItsOwnRunUnderThisBoot(t *testing.T) { |
| } |
| } |
| +// A run of this boot that is no longer executing and has its result, but is |
| +// still picked (its done CAS failed past the finish bound), is CASed to done |
| +// on the next pass, so it does not hold the instance until the lost sweep. |
| +func TestRunsWorkerFinishesItsOwnStrandedRun(t *testing.T) { |
| + for _, state := range []string{provision.RunPicked, provision.RunCancel} { |
| + s := release.NewDirStore(t.TempDir()) |
| + _, id := queueRun(t, s, time.Now().Add(time.Minute)) |
| + got, err := provision.SetRecordRun(context.Background(), s, "bab", "shop", runHost, provision.RunPick(id, "box-1", "boot-1", time.Now().Add(time.Hour))) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if state == provision.RunCancel { |
| + if got, err = provision.SetRecordRun(context.Background(), s, "bab", "shop", runHost, provision.RunCancelTransition(id)); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if err := runs.WriteResult(context.Background(), s, "bab", "shop", id, runs.Result{Exit: 3, Box: "box-1"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + var l execLog |
| + testWorker(s, runOwnedRoute(runHost), "box-1", "boot-1", &l).consider(context.Background(), *got) |
| + if r := currentRun(t, s); l.count() != 0 || r.State != provision.RunDone { |
| + t.Fatalf("%s: execs %d, run %+v, want done", state, l.count(), r) |
| + } |
| + if res, _ := runs.ReadResult(context.Background(), s, "bab", "shop", id); res == nil || res.Exit != 3 { |
| + t.Fatalf("%s: the box's result changed: %+v", state, res) |
| + } |
| + } |
| +} |
| + |
| // Only a provisioned row for the record's account and app that the |
| // activator manages is this box's to pick for. |
| func TestRunsWorkerPicksOnlyForActivatorManagedRowsOfTheRecord(t *testing.T) { |