Commit b3f25d9
UN-3563 [FEAT] PG Queue 9e PR 2c — live PG fan-out/barrier/callback (fire-and-forget) (#2069)
* UN-3563 [FEAT] PG Queue 9e PR 2c — live PG fan-out/barrier/callback (fire-and-forget)
Wires the coupled pipeline's fan-out → barrier → callback onto the PG queue for
a transport=="pg_queue" execution. Gated: resolve_transport() still returns
celery (PR3 Flipt flips it), so the whole PG branch is present-but-unreachable —
default path byte-identical. Orchestrator task (async_execute_bin) stays on
Celery (hybrid); routing it onto PG is a 2d follow-up.
- barrier.py: Barrier Protocol + CeleryChordBarrier/RedisDecrBarrier accept (and
ignore) a `transport` param; CallbackDescriptor gains an optional `backend`.
- orchestration_utils._barrier_for_transport: pg_queue → fresh PgBarrier()
(bypasses the WORKER_BARRIER_BACKEND singleton), else the singleton.
- pg_barrier.PgBarrier.enqueue(transport): pg_queue → fire-and-forget mode —
_dispatch_header_pg sends each header via dispatch(backend=PG) with an injected
_barrier_context {execution_id, batch_index, callback_descriptor}, no .link;
descriptor marked backend=pg_queue; UPSERT block also clears pg_batch_dedup
(greptile #2068 reuse-reset). _fire_barrier_callback self-chains the callback
onto PG when backend==pg_queue. clear_execution_batches at finalise + abort.
run_batch_with_barrier(): claim → work → in-body _barrier_pg_decrement;
redelivery skips; exception → barrier_pg_abort.
- file_processing.process_file_batch(_barrier_context=None): core routes None →
_run_batch_stages (celery chord path), else → run_batch_with_barrier.
- general/api fan-outs thread transport into create_chord_execution.
Tests: +8 PgBarrier fire-and-forget + 2 orchestration routing + 2 process_file_batch
routing. Each test file green alone; ruff clean. End-to-end forced-pg dev-test
pending before PR.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* UN-3563 fix SonarCloud S1172: drop unused task_instance from _run_batch_stages
The extracted _run_batch_stages never uses task_instance — its only purpose
(deriving celery_task_id) happens in _process_file_batch_core before the call.
Removed the param + updated both call sites. _process_file_batch_core keeps
task_instance (it reads .request.id). Routing test mocks with *a, unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* UN-3563 address review (muhammad-ali-e, 15): strand-on-failure hardening + typing/dedup/docs/tests
Decision (with reviewer): reaper-as-safety-net for the un-catchable strand
windows + fix what's catchable + document + gate on PR3.
Failure handling:
- [#69 Critical] run_batch_with_barrier wraps BOTH work + decrement in the abort:
a decrement-side failure (guard / DB / last-batch callback dispatch) tears the
barrier down in-body instead of stranding to expiry.
- [#79] extracted _abort_barrier_in_body — logs when the teardown itself fails
(was silently suppressed under a misleading "torn down" message).
- [#74/#81] documented the two un-catchable strand windows (hard-crash-during-work,
post-commit callback-dispatch-fail) as a HARD reaper dependency for PR3.
- [#86] finalise cleanup split into independent try/excepts with distinct logs.
Typing / clarity:
- [#1] BarrierContext(TypedDict) for _barrier_context (header fan-out, run_batch_with_barrier,
process_file_batch).
- [#3] renamed CallbackDescriptor "backend" -> "transport" (WorkflowTransport value;
avoids the QueueBackend "pg" collision).
- [#27] is_pg_transport() predicate in core; used in orchestration_utils + pg_barrier.
- [#20] extracted _dispatch_pg() — single home for cycle-avoiding local import + backend=PG.
- [#35] normalize_transport() at the general worker entry (parity w/ api/scheduler).
- [#94] log when a header has no queue option.
- [#9/#13] fixed born-stale comment + kwargs-not-args docstring.
Tests (+#37/#41): last-batch self-chains callback to PG + cleans up barrier/dedup;
decrement-failure aborts; PG-branch mid-loop dispatch-failure deletes row; header
args/queue/pre-existing-kwargs preservation. 137 barrier/dedup/routing tests green;
bootstrap clean under WORKER_BARRIER_BACKEND=pg.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* UN-3563 fix run_batch_with_barrier strand-window doc inconsistency (review)
The second "NOT catchable" bullet conflated two different things: it described
the in-body catchable abort ("the abort here removes the row") and a *software*
callback-dispatch failure — but that failure is already caught + torn down by
step 3's wrap (paragraph 1), so it doesn't belong under the un-catchable
heading, and on the PG path _fire_barrier_callback IS the enqueue so "committed
but before the enqueue" couldn't both hold.
Rewrote the bullet to the genuinely un-catchable window: a hard crash BETWEEN
the decrement committing (remaining→0) and the callback enqueue completing —
decrement committed (redelivery blocked by the marker), process gone before the
callback enqueues or any abort runs, row survives to expiry, reaper-only
recovery. Explicitly notes a software dispatch failure is the catchable case.
Keeps this list an accurate spec for the PR-3 reaper-recovery dependency.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* UN-3563 address greptile (#2069, 2): clear dedup on mid-loop PG failure + carry fairness on PG callback
Both in the gated PG path (greptile 4/5, safe to merge).
- Issue 1: PgBarrier.enqueue mid-loop dispatch-failure handler now also calls
clear_execution_batches on the PG path. Earlier headers may have committed a
claim_batch marker; with the barrier row deleted, their in-flight
barrier_pg_abort is a no-op (already_aborted) and never reaches the clear
inside it, so reclaim the markers directly here.
- Issue 2: the PG callback now carries the producer's fairness. Added
_fairness_from_headers() to reconstruct the FairnessKey from the stored
x-fairness-key headers and pass it to _dispatch_pg, so the callback rides the
same org/priority as the Celery path (was always default priority).
Tests: +fairness-carried / +fairness-none-safe on _fire_barrier_callback;
extended the PG mid-loop test to assert an already-claimed marker is reclaimed.
75 barrier/dedup tests green; bootstrap clean under WORKER_BARRIER_BACKEND=pg.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* UN-3563 fix SonarCloud S3776: reduce PgBarrier.enqueue cognitive complexity (17→under 15)
Extracted the per-header dispatch loop into PgBarrier._dispatch_headers — the
deeply-nested for→try/except→if/else→if (PG-vs-celery branch + mid-loop failure
teardown + PG dedup-clear) was the complexity driver. enqueue now calls the
helper; behaviour identical. radon: enqueue C(11)→B(6); ruff C901 passes.
75 barrier/dedup tests green; ruff + ruff-format clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* UN-3563 fix greptile #2069: mid-loop dedup-clear test passed for the wrong reason
The pre-seeded claim_batch marker was wiped by enqueue's UPSERT block (the
reuse-reset DELETE) before the dispatch loop, so the mid-loop
clear_execution_batches deleted 0 rows — the count==0 assertion passed on the
UPSERT, not the guard under test. Now the first dispatch side-effect claims the
marker AFTER the UPSERT (simulating a fast PG consumer), so the mid-loop clear
is what removes it. Verified: with the clear disabled the marker orphans (count=1).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>1 parent 5f2d18d commit b3f25d9
11 files changed
Lines changed: 894 additions & 93 deletions
File tree
- unstract/core/src/unstract/core
- workers
- api-deployment
- file_processing
- general
- queue_backend
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
249 | 249 | | |
250 | 250 | | |
251 | 251 | | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
252 | 263 | | |
253 | 264 | | |
254 | 265 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
29 | | - | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
30 | 36 | | |
31 | 37 | | |
32 | 38 | | |
| |||
683 | 689 | | |
684 | 690 | | |
685 | 691 | | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
686 | 700 | | |
687 | 701 | | |
688 | 702 | | |
| |||
694 | 708 | | |
695 | 709 | | |
696 | 710 | | |
| 711 | + | |
697 | 712 | | |
698 | 713 | | |
699 | 714 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
| 15 | + | |
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
| |||
222 | 224 | | |
223 | 225 | | |
224 | 226 | | |
225 | | - | |
226 | | - | |
| 227 | + | |
| 228 | + | |
227 | 229 | | |
228 | | - | |
229 | | - | |
230 | | - | |
231 | | - | |
| 230 | + | |
232 | 231 | | |
233 | | - | |
234 | | - | |
235 | | - | |
236 | | - | |
237 | | - | |
238 | | - | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
239 | 236 | | |
240 | | - | |
241 | | - | |
242 | | - | |
243 | | - | |
244 | 237 | | |
245 | 238 | | |
246 | 239 | | |
| |||
260 | 253 | | |
261 | 254 | | |
262 | 255 | | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
263 | 294 | | |
264 | 295 | | |
265 | 296 | | |
| |||
272 | 303 | | |
273 | 304 | | |
274 | 305 | | |
275 | | - | |
276 | | - | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
277 | 312 | | |
278 | 313 | | |
279 | 314 | | |
280 | 315 | | |
281 | 316 | | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
282 | 321 | | |
283 | 322 | | |
284 | 323 | | |
285 | 324 | | |
286 | | - | |
| 325 | + | |
287 | 326 | | |
288 | 327 | | |
289 | 328 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| 59 | + | |
59 | 60 | | |
60 | 61 | | |
61 | 62 | | |
| |||
488 | 489 | | |
489 | 490 | | |
490 | 491 | | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
491 | 498 | | |
492 | 499 | | |
493 | 500 | | |
| |||
710 | 717 | | |
711 | 718 | | |
712 | 719 | | |
| 720 | + | |
713 | 721 | | |
714 | 722 | | |
715 | 723 | | |
| |||
764 | 772 | | |
765 | 773 | | |
766 | 774 | | |
| 775 | + | |
767 | 776 | | |
768 | 777 | | |
769 | 778 | | |
| |||
957 | 966 | | |
958 | 967 | | |
959 | 968 | | |
| 969 | + | |
960 | 970 | | |
961 | 971 | | |
962 | 972 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
39 | | - | |
| 39 | + | |
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
| 43 | + | |
| 44 | + | |
43 | 45 | | |
44 | 46 | | |
45 | 47 | | |
| |||
92 | 94 | | |
93 | 95 | | |
94 | 96 | | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
95 | 120 | | |
96 | 121 | | |
97 | 122 | | |
| |||
112 | 137 | | |
113 | 138 | | |
114 | 139 | | |
| 140 | + | |
115 | 141 | | |
116 | 142 | | |
117 | 143 | | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
118 | 149 | | |
119 | 150 | | |
120 | 151 | | |
| |||
166 | 197 | | |
167 | 198 | | |
168 | 199 | | |
| 200 | + | |
169 | 201 | | |
170 | 202 | | |
| 203 | + | |
171 | 204 | | |
172 | 205 | | |
173 | 206 | | |
| |||
0 commit comments