Skip to content

fix: make lifecycle job teardown atomic - #204

Draft
NikolayS wants to merge 1 commit into
NikolayS:mainfrom
samo-agent:agent/fix-203-lifecycle-atomicity
Draft

fix: make lifecycle job teardown atomic#204
NikolayS wants to merge 1 commit into
NikolayS:mainfrom
samo-agent:agent/fix-203-lifecycle-atomicity

Conversation

@NikolayS

Copy link
Copy Markdown
Owner

Fixes #203.

Contract decision

Lifecycle functions now raise and roll back when they cannot complete managed pg_cron work. A warning or partial-success row would still be unsafe because rebuild_partitions() and uninstall() consume ash.stop() internally; an uncaught error gives all four entry points one honest, statement-atomic contract.

  • ash.stop() pre-resolves genuine absence, unschedules exact job IDs, requires cron.unschedule() to return true, and emits a removed row only after proven deletion (with the real job ID).
  • Ordinary unschedule errors preserve their original SQLSTATE and name the job/jobid. Cancellation still propagates naturally.
  • ash.rebuild_partitions() and ash.uninstall() inherit that error, so config changes, earlier unschedules, DDL/data changes, and schema removal roll back together.
  • ash.start() applies the same exact-ID/true-result rule to rollup replacement. Existing sampler/rotation jobs have schedule, command, target database, and active state repaired.
  • Lifecycle calls must run as the ash schema owner. This closes the cross-superuser variant where role B bypasses owner-only ACLs, misses role A's jobs, and drops the schema. Visible foreign-owned managed jobs targeting the installation database make teardown fail closed with the job and owner named.
  • Genuinely absent jobs remain idempotent.

The catalog comments, README, release notes, and the rollup blueprint now describe this contract; the blueprint no longer prescribes EXCEPTION WHEN OTHERS THEN NULL.

RED

Captured before the fix with the issue's locked-row reproduction:

ERROR:  issue #203: lifecycle functions reported success while ash_rollup_1m could not be unscheduled: ash.stop(), ash.rebuild_partitions(), ash.uninstall()

Artifact: /tmp/pgash-203-red-TunI9x/results.tsv (log line 2298 in /tmp/pgash-203-red-TunI9x/17_fresh-install.log).

GREEN

The exact deterministic workflow assertion passed on every supported surface major, including the row-lock rollback, real-ID/idempotency checks, inactive/drifted-job repair, cross-superuser rejection, and foreign-owned-job fail-closed probe:

PostgreSQL #203 assertion
14 PASS
15 PASS
16 PASS
17 PASS
18 PASS
19beta2 PASS

Complete PG17 release gate:

Surface Result
fresh-install PASS
upgrade-chain PASS
features PASS
degraded-no-cron PASS
degraded-no-pgss PASS
degraded-neither PASS
cron-path PASS

Machine-readable TSV: /tmp/pgash-203-owner-all-D4Izt0/results.tsv.

Also green: test_release_stamp.py (17), test_ci_step_script.py (7), extracted-step bash -n, git diff --check, and overlay discovery (2.0-beta1, devel/sql/ash-install.sql).

Further lifecycle-result sites found

These remain intentionally deferred; they are separate operational-result or frozen-migration contracts, but they share the same family of misleading SQL success and should be tracked:

  1. sql/migrations/ash-1.1-to-1.2.sql:21-30 catches every sampler-command update error and does nothing. A lock/permission failure can leave the old command while the migration continues.
  2. sql/migrations/ash-1.2-to-1.3.sql:2612-2630 repeats that pattern around cron.alter_job(). Both are frozen legacy migration legs, so this PR does not rewrite them casually.
  3. devel/sql/ash-install.sql:1590-1652,1680-1682: ash.rotate() returns failed: ... text for rollup contention/failure/incompleteness and truncate lock timeout, so pg_cron records SQL success (the known B2 result-contract family).
  4. devel/sql/ash-install.sql:3541-3546,3561-3566: ash.rollup_minute() returns 0 both when caught up and when rollup/sampler locks are busy; callers cannot distinguish no work from no progress.
  5. devel/sql/ash-install.sql:3905-3910: ash.rollup_cleanup() returns a skipped string on lock contention; a direct caller sees it, but pg_cron still records SQL success.
  6. Without pg_cron, ash.stop() returns an external-scheduler reminder, but ash.uninstall() consumes that row and drops the schema. It truthfully counts zero pg_cron removals, but the reminder is lost; external scheduler ownership needs its own policy.

Landing coordination

This branch is rebased on current origin/main (87c22a3) and uses the post-#179 development-overlay policy: released sql/ash-install.sql and frozen migrations are unchanged; the complete next payload is devel/sql/ash-install.sql.

PR #201 also adds that complete overlay and already contains the same overlay-aware config-ordinal chain split, so landing #201 first will create an add/add overlap. Rebase this PR after #201 and apply the lifecycle delta on top of #201's copy-reviewed development installer; keep #201's wording where unrelated.

PR #200 should then promote the composed #201 + #203 development payload while retaining #200/#202's transaction-local installer wrapper and #200's release identity/RELEASE_NOTES organization. Port the #203 lifecycle note deliberately rather than letting either full-file version overwrite the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ash.stop() suppresses rollup unschedule failures, leaving jobs after rebuild/uninstall

2 participants