diff --git a/AGENTS.md b/AGENTS.md index 2797838..092c3fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -214,8 +214,15 @@ Tables live in the `pgconductor` schema with a `_private_` prefix (`_private_exe **Cancel With Parent** - `cancel_with_parent` on a child row links it to `parent_execution_id` without a waiting edge -- `cancel_execution()` on a pending execution cancels the linked children of every row it failed by calling itself, with that row's reason. A running execution only gets the flag; its settlement cascades -- Settlement cascades from `terminal_failures` (permanent failures, cancellations and failed waiting ancestors) in the same statement: `linked_descendants` follows linked children and, below the first level, the children they wait on. Running ones get the cancelled flag; pending ones fail as cancelled. A row in the same batch whose outcome would keep it alive (retry, sleep, child, signal re-queue) is cancelled instead, so those CTEs exclude `cancelled_descendants` +- `_private_cancel_linked_children()` calls `cancel_execution()` for the linked children of ended executions, each with the reason its parent ended with. Every path that ends an execution calls it after its own writes: `cancel_execution()` for every row it failed, settlement for `terminal_failures` (permanent failures, cancellations and failed waiting ancestors) and `invoke()` for the execution it supersedes. A timed-out `invokeAndWait` (`clearWaitingState`) cancels the awaited child with `cancel_execution()` +- `cancel_execution()` on a pending execution fails it with the executions it waits on and cancels their linked children; a running execution only gets the flag, and its settlement cascades +- The cascade reads the graph with fresh snapshots, so it finds children started or suspended while the caller waited for a lock. Settlement calls it in the main query after the `writes` CTE, which runs every write of the statement to completion, so it sees a linked child settled in the same batch in its new state (a retried, sleeping or suspended child is cancelled; a completed one stays completed) and writes no row the statement writes later. A new write CTE in settlement must be added to `writes` + +**Lock Order** +- An awaited execution is locked before the executions waiting on it (`waiting_on_execution_id` and `result_waiters`): settlement locks its claimed rows, then waiting parents, failed ancestors (child first) and result waiters; `clearWaitingState` cancels the child before it clears the parent +- A parent is locked before its linked children: the cascade runs after the writes of the path that ended the parent +- `cancel_execution()` and `_private_register_event_race()` lock their whole set at once with `_private_lock_executions()`: `for update nowait`, and when a row is busy, it waits for each row alone in a subtransaction it rolls back, then tries again. They never wait while holding part of the set, so its order does not matter. `cancel_execution()` includes the linked subtree of the rows it fails, so its calls for linked children find their locks held +- Queue configuration: registration and `drop_queue()` lock queue rows in name order before tasks and subscriptions **Cron Schedules** - A claimed occurrence inserts its successor before the handler runs, through `_private_schedule_next_occurrence()`: it takes a share lock on the queue row, then inserts only if the occurrence is still claimed and not cancelled @@ -252,9 +259,9 @@ Tables live in the `pgconductor` schema with a `_private_` prefix (`_private_exe **Result Waits** - `{ execution: id }` branches of `ctx.waitForAny()` are `execution_wait` subscriptions with `awaited_execution_id` set and no event key, so race, timeout and cleanup work as for event waits. `ctx.waitForResult()` is a `waitForAny()` with one such branch -- `_private_register_event_race` locks awaited executions before the waiter (the order settlement uses), stores the outcome of a settled one at once, and otherwise appends `{execution_id, queue, step_key}` to the awaited row's `result_waiters` +- `_private_register_event_race` locks awaited executions, all at once, before the waiter (the order settlement uses), stores the outcome of a settled one at once, and otherwise appends `{execution_id, queue, step_key}` to the awaited row's `result_waiters` - Every path that ends an execution (settlement's `resolved_result_waits`, `cancel_execution()`, `invoke()` superseding a running execution, `clearWaitingState`, and removing pending schedule executions) reads `result_waiters` from the locked or deleted row, like `signal_count`, so a wait registered after its snapshot is not missed. It passes the waits with their outcomes to `_private_resolve_result_waits`, which stores `{status: 'resolved', outcome}` under each wait's step key and wakes a waiter suspended at a wait, like dispatch does. A new path that ends or removes executions must do the same; `drop_queue()` does not -- `_private_resolve_result_waits` wakes suspended waiters with its own update, which runs before the data-modifying CTEs of the calling statement. A statement that also updates or deletes a suspended execution must exclude it from the waits it passes (settlement excludes `cancelled_descendants`), or the later update of that row fails or is lost +- `_private_resolve_result_waits` wakes suspended waiters with its own update, which runs before the data-modifying CTEs of the calling statement. A statement that also updates or deletes a suspended execution must exclude it from the waits it passes, or the later update of that row fails or is lost - A race that settles removes its entries from the `result_waiters` of awaited executions still running, so timeouts and lost races do not accumulate entries - The delivery is a plpgsql function called only when a settled execution has waiters: settlement is planned on every call, and each reference to the partitioned executions table adds planning cost per queue - Waiting creates no parent link: a failed or cancelled execution never fails its waiter, and timeouts or cancelling the waiter leave it running. An unknown execution raises at registration diff --git a/docs/content/crafting-tasks/child-invocation.md b/docs/content/crafting-tasks/child-invocation.md index dc6b8d3..70c7f0a 100644 --- a/docs/content/crafting-tasks/child-invocation.md +++ b/docs/content/crafting-tasks/child-invocation.md @@ -61,7 +61,7 @@ const result = await ctx.invokeAndWait( ); ``` -If the child doesn't complete within the timeout, `ctx.invokeAndWait()` throws a timeout error and the child is cancelled. +If the child doesn't complete within the timeout, `ctx.invokeAndWait()` throws a timeout error and the child is cancelled with `Cancelled: parent timed out`, together with the executions it started with `cancelWithParent` and the children it waits on. ## Error Handling @@ -136,7 +136,7 @@ Retrying the invoking attempt never starts a second execution, even if it crashe ### Cancelling with the Parent -Pass `cancelWithParent: true` to cancel the execution together with the caller: when the caller is cancelled or fails permanently, the execution is cancelled with the same reason, whatever state it is in (pending, running or suspended in a sleep, wait or child). This applies transitively to executions it started with `cancelWithParent` and to the children it waits on: +Pass `cancelWithParent: true` to cancel the execution together with the caller: when the caller is cancelled, fails permanently or is [superseded](../task-execution/deduplication.md#duplicate-keys), the execution is cancelled with the same reason, whatever state it is in (pending, running or suspended in a sleep, wait or child). This applies transitively to executions it started with `cancelWithParent` and to the children it waits on: ```typescript const session = conductor.createTask( diff --git a/docs/content/task-execution/deduplication.md b/docs/content/task-execution/deduplication.md index 81eb276..5b54339 100644 --- a/docs/content/task-execution/deduplication.md +++ b/docs/content/task-execution/deduplication.md @@ -29,7 +29,7 @@ What a repeated invoke does depends on the execution that holds the key: | Existing execution | Result | |--------------------|--------| | Waiting to run | Its payload, `run_at`, `priority` and `group` are replaced. The same ID is returned. A sleeping execution resumes right away with the new payload. | -| Running | It is superseded: marked failed with `superseded by reinvoke`, and a new execution is created with the new payload. The running handler is not stopped, but its result is discarded. | +| Running | It is superseded: marked failed with `superseded by reinvoke`, and a new execution is created with the new payload. The running handler is not stopped, but its result is discarded. Executions it started with `cancelWithParent` are cancelled. | | Completed or failed | Nothing runs. Its stored payload is replaced and the same ID is returned. | A key stays taken as long as its execution is stored. Once [retention](../crafting-tasks/retention.md) removes a completed or failed execution, the next invoke with that key creates a new execution. diff --git a/migrations/0000000001_setup.sql b/migrations/0000000001_setup.sql index 12ec6c1..de2ada2 100644 --- a/migrations/0000000001_setup.sql +++ b/migrations/0000000001_setup.sql @@ -749,7 +749,7 @@ declare v_singleton_on timestamptz; v_next_singleton_on timestamptz; v_run_at timestamptz; - v_resolved integer; + v_superseded_id uuid; begin perform pgconductor._private_check_metadata(p_metadata); v_now := pgconductor._private_current_time(); @@ -784,14 +784,25 @@ begin last_error = 'superseded by reinvoke' from superseded s where e.id = s.id and e.queue = s.queue and s.should_remove is not true + ), + resolved as ( + select pgconductor._private_resolve_result_waits(jsonb_agg(w.value || jsonb_build_object( + 'outcome', jsonb_build_object('status', 'failed', 'error', 'superseded by reinvoke') + ))) + from superseded s + cross join lateral jsonb_array_elements(s.result_waiters) w + having count(*) > 0 ) - select pgconductor._private_resolve_result_waits(jsonb_agg(w.value || jsonb_build_object( - 'outcome', jsonb_build_object('status', 'failed', 'error', 'superseded by reinvoke') - ))) - into v_resolved + select s.id + into v_superseded_id from superseded s - cross join lateral jsonb_array_elements(s.result_waiters) w - having count(*) > 0; + left join resolved on true; + + if v_superseded_id is not null then + perform pgconductor._private_cancel_linked_children(jsonb_build_array(jsonb_build_object( + 'execution_id', v_superseded_id, 'reason', 'superseded by reinvoke' + ))); + end if; end if; -- singleton throttle/debounce logic @@ -1070,6 +1081,58 @@ begin end; $function$; +-- locks the executions in p_ids all at once, or waits without holding any of them: waiting for +-- one while holding another can deadlock with a statement that locks them the other way round +create or replace function pgconductor._private_lock_executions(p_ids uuid[]) +returns void +language plpgsql +volatile +set search_path to '' +as $function$ +declare + v_id uuid; +begin + loop + begin + perform 1 + from pgconductor._private_executions e + where e.id = any(p_ids) + for update of e nowait; + return; + exception when lock_not_available then + -- wait for each one alone, rolling back to release it, then try them all again + foreach v_id in array p_ids loop + begin + perform 1 from pgconductor._private_executions e where e.id = v_id for update of e; + raise exception using errcode = 'PGC01'; + exception when sqlstate 'PGC01' then + null; + end; + end loop; + end; + end loop; +end; +$function$; + +-- cancels the linked children of ended executions ([{execution_id, reason}]) with the reason +-- their parent ended with. callers run it after their own writes, and it reads the children +-- with a fresh snapshot, so it also finds the ones started while the caller waited for a lock. +create or replace function pgconductor._private_cancel_linked_children(p_parents jsonb) +returns void +language plpgsql +volatile +set search_path to '' +as $function$ +begin + perform pgconductor.cancel_execution(child.id, parent.reason) + from jsonb_to_recordset(p_parents) as parent(execution_id uuid, reason text) + join pgconductor._private_executions child on child.parent_execution_id = parent.execution_id + where child.cancel_with_parent + and child.completed_at is null + and child.failed_at is null; +end; +$function$; + -- cancel an execution create or replace function pgconductor.cancel_execution( p_execution_id uuid, @@ -1085,21 +1148,21 @@ declare v_queue text; v_descendant_ids uuid[]; v_ancestor_ids uuid[]; + v_linked_ids uuid[]; v_locked_ids uuid[]; v_rows_affected integer; v_resolved integer; begin - -- a waiting execution is cancelled with the executions it waits on, and - -- fails the executions waiting on it. the graph may change while we wait for - -- locks, so it is read again after locking. a changed graph releases the - -- stale locks by rolling back, then locks the new graph children first. + -- a waiting execution is cancelled with the executions it waits on and its + -- linked children, and fails the executions waiting on it. the graph may + -- change while we wait for locks, so it is read again after locking. a + -- changed graph releases the stale locks by rolling back, then locks the new + -- graph. the whole graph is locked at once, so a settlement that locks a + -- child before the parent waiting on it cannot deadlock with this, and the + -- calls for linked children find their locks held. loop begin - perform 1 - from unnest(v_locked_ids) with ordinality as chain(id, position) - join pgconductor._private_executions e on e.id = chain.id - order by chain.position - for update of e; + perform pgconductor._private_lock_executions(v_locked_ids); with recursive descendants as ( select e.waiting_on_execution_id as id, 1 as depth @@ -1125,8 +1188,28 @@ begin ) select array_agg(id order by depth) into v_ancestor_ids from ancestors; - exit when v_locked_ids = v_descendant_ids || p_execution_id || v_ancestor_ids; - v_locked_ids := v_descendant_ids || p_execution_id || v_ancestor_ids; + -- a running execution is only flagged; its settlement cancels its linked children + with recursive linked as ( + select child.id, child.waiting_on_execution_id + from pgconductor._private_executions e + join pgconductor._private_executions child + on child.parent_execution_id = e.id and child.cancel_with_parent + where e.id = any(v_descendant_ids || p_execution_id || v_ancestor_ids) + and (e.id <> p_execution_id or e.locked_by is null) + and child.completed_at is null + and child.failed_at is null + union all + select child.id, child.waiting_on_execution_id + from linked l + join pgconductor._private_executions child + on child.parent_execution_id = l.id + and (child.cancel_with_parent or child.id = l.waiting_on_execution_id) + where child.completed_at is null and child.failed_at is null + ) + select array_agg(id order by id) into v_linked_ids from linked; + + exit when v_locked_ids = v_linked_ids || v_descendant_ids || p_execution_id || v_ancestor_ids; + v_locked_ids := v_linked_ids || v_descendant_ids || p_execution_id || v_ancestor_ids; raise exception using errcode = 'PGC01'; exception when sqlstate 'PGC01' then null; @@ -1221,17 +1304,12 @@ begin cross join lateral jsonb_array_elements(p.result_waiters) w having count(*) > 0; - -- linked children are cancelled with the reason their parent ended with - perform pgconductor.cancel_execution( - e.id, - case when e.parent_execution_id = any(v_ancestor_ids) then 'Child execution failed: ' || p_reason + perform pgconductor._private_cancel_linked_children(jsonb_agg(jsonb_build_object( + 'execution_id', id, + 'reason', case when id = any(v_ancestor_ids) then 'Child execution failed: ' || p_reason else p_reason end - ) - from pgconductor._private_executions e - where e.parent_execution_id = any(v_descendant_ids || p_execution_id || v_ancestor_ids) - and e.cancel_with_parent - and e.completed_at is null - and e.failed_at is null; + ))) + from unnest(v_descendant_ids || p_execution_id || v_ancestor_ids) id; return true; end; @@ -1302,6 +1380,18 @@ create or replace function pgconductor.drop_queue(queue_name text) volatile set search_path to '' as $function$ + -- registration locks a queue before its tasks, so this locks the queue and the queues whose + -- tasks dead-letter into it first, in the same order + select 1 + from pgconductor._private_queues queue + where queue.name = drop_queue.queue_name + or queue.name in ( + select task.queue from pgconductor._private_tasks task + where task.dead_letter_queue = drop_queue.queue_name + ) + order by queue.name + for update; + update pgconductor._private_tasks set dead_letter_queue = null, dead_letter_task_key = null where dead_letter_queue = drop_queue.queue_name; @@ -1709,13 +1799,10 @@ declare v_expires_at timestamptz; v_now timestamptz := pgconductor._private_current_time(); begin - -- Settlement locks an execution before its waiters, so awaited executions are locked first + -- Settlement locks an execution before its waiters, so awaited executions are locked first, + -- all at once: one of them may be waiting on another if cardinality(v_awaited_ids) > 0 then - perform 1 - from pgconductor._private_executions awaited - where awaited.id = any(v_awaited_ids) - order by awaited.id - for update; + perform pgconductor._private_lock_executions(v_awaited_ids); end if; perform 1 diff --git a/packages/pgconductor-js/src/generated/sql.ts b/packages/pgconductor-js/src/generated/sql.ts index 5d687a3..63c7b76 100644 --- a/packages/pgconductor-js/src/generated/sql.ts +++ b/packages/pgconductor-js/src/generated/sql.ts @@ -763,7 +763,7 @@ declare v_singleton_on timestamptz; v_next_singleton_on timestamptz; v_run_at timestamptz; - v_resolved integer; + v_superseded_id uuid; begin perform pgconductor._private_check_metadata(p_metadata); v_now := pgconductor._private_current_time(); @@ -798,14 +798,25 @@ begin last_error = 'superseded by reinvoke' from superseded s where e.id = s.id and e.queue = s.queue and s.should_remove is not true + ), + resolved as ( + select pgconductor._private_resolve_result_waits(jsonb_agg(w.value || jsonb_build_object( + 'outcome', jsonb_build_object('status', 'failed', 'error', 'superseded by reinvoke') + ))) + from superseded s + cross join lateral jsonb_array_elements(s.result_waiters) w + having count(*) > 0 ) - select pgconductor._private_resolve_result_waits(jsonb_agg(w.value || jsonb_build_object( - 'outcome', jsonb_build_object('status', 'failed', 'error', 'superseded by reinvoke') - ))) - into v_resolved + select s.id + into v_superseded_id from superseded s - cross join lateral jsonb_array_elements(s.result_waiters) w - having count(*) > 0; + left join resolved on true; + + if v_superseded_id is not null then + perform pgconductor._private_cancel_linked_children(jsonb_build_array(jsonb_build_object( + 'execution_id', v_superseded_id, 'reason', 'superseded by reinvoke' + ))); + end if; end if; -- singleton throttle/debounce logic @@ -1084,6 +1095,58 @@ begin end; $function$; +-- locks the executions in p_ids all at once, or waits without holding any of them: waiting for +-- one while holding another can deadlock with a statement that locks them the other way round +create or replace function pgconductor._private_lock_executions(p_ids uuid[]) +returns void +language plpgsql +volatile +set search_path to '' +as $function$ +declare + v_id uuid; +begin + loop + begin + perform 1 + from pgconductor._private_executions e + where e.id = any(p_ids) + for update of e nowait; + return; + exception when lock_not_available then + -- wait for each one alone, rolling back to release it, then try them all again + foreach v_id in array p_ids loop + begin + perform 1 from pgconductor._private_executions e where e.id = v_id for update of e; + raise exception using errcode = 'PGC01'; + exception when sqlstate 'PGC01' then + null; + end; + end loop; + end; + end loop; +end; +$function$; + +-- cancels the linked children of ended executions ([{execution_id, reason}]) with the reason +-- their parent ended with. callers run it after their own writes, and it reads the children +-- with a fresh snapshot, so it also finds the ones started while the caller waited for a lock. +create or replace function pgconductor._private_cancel_linked_children(p_parents jsonb) +returns void +language plpgsql +volatile +set search_path to '' +as $function$ +begin + perform pgconductor.cancel_execution(child.id, parent.reason) + from jsonb_to_recordset(p_parents) as parent(execution_id uuid, reason text) + join pgconductor._private_executions child on child.parent_execution_id = parent.execution_id + where child.cancel_with_parent + and child.completed_at is null + and child.failed_at is null; +end; +$function$; + -- cancel an execution create or replace function pgconductor.cancel_execution( p_execution_id uuid, @@ -1099,21 +1162,21 @@ declare v_queue text; v_descendant_ids uuid[]; v_ancestor_ids uuid[]; + v_linked_ids uuid[]; v_locked_ids uuid[]; v_rows_affected integer; v_resolved integer; begin - -- a waiting execution is cancelled with the executions it waits on, and - -- fails the executions waiting on it. the graph may change while we wait for - -- locks, so it is read again after locking. a changed graph releases the - -- stale locks by rolling back, then locks the new graph children first. + -- a waiting execution is cancelled with the executions it waits on and its + -- linked children, and fails the executions waiting on it. the graph may + -- change while we wait for locks, so it is read again after locking. a + -- changed graph releases the stale locks by rolling back, then locks the new + -- graph. the whole graph is locked at once, so a settlement that locks a + -- child before the parent waiting on it cannot deadlock with this, and the + -- calls for linked children find their locks held. loop begin - perform 1 - from unnest(v_locked_ids) with ordinality as chain(id, position) - join pgconductor._private_executions e on e.id = chain.id - order by chain.position - for update of e; + perform pgconductor._private_lock_executions(v_locked_ids); with recursive descendants as ( select e.waiting_on_execution_id as id, 1 as depth @@ -1139,8 +1202,28 @@ begin ) select array_agg(id order by depth) into v_ancestor_ids from ancestors; - exit when v_locked_ids = v_descendant_ids || p_execution_id || v_ancestor_ids; - v_locked_ids := v_descendant_ids || p_execution_id || v_ancestor_ids; + -- a running execution is only flagged; its settlement cancels its linked children + with recursive linked as ( + select child.id, child.waiting_on_execution_id + from pgconductor._private_executions e + join pgconductor._private_executions child + on child.parent_execution_id = e.id and child.cancel_with_parent + where e.id = any(v_descendant_ids || p_execution_id || v_ancestor_ids) + and (e.id <> p_execution_id or e.locked_by is null) + and child.completed_at is null + and child.failed_at is null + union all + select child.id, child.waiting_on_execution_id + from linked l + join pgconductor._private_executions child + on child.parent_execution_id = l.id + and (child.cancel_with_parent or child.id = l.waiting_on_execution_id) + where child.completed_at is null and child.failed_at is null + ) + select array_agg(id order by id) into v_linked_ids from linked; + + exit when v_locked_ids = v_linked_ids || v_descendant_ids || p_execution_id || v_ancestor_ids; + v_locked_ids := v_linked_ids || v_descendant_ids || p_execution_id || v_ancestor_ids; raise exception using errcode = 'PGC01'; exception when sqlstate 'PGC01' then null; @@ -1235,17 +1318,12 @@ begin cross join lateral jsonb_array_elements(p.result_waiters) w having count(*) > 0; - -- linked children are cancelled with the reason their parent ended with - perform pgconductor.cancel_execution( - e.id, - case when e.parent_execution_id = any(v_ancestor_ids) then 'Child execution failed: ' || p_reason + perform pgconductor._private_cancel_linked_children(jsonb_agg(jsonb_build_object( + 'execution_id', id, + 'reason', case when id = any(v_ancestor_ids) then 'Child execution failed: ' || p_reason else p_reason end - ) - from pgconductor._private_executions e - where e.parent_execution_id = any(v_descendant_ids || p_execution_id || v_ancestor_ids) - and e.cancel_with_parent - and e.completed_at is null - and e.failed_at is null; + ))) + from unnest(v_descendant_ids || p_execution_id || v_ancestor_ids) id; return true; end; @@ -1316,6 +1394,18 @@ create or replace function pgconductor.drop_queue(queue_name text) volatile set search_path to '' as $function$ + -- registration locks a queue before its tasks, so this locks the queue and the queues whose + -- tasks dead-letter into it first, in the same order + select 1 + from pgconductor._private_queues queue + where queue.name = drop_queue.queue_name + or queue.name in ( + select task.queue from pgconductor._private_tasks task + where task.dead_letter_queue = drop_queue.queue_name + ) + order by queue.name + for update; + update pgconductor._private_tasks set dead_letter_queue = null, dead_letter_task_key = null where dead_letter_queue = drop_queue.queue_name; @@ -1723,13 +1813,10 @@ declare v_expires_at timestamptz; v_now timestamptz := pgconductor._private_current_time(); begin - -- Settlement locks an execution before its waiters, so awaited executions are locked first + -- Settlement locks an execution before its waiters, so awaited executions are locked first, + -- all at once: one of them may be waiting on another if cardinality(v_awaited_ids) > 0 then - perform 1 - from pgconductor._private_executions awaited - where awaited.id = any(v_awaited_ids) - order by awaited.id - for update; + perform pgconductor._private_lock_executions(v_awaited_ids); end if; perform 1 diff --git a/packages/pgconductor-js/src/query-builder.ts b/packages/pgconductor-js/src/query-builder.ts index bf74ed2..50d5b0c 100644 --- a/packages/pgconductor-js/src/query-builder.ts +++ b/packages/pgconductor-js/src/query-builder.ts @@ -690,7 +690,6 @@ export class QueryBuilder { from now_ts nt, signal_requeued q where e.id = q.execution_id and e.queue = q.queue and e.locked_by = ${orchestratorId}::uuid - and not exists (select 1 from cancelled_descendants d where d.id = e.id) returning e.id )`); ctes.push(this.sql`failed_results as ( @@ -698,12 +697,10 @@ export class QueryBuilder { where status in ('failed', 'permanently_failed') or execution_cancelled )`); ctes.push(this.sql`released_results as ( - select * from valid_results r where status = 'released' and not execution_cancelled - and not exists (select 1 from cancelled_descendants d where d.id = r.execution_id) + select * from valid_results where status = 'released' and not execution_cancelled )`); ctes.push(this.sql`invoke_child_data as ( - select * from valid_results r where status = 'invoke_child' and not execution_cancelled - and not exists (select 1 from cancelled_descendants d where d.id = r.execution_id) + select * from valid_results where status = 'invoke_child' and not execution_cancelled )`); // A completed child may wake only a parent which is still waiting and is not @@ -717,7 +714,6 @@ export class QueryBuilder { and parent.completed_at is null and parent.failed_at is null and parent.locked_by is null - and not exists (select 1 from cancelled_descendants d where d.id = parent.id) for update of parent )`); ctes.push(this.sql`parent_steps_all as ( @@ -854,57 +850,6 @@ export class QueryBuilder { p.parent_metadata from failed_parent_targets p )`); - // Linked children of a terminal failure are cancelled with its error, transitively, with the - // executions they wait on. A row settled by this statement keeps a terminal outcome. - ctes.push(this.sql`linked_descendants as ( - select child.id, child.waiting_on_execution_id, t.failure_error as reason, 1 as depth - from terminal_failures t - join pgconductor._private_executions child - on child.parent_execution_id = t.execution_id and child.cancel_with_parent - where child.completed_at is null and child.failed_at is null - union all - select child.id, child.waiting_on_execution_id, d.reason, d.depth + 1 - from linked_descendants d - join pgconductor._private_executions child - on child.parent_execution_id = d.id - and (child.cancel_with_parent or child.id = d.waiting_on_execution_id) - where child.completed_at is null and child.failed_at is null - )`); - ctes.push(this.sql`cancelled_descendants as materialized ( - select e.id, e.queue, d.reason, e.result_waiters, - e.locked_by is not null and v.execution_id is null as running, - t.remove_on_fail_days = 0 as should_remove - from linked_descendants d - join pgconductor._private_executions e on e.id = d.id - left join pgconductor._private_tasks t on t.key = e.task_key and t.queue = e.queue - left join valid_results v on v.execution_id = e.id - where e.completed_at is null and e.failed_at is null - and not exists (select 1 from completed_results c where c.execution_id = e.id) - and not exists (select 1 from permanently_failed_children p where p.execution_id = e.id) - and not exists (select 1 from locked_ancestors a where a.parent_id = e.id) - order by d.depth - for update of e - )`); - // running ones are aborted by the workers running them on their next poll - ctes.push(this.sql`flagged_descendants as ( - update pgconductor._private_executions e - set cancelled = true, last_error = d.reason - from cancelled_descendants d - where e.id = d.id and e.queue = d.queue and d.running and not e.cancelled - )`); - ctes.push(this.sql`deleted_descendants as ( - delete from pgconductor._private_executions e - using cancelled_descendants d - where e.id = d.id and e.queue = d.queue and not d.running and d.should_remove - )`); - ctes.push(this.sql`updated_descendants as ( - update pgconductor._private_executions e - set failed_at = nt.ts, cancelled = true, last_error = d.reason, - waiting_on_execution_id = null, waiting_step_key = null, - locked_by = null, locked_at = null - from now_ts nt, cancelled_descendants d - where e.id = d.id and e.queue = d.queue and not d.running and d.should_remove is not true - )`); ctes.push(this.sql`dead_lettered as ( insert into pgconductor._private_executions (task_key, queue, payload, run_at, "group", trace_context, metadata, dead_letter) select @@ -925,6 +870,7 @@ export class QueryBuilder { and not p.execution_cancelled on conflict ((dead_letter->>'sourceExecutionId'), queue, task_key) where dead_letter is not null do nothing + returning id )`); ctes.push(this.sql`failed_updates as ( select p.execution_id as target_id, p.queue, p.child_error, true as is_child @@ -968,8 +914,6 @@ export class QueryBuilder { select id, queue from updated_completed union all select id, queue from updated_failed - union all - select id, queue from cancelled_descendants where not running ) terminal where subscription.kind = 'execution_wait' and subscription.execution_id = terminal.id @@ -977,8 +921,7 @@ export class QueryBuilder { returning subscription.id )`); // Waits for a settled execution are read from its locked row, which also holds waits - // registered after this statement's snapshot. Descendants cancelled here are not woken: - // the function's update would precede the statement's own and lose it. + // registered after this statement's snapshot ctes.push(this.sql`resolved_result_waits as ( select pgconductor._private_resolve_result_waits(jsonb_agg(w.value || jsonb_build_object('outcome', s.outcome))) from ( @@ -1001,13 +944,8 @@ export class QueryBuilder { select p.parent_result_waiters, jsonb_build_object('status', 'failed', 'error', 'Child execution failed: ' || p.child_error) from failed_parent_targets p - union all - select d.result_waiters, jsonb_build_object('status', 'cancelled', 'reason', d.reason) - from cancelled_descendants d - where not d.running ) s cross join lateral jsonb_array_elements(s.result_waiters) w - where (w.value->>'execution_id')::uuid not in (select id from cancelled_descendants) having count(*) > 0 )`); ctes.push(this.sql`retried as ( @@ -1023,7 +961,6 @@ export class QueryBuilder { and r.status <> 'permanently_failed' and not r.execution_cancelled and e.attempts < tc.max_attempts - and not exists (select 1 from cancelled_descendants d where d.id = e.id) returning e.id )`); @@ -1032,6 +969,7 @@ export class QueryBuilder { select execution_id, queue, step_key, null::jsonb from released_results where step_key is not null on conflict (execution_id, key) do nothing + returning execution_id )`); ctes.push(this.sql`updated_released as ( update pgconductor._private_executions e @@ -1069,9 +1007,40 @@ export class QueryBuilder { returning e.id )`); + ctes.push(this.sql`writes as ( + select 1 from updated_signal_requeued + union all select 1 from parent_steps_all + union all select 1 from orphaned_children + union all select 1 from updated_parents_all + union all select 1 from deleted_completed + union all select 1 from updated_completed + union all select 1 from dead_lettered + union all select 1 from deleted_failed + union all select 1 from updated_failed + union all select 1 from cleaned_terminal_event_waits + union all select 1 from resolved_result_waits + union all select 1 from retried + union all select 1 from released_steps + union all select 1 from updated_released + union all select 1 from inserted_children + union all select 1 from updated_invoke_parents + )`); + const combined = ctes.reduce((acc, cte, i) => (i === 0 ? cte : this.sql`${acc}, ${cte}`)); - return this.sql<[{ result: number }]>` - with recursive ${combined} select 1 as result from (select count(*) from resolved_result_waits) r + // Linked children of a terminal failure are cancelled with its error. The cascade reads the + // graph with fresh snapshots, so it sees children and waits that changed while this + // statement waited for locks, and it writes rows this statement may write too. It runs + // last: filtering on the row count of every write runs them to completion first (the + // planner drops a count nothing reads). + return this.sql` + with recursive ${combined} + select pgconductor._private_cancel_linked_children(jsonb_agg(jsonb_build_object( + 'execution_id', t.execution_id, 'reason', t.failure_error + ))) + from (select count(*) as writes from writes) w + left join terminal_failures t on true + where w.writes >= 0 + having count(t.execution_id) > 0 `; } buildRemoveExecutions({ @@ -1526,78 +1495,27 @@ export class QueryBuilder { claimToken, }: ClearWaitingStateArgs): PendingQuery> { return this.sql>` - -- Lock the child before the parent, like cancellation and settlement do - with child_info as materialized ( - select c.id as child_id, c.locked_by as child_locked_by - from pgconductor._private_executions p - join pgconductor._private_executions c on c.id = p.waiting_on_execution_id - where p.id = ${executionId}::uuid - and p.queue = ${queue}::text - and p.locked_by = ${orchestratorId}::uuid - and p.claim_token = ${claimToken}::uuid - for update of c - ), claimed_parent as materialized ( - select e.id, e.queue, ci.child_id, ci.child_locked_by + -- The child is cancelled like any other: cancel_execution locks it before this parent + with timed_out as materialized ( + select e.waiting_on_execution_id as child_id from pgconductor._private_executions e - left join child_info ci on ci.child_id = e.waiting_on_execution_id where e.id = ${executionId}::uuid and e.queue = ${queue}::text and e.locked_by = ${orchestratorId}::uuid and e.claim_token = ${claimToken}::uuid - for update of e - ), - -- Fail pending (not locked) children immediately - failed_pending_child as ( - update pgconductor._private_executions e - set - failed_at = pgconductor._private_current_time(), - last_error = 'Cancelled: parent timed out', - locked_by = null, - locked_at = null - from claimed_parent p - where e.id = p.child_id - and p.child_locked_by is null -- not currently executing - and e.completed_at is null - and e.failed_at is null - returning e.id, e.result_waiters - ), - resolved_result_waits as ( - select pgconductor._private_resolve_result_waits(jsonb_agg(w.value || jsonb_build_object( - 'outcome', jsonb_build_object('status', 'failed', 'error', 'Cancelled: parent timed out') - ))) - from failed_pending_child c - cross join lateral jsonb_array_elements(c.result_waiters) w - having count(*) > 0 - ), - -- Signal executing (locked) children to cancel - signaled_executing_child as ( - update pgconductor._private_executions e - set cancelled = true - from claimed_parent p - where e.id = p.child_id - and p.child_locked_by is not null -- currently executing - and e.completed_at is null - and e.failed_at is null - returning e.id - ), - -- Always clear parent's waiting state - cleared_parent as ( - update pgconductor._private_executions e - set - waiting_on_execution_id = null, - waiting_step_key = null - from claimed_parent p - where e.id = p.id - and e.queue = p.queue - returning e.id + and e.waiting_on_execution_id is not null + ), cancelled_child as materialized ( + select pgconductor.cancel_execution(child_id, 'Cancelled: parent timed out') + from timed_out ) - select id from cleared_parent - union all - select id from failed_pending_child - union all - select id from signaled_executing_child - union all - select null from resolved_result_waits + update pgconductor._private_executions e + set waiting_on_execution_id = null, waiting_step_key = null + from (select count(*) from cancelled_child) c + where e.id = ${executionId}::uuid + and e.queue = ${queue}::text + and e.locked_by = ${orchestratorId}::uuid + and e.claim_token = ${claimToken}::uuid + returning e.id `; } diff --git a/packages/pgconductor-js/tests/fixtures/sessions.ts b/packages/pgconductor-js/tests/fixtures/sessions.ts new file mode 100644 index 0000000..2dcfb17 --- /dev/null +++ b/packages/pgconductor-js/tests/fixtures/sessions.ts @@ -0,0 +1,141 @@ +import postgres, { type ReservedSql, type Sql } from "postgres"; +import { Conductor } from "../../src/conductor"; +import type { ExecutionResult } from "../../src/database-client"; +import { QueryBuilder } from "../../src/query-builder"; +import { TaskSchemas } from "../../src/schemas"; +import { defineTask } from "../../src/task-definition"; +import type { TestDatabase } from "./test-database"; +import { waitForCondition } from "../test-utils"; + +// Helpers for tests that interleave real statements of two sessions on executions in a given state + +export const TASKS = ["parent", "linked", "child", "grandchild", "racer"] as const; + +// Installs the schema and registers TASKS on the default queue without starting workers +export async function install(db: TestDatabase, removeOnFailDays: number | null = null) { + const conductor = Conductor.create({ + sql: db.sql, + tasks: TaskSchemas.fromSchema(TASKS.map((name) => defineTask({ name }))), + context: {}, + }); + await conductor.ensureInstalled(); + await db.sql`insert into pgconductor._private_queues (name) values ('default') on conflict do nothing`; + for (const key of TASKS) { + await db.sql` + insert into pgconductor._private_tasks (key, queue, max_attempts, remove_on_fail_days) + values (${key}, 'default', 1, ${removeOnFailDays}) + `; + } +} + +export type Row = { + id?: string; + task_key: string; + locked_by?: string; + claim_token?: string; + parent_execution_id?: string; + cancel_with_parent?: boolean; + waiting_on_execution_id?: string; + waiting_step_key?: string; + dedupe_key?: string; + result_waiters?: { execution_id: string; queue: string; step_key: string }[]; + suspended?: boolean; +}; + +// Inserts an execution in the given state: claimed when locked_by is set, otherwise +// suspended (run_at infinity) when suspended is set, otherwise pending +export async function insertExecution(sql: Sql, row: Row): Promise { + const id = row.id || crypto.randomUUID(); + await sql` + insert into pgconductor._private_executions ( + id, task_key, queue, locked_by, locked_at, claim_token, attempts, run_at, + parent_execution_id, cancel_with_parent, waiting_on_execution_id, waiting_step_key, + dedupe_key, result_waiters + ) values ( + ${id}::uuid, ${row.task_key}, 'default', ${row.locked_by || null}::uuid, + case when ${row.locked_by || null}::uuid is null then null else pgconductor._private_current_time() end, + ${row.claim_token || null}::uuid, + case when ${row.locked_by || null}::uuid is null then 0 else 1 end, + case when ${row.suspended || false}::boolean then 'infinity'::timestamptz + else pgconductor._private_current_time() end, + ${row.parent_execution_id || null}::uuid, ${row.cancel_with_parent || false}, + ${row.waiting_on_execution_id || null}::uuid, ${row.waiting_step_key || null}, + ${row.dedupe_key || null}, + ${row.result_waiters ? sql.json(row.result_waiters) : null}::jsonb + ) + `; + return id; +} + +export type Execution = { + id: string; + task_key: string; + locked_by: string | null; + completed_at: Date | null; + failed_at: Date | null; + cancelled: boolean; + last_error: string | null; + waiting_on_execution_id: string | null; + parent_execution_id: string | null; +}; + +export async function getExecution(sql: Sql, id: string): Promise { + const [row] = await sql` + select id, task_key, locked_by, completed_at, failed_at, cancelled, last_error, + waiting_on_execution_id, parent_execution_id + from pgconductor._private_executions where id = ${id}::uuid + `; + return row; +} + +// "removed", or whether the execution failed and was cancelled +export async function ending(sql: Sql, id: string) { + const execution = await getExecution(sql, id); + return execution + ? { failed: execution.failed_at !== null, cancelled: execution.cancelled } + : "removed"; +} + +// A dedicated backend, so a test controls its transactions and can name it as a lock holder +export type Session = { sql: ReservedSql; pid: number; end: () => Promise }; + +export async function openSession(db: TestDatabase): Promise { + const pool = postgres(db.url, { max: 1, onnotice: () => {} }); + const sql = await pool.reserve(); + const [{ pid }] = await sql<[{ pid: number }]>`select pg_backend_pid() as pid`; + return { + sql, + pid, + end: async () => { + sql.release(); + await pool.end({ timeout: 1 }); + }, + }; +} + +// The settlement statement a worker flushes, run on the given session +export function settle(sql: Sql, orchestratorId: string, results: ExecutionResult[]) { + return new QueryBuilder(sql).buildReturnExecutions({ orchestratorId, results }); +} + +// Barrier: resolves once `waiter` waits for a lock held by `holder` +export async function waitUntilBlocked(observer: Sql, waiter: Session, holder: Session) { + await waitForCondition(async () => { + const [{ blocked }] = await observer<[{ blocked: boolean }]>` + select ${holder.pid}::int = any(pg_blocking_pids(${waiter.pid}::int)) as blocked + `; + return blocked; + }, 10_000); +} + +// "ok", or the SQLSTATE, message and context the statement failed with +export async function outcome(promise: PromiseLike | null): Promise { + try { + await promise; + return "ok"; + } catch (error) { + const err = error as Error & { code?: string; where?: string }; + const where = err.where ? ` [${err.where.replace(/\s+/g, " ")}]` : ""; + return `${err.code}: ${err.message}${where}`; + } +} diff --git a/packages/pgconductor-js/tests/integration/cancellation-cascades.test.ts b/packages/pgconductor-js/tests/integration/cancellation-cascades.test.ts new file mode 100644 index 0000000..dd1813b --- /dev/null +++ b/packages/pgconductor-js/tests/integration/cancellation-cascades.test.ts @@ -0,0 +1,277 @@ +import { afterAll, afterEach, beforeAll, describe, expect, test } from "vitest"; +import { TestDatabasePool, type TestDatabase } from "../fixtures/test-database"; +import { + ending, + getExecution, + insertExecution, + install, + openSession, + outcome, + settle, + waitUntilBlocked, + type Session, +} from "../fixtures/sessions"; + +// Every path that ends an execution cancels its linked children (cancelWithParent) from the +// graph as it is once the path holds its locks +describe("cancellation cascades (Postgres integration)", () => { + let pool: TestDatabasePool; + const databases: TestDatabase[] = []; + const sessions: Session[] = []; + const ORCH_A = crypto.randomUUID(); + const ORCH_B = crypto.randomUUID(); + + beforeAll(async () => { + pool = await TestDatabasePool.create(); + }, 60000); + + afterEach(async () => { + await Promise.allSettled(sessions.map((s) => s.end())); + sessions.length = 0; + await Promise.all(databases.map((db) => db.destroy())); + databases.length = 0; + }); + + afterAll(async () => { + await pool?.destroy(); + }); + + async function database(removeOnFailDays: number | null = null) { + const db = await pool.child(); + databases.push(db); + await install(db, removeOnFailDays); + return db; + } + + // P has linked child L, which is running. Session A settles L as invoke_child (starts G and + // suspends L on it) and leaves the transaction open. + async function suspendingLinkedChild(db: TestDatabase, parentRunning: boolean) { + const a = await openSession(db); + const b = await openSession(db); + sessions.push(a, b); + + const tokenP = crypto.randomUUID(); + const tokenL = crypto.randomUUID(); + const P = await insertExecution( + db.sql, + parentRunning + ? { task_key: "parent", locked_by: ORCH_B, claim_token: tokenP } + : { task_key: "parent", waiting_step_key: "some-event", suspended: true }, + ); + const L = await insertExecution(db.sql, { + task_key: "linked", + parent_execution_id: P, + cancel_with_parent: true, + locked_by: ORCH_A, + claim_token: tokenL, + }); + + await a.sql`begin`; + await settle(a.sql, ORCH_A, [ + { + execution_id: L, + queue: "default", + task_key: "linked", + claim_token: tokenL, + status: "invoke_child", + timeout_ms: "infinity", + step_key: "invoke-g", + child_task_name: "grandchild", + child_task_queue: "default", + child_payload: null, + }, + ]); + const [{ G }] = await a.sql<[{ G: string }]>` + select id as "G" from pgconductor._private_executions where parent_execution_id = ${L}::uuid + `; + return { a, b, P, L, G, tokenP }; + } + + for (const removeOnFailDays of [null, 0]) { + test(`a parent's failure settlement cancels a grandchild started while it waited (removeOnFail ${removeOnFailDays})`, async () => { + const db = await database(removeOnFailDays); + const { a, b, P, L, G, tokenP } = await suspendingLinkedChild(db, true); + + const failP = outcome( + settle(b.sql, ORCH_B, [ + { + execution_id: P, + queue: "default", + task_key: "parent", + claim_token: tokenP, + status: "permanently_failed", + error: "parent failed", + }, + ]), + ); + await waitUntilBlocked(db.sql, b, a); + await a.sql`commit`; + expect(await failP).toBe("ok"); + + const cancelled = removeOnFailDays === 0 ? "removed" : { failed: true, cancelled: true }; + expect(await ending(db.sql, L)).toEqual(cancelled); + expect(await ending(db.sql, G)).toEqual(cancelled); + }); + } + + test("a parent's failure settlement sees linked children settled in the same batch in their new state", async () => { + const db = await database(); + const token = crypto.randomUUID(); + const P = await insertExecution(db.sql, { + task_key: "parent", + locked_by: ORCH_A, + claim_token: token, + }); + const linked = () => + insertExecution(db.sql, { + task_key: "linked", + parent_execution_id: P, + cancel_with_parent: true, + locked_by: ORCH_A, + claim_token: token, + }); + const sleeping = await linked(); + const completing = await linked(); + + await settle(db.sql, ORCH_A, [ + { + execution_id: P, + queue: "default", + task_key: "parent", + claim_token: token, + status: "permanently_failed", + error: "parent failed", + }, + { + execution_id: sleeping, + queue: "default", + task_key: "linked", + claim_token: token, + status: "released", + reschedule_in_ms: "infinity", + step_key: "nap", + }, + { + execution_id: completing, + queue: "default", + task_key: "linked", + claim_token: token, + status: "completed", + result: { ok: true }, + }, + ]); + + expect(await getExecution(db.sql, sleeping)).toMatchObject({ + cancelled: true, + last_error: "parent failed", + locked_by: null, + }); + expect((await getExecution(db.sql, sleeping))?.failed_at).not.toBeNull(); + expect(await getExecution(db.sql, completing)).toMatchObject({ + cancelled: false, + failed_at: null, + }); + expect((await getExecution(db.sql, completing))?.completed_at).not.toBeNull(); + }); + + test("cancel_execution of a suspended parent cancels a grandchild started while it waited", async () => { + const db = await database(); + const { a, b, P, L, G } = await suspendingLinkedChild(db, false); + + const cancelP = outcome(b.sql`select pgconductor.cancel_execution(${P}::uuid)`); + await waitUntilBlocked(db.sql, b, a); + await a.sql`commit`; + expect(await cancelP).toBe("ok"); + + expect(await ending(db.sql, L)).toEqual({ failed: true, cancelled: true }); + expect(await ending(db.sql, G)).toEqual({ failed: true, cancelled: true }); + }); + + // Reinvoking a deduped parent supersedes its running attempt, which releases the claim, so + // neither that attempt's settlement nor a later cancel can cascade to the children + for (const removeOnFailDays of [null, 0]) { + for (const childState of ["pending", "suspended"] as const) { + test(`superseding a parent cancels its ${childState} linked child (removeOnFail ${removeOnFailDays})`, async () => { + const db = await database(removeOnFailDays); + const P = await insertExecution(db.sql, { + task_key: "parent", + dedupe_key: "k", + locked_by: ORCH_A, + claim_token: crypto.randomUUID(), + }); + const C = await insertExecution(db.sql, { + task_key: "linked", + parent_execution_id: P, + cancel_with_parent: true, + ...(childState === "suspended" ? { waiting_step_key: "forever", suspended: true } : {}), + }); + + const [reinvoked] = await db.sql<[{ id: string }]>` + select id from pgconductor.invoke(p_task_key := 'parent', p_dedupe_key := 'k') + `; + expect(reinvoked.id).not.toBe(P); + expect((await getExecution(db.sql, P))?.last_error || "removed").toMatch( + removeOnFailDays === 0 ? "removed" : "superseded by reinvoke", + ); + + const child = await getExecution(db.sql, C); + expect( + child ? { cancelled: child.cancelled, last_error: child.last_error } : "removed", + ).toEqual( + removeOnFailDays === 0 + ? "removed" + : { cancelled: true, last_error: "superseded by reinvoke" }, + ); + }); + } + } + + test("an invokeAndWait timeout cancels the awaited child with its children", async () => { + const db = await database(); + const tokenP = crypto.randomUUID(); + const P = await insertExecution(db.sql, { + task_key: "parent", + locked_by: ORCH_A, + claim_token: tokenP, + }); + const C = crypto.randomUUID(); + const G = await insertExecution(db.sql, { task_key: "grandchild", parent_execution_id: C }); + await insertExecution(db.sql, { + id: C, + task_key: "child", + parent_execution_id: P, + waiting_on_execution_id: G, + waiting_step_key: "wait-g", + suspended: true, + }); + const L = await insertExecution(db.sql, { + task_key: "linked", + parent_execution_id: C, + cancel_with_parent: true, + }); + // P resumed after its timeout, still waiting on C + await db.sql` + update pgconductor._private_executions set waiting_on_execution_id = ${C}::uuid, + waiting_step_key = 'wait-c' + where id = ${P}::uuid + `; + + await db.client.clearWaitingState({ + executionId: P, + queue: "default", + orchestratorId: ORCH_A, + claimToken: tokenP, + }); + + const cancelled = { failed: true, cancelled: true }; + expect(await ending(db.sql, C)).toEqual(cancelled); + expect((await getExecution(db.sql, C))?.last_error).toBe("Cancelled: parent timed out"); + expect(await ending(db.sql, G)).toEqual(cancelled); + expect(await ending(db.sql, L)).toEqual(cancelled); + expect(await getExecution(db.sql, P)).toMatchObject({ + waiting_on_execution_id: null, + failed_at: null, + cancelled: false, + }); + }); +}); diff --git a/packages/pgconductor-js/tests/integration/cancellation.test.ts b/packages/pgconductor-js/tests/integration/cancellation.test.ts index 096fa40..f11a477 100644 --- a/packages/pgconductor-js/tests/integration/cancellation.test.ts +++ b/packages/pgconductor-js/tests/integration/cancellation.test.ts @@ -1378,7 +1378,7 @@ describe("Cancellation Support", () => { expect(await executionState(db, child)).toEqual({ failed: true, - cancelled: false, + cancelled: true, last_error: "Cancelled: parent timed out", waiting_on_execution_id: null, }); diff --git a/packages/pgconductor-js/tests/integration/invoke-support.test.ts b/packages/pgconductor-js/tests/integration/invoke-support.test.ts index 7194373..118d644 100644 --- a/packages/pgconductor-js/tests/integration/invoke-support.test.ts +++ b/packages/pgconductor-js/tests/integration/invoke-support.test.ts @@ -616,7 +616,7 @@ describe("Invoke Support", () => { `; expect(children.length).toBe(1); - expect(children[0]?.cancelled).toBe(false); // Not cancelled since it was pending + expect(children[0]?.cancelled).toBe(true); expect(children[0]?.failed_at).not.toBeNull(); expect(children[0]?.last_error).toContain("parent timed out"); expect(children[0]?.group).toBe("tenant-a"); diff --git a/packages/pgconductor-js/tests/integration/lock-order.test.ts b/packages/pgconductor-js/tests/integration/lock-order.test.ts new file mode 100644 index 0000000..2b8bdaa --- /dev/null +++ b/packages/pgconductor-js/tests/integration/lock-order.test.ts @@ -0,0 +1,271 @@ +import { afterAll, afterEach, beforeAll, describe, expect, test } from "vitest"; +import type { Sql } from "postgres"; +import { QueryBuilder } from "../../src/query-builder"; +import { TestDatabasePool, type TestDatabase } from "../fixtures/test-database"; +import { + insertExecution, + install, + openSession, + outcome, + settle, + waitUntilBlocked, + type Session, +} from "../fixtures/sessions"; + +// Each test lets session B take the first lock of its real statement inside an open +// transaction, starts session A's real statement until Postgres reports it waiting on B, then +// runs B's real statement. Both must succeed; a lock-order inversion makes Postgres abort one +// of them with 40P01. +describe("lock order (Postgres integration)", () => { + let pool: TestDatabasePool; + const databases: TestDatabase[] = []; + const sessions: Session[] = []; + const ORCH_A = crypto.randomUUID(); + const ORCH_B = crypto.randomUUID(); + + beforeAll(async () => { + pool = await TestDatabasePool.create(); + }, 60000); + + afterEach(async () => { + await Promise.allSettled(sessions.map((s) => s.end())); + sessions.length = 0; + await Promise.all(databases.map((db) => db.destroy())); + databases.length = 0; + }); + + afterAll(async () => { + await pool?.destroy(); + }); + + async function setup() { + const db = await pool.child(); + databases.push(db); + await install(db); + const a = await openSession(db); + const b = await openSession(db); + sessions.push(a, b); + return { db, a, b }; + } + + async function lockExecution(session: Session, id: string) { + await session.sql`begin`; + await session.sql`select 1 from pgconductor._private_executions where id = ${id}::uuid for update`; + } + + // Runs A until it waits on B, then B's statement; returns how each ended + async function race( + db: TestDatabase, + a: Session, + b: Session, + statementA: PromiseLike | null, + statementB: () => PromiseLike | null, + ) { + const doneA = outcome(statementA); + await waitUntilBlocked(db.sql, a, b); + const resultB = await outcome(statementB()); + await b.sql.unsafe(resultB === "ok" ? "commit" : "rollback"); + return { a: await doneA, b: resultB }; + } + + // P has linked child L (cancelWithParent); L is suspended in invokeAndWait on running C + async function linkedChain(db: TestDatabase) { + const tokenP = crypto.randomUUID(); + const tokenC = crypto.randomUUID(); + const P = await insertExecution(db.sql, { + task_key: "parent", + locked_by: ORCH_A, + claim_token: tokenP, + }); + const L = crypto.randomUUID(); + const C = await insertExecution(db.sql, { + task_key: "child", + parent_execution_id: L, + locked_by: ORCH_B, + claim_token: tokenC, + }); + await insertExecution(db.sql, { + id: L, + task_key: "linked", + parent_execution_id: P, + cancel_with_parent: true, + waiting_on_execution_id: C, + waiting_step_key: "wait-c", + suspended: true, + }); + const failP = { + execution_id: P, + queue: "default", + task_key: "parent", + claim_token: tokenP, + status: "permanently_failed", + error: "parent failed", + } as const; + return { P, L, C, tokenC, failP }; + } + + test("a parent failure cascading to L and the child C it waits on vs completion of C waking L", async () => { + const { db, a, b } = await setup(); + const { C, tokenC, failP } = await linkedChain(db); + + await lockExecution(b, C); + const result = await race(db, a, b, settle(a.sql, ORCH_A, [failP]), () => + settle(b.sql, ORCH_B, [ + { + execution_id: C, + queue: "default", + task_key: "child", + claim_token: tokenC, + status: "completed", + result: { ok: true }, + }, + ]), + ); + expect(result).toEqual({ a: "ok", b: "ok" }); + }); + + test("a parent failure cascading to L and the child C it waits on vs permanent failure of C failing L", async () => { + const { db, a, b } = await setup(); + const { C, tokenC, failP } = await linkedChain(db); + + await lockExecution(b, C); + const result = await race(db, a, b, settle(a.sql, ORCH_A, [failP]), () => + settle(b.sql, ORCH_B, [ + { + execution_id: C, + queue: "default", + task_key: "child", + claim_token: tokenC, + status: "permanently_failed", + error: "child failed", + }, + ]), + ); + expect(result).toEqual({ a: "ok", b: "ok" }); + }); + + test("cancel_execution of a parent waiting for the result of its linked child vs that child's result delivery", async () => { + const { db, a, b } = await setup(); + const P = await insertExecution(db.sql, { + task_key: "parent", + waiting_step_key: "wait-c", + suspended: true, + }); + const tokenC = crypto.randomUUID(); + const C = await insertExecution(db.sql, { + task_key: "linked", + parent_execution_id: P, + cancel_with_parent: true, + locked_by: ORCH_B, + claim_token: tokenC, + result_waiters: [{ execution_id: P, queue: "default", step_key: "wait-c" }], + }); + + await lockExecution(b, C); + const result = await race( + db, + a, + b, + a.sql`select pgconductor.cancel_execution(${P}::uuid)`, + () => + settle(b.sql, ORCH_B, [ + { + execution_id: C, + queue: "default", + task_key: "linked", + claim_token: tokenC, + status: "completed", + result: { ok: true }, + }, + ]), + ); + expect(result).toEqual({ a: "ok", b: "ok" }); + }); + + test("waitForAny registration over [X, Y] vs completion of Y waking X (X < Y)", async () => { + const { db, a, b } = await setup(); + const [X, Y] = [crypto.randomUUID(), crypto.randomUUID()].sort() as [string, string]; + const tokenP = crypto.randomUUID(); + const tokenY = crypto.randomUUID(); + const P = await insertExecution(db.sql, { + task_key: "racer", + locked_by: ORCH_A, + claim_token: tokenP, + }); + await insertExecution(db.sql, { + id: X, + task_key: "parent", + waiting_on_execution_id: Y, + waiting_step_key: "wait-y", + suspended: true, + }); + await insertExecution(db.sql, { + id: Y, + task_key: "child", + parent_execution_id: X, + locked_by: ORCH_B, + claim_token: tokenY, + }); + const branches = [ + { key: "x", stepKey: "race:x", executionId: X }, + { key: "y", stepKey: "race:y", executionId: Y }, + ]; + + await lockExecution(b, Y); + const register = a.sql` + select pgconductor._private_register_event_race( + ${P}::uuid, 'default', 'racer', ${ORCH_A}::uuid, ${tokenP}::uuid, 'race', + ${a.sql.json(branches)}::jsonb, null + ) + `; + const result = await race(db, a, b, register, () => + settle(b.sql, ORCH_B, [ + { + execution_id: Y, + queue: "default", + task_key: "child", + claim_token: tokenY, + status: "completed", + result: { ok: true }, + }, + ]), + ); + expect(result).toEqual({ a: "ok", b: "ok" }); + }); + + for (const queueName of ["dq", "src"]) { + test(`drop_queue('dq') vs worker registration of ${queueName === "dq" ? "the same queue" : "a queue dead-lettering into it"}`, async () => { + const { db, a, b } = await setup(); + const register = (sql: Sql) => + new QueryBuilder(sql).buildRegisterWorker({ + queueName, + taskSpecs: [ + queueName === "dq" + ? { key: "dt", queue: "dq" } + : { key: "st", queue: "src", deadLetterQueue: "dq", deadLetterTaskKey: "dt" }, + ], + cronSchedules: [], + eventSubscriptions: [], + }); + await new QueryBuilder(db.sql).buildRegisterWorker({ + queueName: "dq", + taskSpecs: [{ key: "dt", queue: "dq" }], + cronSchedules: [], + eventSubscriptions: [], + }); + await register(db.sql); + + // B: the registration holding its first lock, the rows of its queues + await b.sql`begin`; + await b.sql` + select 1 from pgconductor._private_queues where name in ('dq', ${queueName}) + order by name for update + `; + + const result = await race(db, a, b, a.sql`select pgconductor.drop_queue('dq')`, () => + register(b.sql), + ); + expect(result).toEqual({ a: "ok", b: "ok" }); + }); + } +}); diff --git a/packages/pgconductor-js/tests/integration/wait-for-result.test.ts b/packages/pgconductor-js/tests/integration/wait-for-result.test.ts index 870e2bf..ab2d596 100644 --- a/packages/pgconductor-js/tests/integration/wait-for-result.test.ts +++ b/packages/pgconductor-js/tests/integration/wait-for-result.test.ts @@ -731,7 +731,7 @@ describe("ctx.waitForResult and execution branches (Postgres integration)", () = }); }); - test("a pending child failed when its parent's wait times out resolves the wait", async () => { + test("a pending child cancelled when its parent's wait times out resolves the wait", async () => { const database = await db(); const conductor = await setup(database, { waiter: async () => {} }); await stopOrchestrators(); @@ -764,7 +764,7 @@ describe("ctx.waitForResult and execution branches (Postgres integration)", () = expect(await woken(database, waiterId)).toEqual({ result: { status: "resolved", - outcome: { status: "failed", error: "Cancelled: parent timed out" }, + outcome: { status: "cancelled", reason: "Cancelled: parent timed out" }, }, due: true, }); diff --git a/packages/pgconductor-js/tests/integration/worker-lifecycle.test.ts b/packages/pgconductor-js/tests/integration/worker-lifecycle.test.ts index a53ba46..0f119a6 100644 --- a/packages/pgconductor-js/tests/integration/worker-lifecycle.test.ts +++ b/packages/pgconductor-js/tests/integration/worker-lifecycle.test.ts @@ -487,15 +487,11 @@ describe("flush", () => { let failed = false; const t = await setup( (sql) => - failStatements( - sql, - "select 1 as result from (select count(*) from resolved_result_waits)", - () => { - if (failed) return null; - failed = true; - return connectionError(); - }, - ), + failStatements(sql, "from (select count(*) as writes from writes) w", () => { + if (failed) return null; + failed = true; + return connectionError(); + }), { concurrency: 1, flushIntervalMs: 60_000 }, ); t.gates[0]?.resolve();