[olaberglund]

What

Fixes an active_chain row leak when a chain cannot start a transaction or load its tasks. Each such early return from executeChain left one timetable.active_chain row behind. After max_instances of them the chain was never scheduled again until restart or reconnect.

Seen on pg_timetable 7.0.0 with a chain scheduled every second under query_wait_timeout.

How

• The row is inserted and removed in the same place. chainWorker and intervalChainWorker call removeChainRunStatus right after executeChain returns, before Unlock and before any RepeatAfter reschedule. No exit path inside executeChain can leak the row any more, and the slot now also covers the on_error handler.
• All cleanup (rollback, commit, row removal, on_error, self-destruct) runs on log.DetachedContext: detached from cancellation so notify_chain_stop() cannot skip it, and bounded by a 30s log.DetachedTimeout so a dead database cannot block a worker goroutine forever.
• A chain that fails before any task runs now counts as failed: span error status, RecordChainFailed and the on_error handler fire on every failure path. The ctx.Err() guard in executeOnErrorHandler is gone since it always receives the detached context now.
• StartTransaction rolls back its own transaction when the vxid lookup fails and returns a nil tx, instead of handing a live transaction back together with an error.

Tests

• TestExecuteChainEarlyReturn covers Begin failing, Begin failing under a cancelled context, the vxid lookup failing, and GetChainElements failing. Each case asserts rollback where a transaction exists and that on_error runs.
• TestChainWorkerRemovesRunStatusOnEarlyReturn proves the worker deletes the row after an early return.
• TestStartTransaction covers the vxid failure rollback.
• TestExecuteChainCancelledContextCleansUp now drives the real chainWorker against a Postgres container instead of simulating it by hand.

Investigation and implementation were aided by an LLM (Claude). Targets v7.2.0.

[Copilot]

🟡 Changes recommended

Roll back partially started transactions before removing run status to avoid leaking pool connections.

1 open finding

• Roll back transaction when virtualxid query fails · New

What changed in this PR

Fixes leaked active_chain rows when chain startup or task loading fails.

Changes:

• Cleans up run status on early-return paths.
• Uses cancellation-independent cleanup contexts.
• Adds tests for failure scenarios.

File	Summary
internal/​scheduler/​chain.go	Adds cleanup for early chain failures.
internal/​scheduler/​chain_test.go	Tests active-chain cleanup behavior.

🧠 Review effort: Lite

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[coveralls]

Coverage Report for CI Build 38762415067

Coverage increased (+0.4%) to 88.288%

Details

• Coverage increased (+0.4%) from the base build.
• Patch coverage: 2 uncovered changes across 1 file (39 of 41 lines covered, 95.12%).
• No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
internal/log/log.go	2	0	0.0%
Total (5 files)	41	39	95.12%

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	2570
Covered Lines:	2269
Line Coverage:	88.29%
Coverage Strength:	0.9 hits per line

---

💛 - Coveralls

[pashagolub]

@dependabot rebase

[olaberglund]

The 30s DetachedTimeout in the second commit is counted from the start of the chain, not from the start of cleanup @pashagolub.

bctx is created at the top of executeChain, before any task runs, so for chains longer than 30s CommitTransaction(bctx, tx) gets an expired context. pgx doesn't send COMMIT, the transaction rolls back, and the chain is still logged as successful:

[ERROR] [chain:1|long_35s] [sql:commit] [err:timeout: context already done: context deadline exceeded]
[INFO]  [chain:1|long_35s] Chain executed successfully

To reproduce:

create table probe (job text);
select timetable.add_job('long_35s', '* * * * *', $$insert into probe select 'long_35s' from pg_sleep(35)$$);
update timetable.task set autonomous = false;

No rows ever land in probe. Rollback, on_error and self-destruct after 30s hit the same expired context.

Creating the detached context where cleanup starts, on each exit path, should fix it. I can push that if you'd like.

[pashagolub]

Sure! Go ahead! That's you PR after all! :) Thanks for all the help!

[pashagolub]

Summary

Review fixes in a729814 and 259ae7a.

 executeChain
   StartTransaction / GetChainElements / executeTask
-    on failure: chainFailed(bctx: WithoutCancel + 30s)   # rollback, on_error share 30s
+    on failure: chainFailed(l, tx, err, msg)              # logs once, via task logger
+      rollback             (DetachedTimeout)
+      executeOnErrorHandler(ctx)                           # no deadline; skipped if chain was stopped
-  CommitTransaction(bctx: 30s)                            # error only logged
+  tx.Commit(WithoutCancel(ctx))                           # no deadline
+    on error: chainFailed("Chain failed to commit")       # on_error runs, chain not completed
-  DeleteChain(bctx)
+  DeleteChain(DetachedTimeout)                            # self-destruct only after a successful commit

 RemoveChainRunStatus(chain_id, client_name)
-  DELETE every active_chain row of the chain             # frees other instances' max_instances slots
+  DELETE one row: ctid = (SELECT ... LIMIT 1 FOR UPDATE SKIP LOCKED)

pgengine.CommitTransaction is removed; callers use tx.Commit and handle the error. No schema change.

Evidence

• Before: TestSchedulerFunctions/RemoveChainRunStatus removes one instance only: insert 2 runs, remove 1 → expected: 1, actual: 0
  After: passes
• Before: TestExecuteChainCommit/commit failure runs on_error and keeps chain → remaining expectation which was not matched: ExpectedBegin (on_error never ran, DeleteChain ran instead)
  After: passes
• New: TestExecuteChainEarlyReturn/chain stopped before start asserts on_error does not start; TestExecuteChainCommit/commit is not bounded by DetachedTimeout commits after a delay longer than DetachedTimeout; TestChainWorkerRemovesRunStatusOnEarlyReturn no longer sleeps.
• go test ./internal/... passes, including the testcontainers suites.

Merge Danger

Door: two-way

Code-only change, no migration; revert restores the previous behaviour.

Blast Radius: chain execution

A chain stopped by notify_chain_stop() or shutdown no longer runs on_error, including a stop mid-task (master ran it there). A failed COMMIT now runs on_error and keeps self-destruct chains.

[pashagolub]

hey @olaberglund, do these changes look good to you?

[olaberglund]

Hi @pashagolub ! Yes, I agree with your new changes. Especially treating a failed commit as a failure was a good addition.