diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ad22749..9ee7df7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -226,7 +226,7 @@ automatic** (D-021). method, minutes_total or difficulty a member saved while the dinner was `pending` is kept; grounding fills only those left empty. At boot, after the job runner fails the jobs a previous process left running, every - dinner still `pending` with no `ground_dinner` job for its revision + unarchived dinner still `pending` with no `ground_dinner` job for its revision queued or running is set `failed` with `interrupted`, so no dinner stays pending with nothing left to ground it. `GET /api/v1/dinners/{id}/recipe-candidates` diff --git a/internal/db/dinners.sql.go b/internal/db/dinners.sql.go index 37c723a..ff1200e 100644 --- a/internal/db/dinners.sql.go +++ b/internal/db/dinners.sql.go @@ -150,6 +150,7 @@ SET grounding_status = 'failed', grounding_error = ?1, updated_at = ?2 WHERE grounding_status = 'pending' + AND status <> 'archived' AND NOT EXISTS ( SELECT 1 FROM jobs WHERE jobs.kind = ?3 @@ -165,10 +166,11 @@ type FailOrphanedPendingDinnerGroundingsParams struct { Kind string } -// Fails every dinner still pending whose grounding job for its current -// grounding_revision is no longer queued or running: the process stopped -// before the job, or before its handler recorded a result. Run at boot, -// after the job runner fails the jobs a previous process left running. +// Fails every unarchived dinner still pending whose grounding job for its +// current grounding_revision is no longer queued or running: the process +// stopped before the job, or before its handler recorded a result. An +// archived dinner is left as it is; archiving stopped its grounding. Run at +// boot, after the job runner fails the jobs a previous process left running. func (q *Queries) FailOrphanedPendingDinnerGroundings(ctx context.Context, arg FailOrphanedPendingDinnerGroundingsParams) (int64, error) { result, err := q.db.ExecContext(ctx, failOrphanedPendingDinnerGroundings, arg.GroundingError, arg.UpdatedAt, arg.Kind) if err != nil { diff --git a/internal/db/querier.go b/internal/db/querier.go index c75f774..382e233 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -36,10 +36,11 @@ type Querier interface { DeleteRecipeIngredients(ctx context.Context, recipeID string) error DeleteRecipeSteps(ctx context.Context, recipeID string) error FailJob(ctx context.Context, arg FailJobParams) (int64, error) - // Fails every dinner still pending whose grounding job for its current - // grounding_revision is no longer queued or running: the process stopped - // before the job, or before its handler recorded a result. Run at boot, - // after the job runner fails the jobs a previous process left running. + // Fails every unarchived dinner still pending whose grounding job for its + // current grounding_revision is no longer queued or running: the process + // stopped before the job, or before its handler recorded a result. An + // archived dinner is left as it is; archiving stopped its grounding. Run at + // boot, after the job runner fails the jobs a previous process left running. FailOrphanedPendingDinnerGroundings(ctx context.Context, arg FailOrphanedPendingDinnerGroundingsParams) (int64, error) // Records why the background grounding of an unarchived dinner still // pending at the grounding_revision it was grounded for failed; a dinner diff --git a/internal/dinners/grounding.go b/internal/dinners/grounding.go index a57bb71..9fd4115 100644 --- a/internal/dinners/grounding.go +++ b/internal/dinners/grounding.go @@ -227,10 +227,11 @@ func (s *Service) stopGroundings(dinnerID string, below int64) { } } -// FailInterruptedGroundings fails, with ReasonInterrupted, every dinner -// still pending whose KindGroundDinner job is no longer queued or running: -// a previous process stopped before enqueuing the job or before its -// handler recorded a result. Call it once at boot, after the job runner's +// FailInterruptedGroundings fails, with ReasonInterrupted, every unarchived +// dinner still pending whose KindGroundDinner job is no longer queued or +// running: a previous process stopped before enqueuing the job or before +// its handler recorded a result. An archived dinner is left pending, since +// archiving, not a restart, stopped its grounding. Call it once at boot, after the job runner's // Start has failed the jobs that process left running, so no dinner stays // pending with nothing left to ground it. It returns how many it failed. func (s *Service) FailInterruptedGroundings(ctx context.Context) (int64, error) { diff --git a/internal/dinners/grounding_test.go b/internal/dinners/grounding_test.go index ba90e3e..2e14546 100644 --- a/internal/dinners/grounding_test.go +++ b/internal/dinners/grounding_test.go @@ -946,12 +946,64 @@ func TestRestartFailsInterruptedGrounding(t *testing.T) { } } +// TestRestartKeepsArchivedPendingDinner: a pending Regular archived while +// its job runs stays pending, its job succeeded; the boot cleanup leaves it +// as it is rather than failing it as interrupted. +func TestRestartKeepsArchivedPendingDinner(t *testing.T) { + t.Parallel() + e := newEnv(t) + e.search.Gate = make(chan struct{}) + c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry")}, member) + if err != nil { + t.Fatal(err) + } + e.waitQuery(t, "Chicken curry") + if _, err := e.svc.Archive(t.Context(), c.ID, member); err != nil { + t.Fatal(err) + } + job := e.waitJob(t, *c.GroundingJobID) + close(e.search.Gate) + if job.Status != jobs.StatusSucceeded { + t.Fatalf("job = %+v, want succeeded", job) + } + + // The database is now as the next boot finds it. + n, err := e.svc.FailInterruptedGroundings(t.Context()) + if err != nil { + t.Fatal(err) + } + if n != 0 { + t.Errorf("failed %d dinners, want 0", n) + } + if row := e.dinner(t, c.ID); row.Status != dinners.StatusArchived || + row.GroundingStatus != dinners.GroundingPending || row.GroundingError.Valid { + t.Errorf("dinner = %s, %s (%s), want archived and pending with no error", + row.Status, row.GroundingStatus, row.GroundingError.String) + } +} + // TestUpgradeKeepsLegacyQueuedGrounding: a pending Regular whose // ground_dinner job was queued before migration 0010, so its payload has // no revision, is not failed as interrupted at the first boot after the -// upgrade; its job still grounds it. +// upgrade; its job still grounds it. The upgrade starts either from 0009 +// or from a database that has already applied 0010. func TestUpgradeKeepsLegacyQueuedGrounding(t *testing.T) { t.Parallel() + for name, below := range map[string]string{ + "from 0009": "0010", + "from 0010": "0011", + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + upgradeWithLegacyQueuedGrounding(t, below) + }) + } +} + +// upgradeWithLegacyQueuedGrounding migrates a database to every migration +// named before below, adds a pending Regular with a queued ground_dinner +// job whose payload has no revision, then boots on the latest migrations. +func upgradeWithLegacyQueuedGrounding(t *testing.T, below string) { ctx := t.Context() dataDir := t.TempDir() st := openStore(t, dataDir) @@ -961,7 +1013,7 @@ func TestUpgradeKeepsLegacyQueuedGrounding(t *testing.T) { t.Fatal(err) } for _, name := range files { - if name >= "0010" { + if name >= below { continue } body, err := fs.ReadFile(migrations.FS, name) diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index 75b6491..94b8ec3 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -175,8 +175,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision", got) } } @@ -196,8 +196,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision", got) } if !tableExists(t, s, "dinners") { t.Error("table from 0001_init was not created") diff --git a/migrations/0010_dinner_grounding_revision.sql b/migrations/0010_dinner_grounding_revision.sql index c2d4fc1..5f13bd9 100644 --- a/migrations/0010_dinner_grounding_revision.sql +++ b/migrations/0010_dinner_grounding_revision.sql @@ -5,12 +5,3 @@ -- earlier job — queued or running — can no longer write the dinner. ALTER TABLE dinners ADD COLUMN grounding_revision INTEGER NOT NULL DEFAULT 0; - --- Every dinner starts at revision 0, so a ground_dinner job enqueued --- before this migration, whose payload has only dinner_id, is for revision --- 0. Recording it lets the boot cleanup, which matches a pending dinner to --- its job by revision, see a job still queued for its dinner. -UPDATE jobs -SET payload = json_set(payload, '$.revision', 0) -WHERE kind = 'ground_dinner' - AND json_type(payload, '$.revision') IS NULL; diff --git a/migrations/0011_ground_dinner_job_revision.sql b/migrations/0011_ground_dinner_job_revision.sql new file mode 100644 index 0000000..dd5de37 --- /dev/null +++ b/migrations/0011_ground_dinner_job_revision.sql @@ -0,0 +1,11 @@ +-- 0011_ground_dinner_job_revision: every dinner starts at grounding +-- revision 0 (0010), so a ground_dinner job enqueued before 0010, whose +-- payload has only dinner_id, is for revision 0. Recording it lets the boot +-- cleanup, which matches a pending dinner to its job by revision, see a job +-- still queued for its dinner. This is a migration of its own, not part of +-- 0010, so it also runs on a database that has already applied 0010. + +UPDATE jobs +SET payload = json_set(payload, '$.revision', 0) +WHERE kind = 'ground_dinner' + AND json_type(payload, '$.revision') IS NULL; diff --git a/migrations/queries/dinners.sql b/migrations/queries/dinners.sql index 90333a9..e11dec1 100644 --- a/migrations/queries/dinners.sql +++ b/migrations/queries/dinners.sql @@ -134,15 +134,17 @@ WHERE id = ? AND grounding_revision = ?; -- name: FailOrphanedPendingDinnerGroundings :execrows --- Fails every dinner still pending whose grounding job for its current --- grounding_revision is no longer queued or running: the process stopped --- before the job, or before its handler recorded a result. Run at boot, --- after the job runner fails the jobs a previous process left running. +-- Fails every unarchived dinner still pending whose grounding job for its +-- current grounding_revision is no longer queued or running: the process +-- stopped before the job, or before its handler recorded a result. An +-- archived dinner is left as it is; archiving stopped its grounding. Run at +-- boot, after the job runner fails the jobs a previous process left running. UPDATE dinners SET grounding_status = 'failed', grounding_error = sqlc.arg(grounding_error), updated_at = sqlc.arg(updated_at) WHERE grounding_status = 'pending' + AND status <> 'archived' AND NOT EXISTS ( SELECT 1 FROM jobs WHERE jobs.kind = sqlc.arg(kind)