Skip to content

Commit 5600853

Browse files
committed
fix(worker): prevent child tasks from blocking parent execution
Workers used SyncTaskBackend which executed child tasks inline — e.g. consolidation triggered by retain would block until consolidation finished, tying up the worker slot for both operations. Add WorkerTaskBackend whose submit_task is a no-op: since _submit_async_operation already INSERTs the child row with task_payload, the poller picks it up on the next cycle as an independent task.
1 parent bdb3a55 commit 5600853

3 files changed

Lines changed: 227 additions & 6 deletions

File tree

hindsight-api-slim/hindsight_api/engine/task_backend.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
Task backend for distributed task processing.
33
44
This provides an abstraction for task storage and execution:
5-
- BrokerTaskBackend: Uses PostgreSQL as broker (production)
5+
- BrokerTaskBackend: Uses PostgreSQL as broker (production API servers)
6+
- WorkerTaskBackend: No-op submit_task (production workers — child tasks are polled)
67
- SyncTaskBackend: Executes tasks immediately (testing/embedded)
78
"""
89

@@ -125,6 +126,33 @@ async def shutdown(self):
125126
logger.debug("SyncTaskBackend shutdown")
126127

127128

129+
class WorkerTaskBackend(TaskBackend):
130+
"""
131+
Task backend for worker processes.
132+
133+
Workers execute tasks directly via the poller (claim → execute), so they
134+
don't need submit_task to run anything. When engine code running *inside*
135+
a worker-executed task calls submit_task (e.g. retain triggers consolidation),
136+
the row has already been INSERTed into async_operations with task_payload by
137+
_submit_async_operation — so submit_task is a no-op. The new task will be
138+
picked up by a worker on the next poll cycle instead of being executed inline,
139+
which avoids blocking the parent task.
140+
"""
141+
142+
async def initialize(self):
143+
self._initialized = True
144+
logger.debug("WorkerTaskBackend initialized")
145+
146+
async def submit_task(self, task_dict: dict[str, Any]):
147+
"""No-op: the row already exists in async_operations; a worker will claim it."""
148+
task_type = task_dict.get("type", "unknown")
149+
logger.debug(f"WorkerTaskBackend: submit_task no-op for {task_type} (will be picked up by poller)")
150+
151+
async def shutdown(self):
152+
self._initialized = False
153+
logger.debug("WorkerTaskBackend shutdown")
154+
155+
128156
class BrokerTaskBackend(TaskBackend):
129157
"""
130158
Task backend using PostgreSQL as broker.

hindsight-api-slim/hindsight_api/worker/main.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import warnings
1919

2020
from ..config import get_config
21-
from ..engine.task_backend import SyncTaskBackend
21+
from ..engine.task_backend import WorkerTaskBackend
2222
from .poller import WorkerPoller
2323

2424
# Filter deprecation warnings from third-party libraries
@@ -195,11 +195,13 @@ async def run():
195195
logger.info(f"Loaded operation validator: {operation_validator.__class__.__name__}")
196196

197197
# Initialize MemoryEngine
198-
# Workers use SyncTaskBackend because they execute tasks directly,
199-
# they don't need to store tasks (they poll from DB)
198+
# Workers use WorkerTaskBackend: submit_task is a no-op because the
199+
# row already exists in async_operations. Child tasks (e.g. consolidation
200+
# triggered by retain) will be picked up by the poller on the next cycle
201+
# instead of being executed inline, which avoids blocking the parent task.
200202
memory = MemoryEngine(
201203
run_migrations=False, # Workers don't run migrations
202-
task_backend=SyncTaskBackend(),
204+
task_backend=WorkerTaskBackend(),
203205
tenant_extension=tenant_extension,
204206
operation_validator=operation_validator,
205207
)

hindsight-api-slim/tests/test_worker.py

Lines changed: 192 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import pytest
1818
import pytest_asyncio
1919

20-
from hindsight_api.engine.task_backend import BrokerTaskBackend, SyncTaskBackend
20+
from hindsight_api.engine.task_backend import BrokerTaskBackend, SyncTaskBackend, WorkerTaskBackend
2121

2222

2323
async def _ensure_bank(pool, bank_id: str) -> None:
@@ -1317,6 +1317,197 @@ async def failing_executor(task_dict):
13171317
await backend.submit_task({"type": "test"})
13181318

13191319

1320+
class TestWorkerTaskBackend:
1321+
"""Tests for WorkerTaskBackend (used by worker processes).
1322+
1323+
WorkerTaskBackend.submit_task is a no-op: child tasks spawned during
1324+
execution (e.g. consolidation triggered by retain) are left as pending
1325+
rows in async_operations for the poller to pick up on the next cycle,
1326+
instead of being executed inline which would block the parent task.
1327+
"""
1328+
1329+
@pytest.mark.asyncio
1330+
async def test_worker_backend_does_not_execute_inline(self):
1331+
"""WorkerTaskBackend.submit_task must NOT call the executor."""
1332+
executed = []
1333+
1334+
async def mock_executor(task_dict):
1335+
executed.append(task_dict)
1336+
1337+
backend = WorkerTaskBackend()
1338+
backend.set_executor(mock_executor)
1339+
await backend.initialize()
1340+
1341+
await backend.submit_task({"type": "consolidation", "bank_id": "b1"})
1342+
1343+
assert len(executed) == 0, (
1344+
"WorkerTaskBackend must not execute tasks inline — "
1345+
"child tasks should be picked up by the poller"
1346+
)
1347+
1348+
@pytest.mark.asyncio
1349+
async def test_sync_backend_blocks_parent_on_child_task(self):
1350+
"""Demonstrate the blocking problem: with SyncTaskBackend, a parent
1351+
task that calls submit_task for a child is blocked until the child
1352+
completes, because submit_task executes inline.
1353+
"""
1354+
execution_order: list[str] = []
1355+
1356+
async def mock_executor(task_dict):
1357+
task_type = task_dict.get("type", "unknown")
1358+
execution_order.append(f"{task_type}:start")
1359+
if task_type == "parent":
1360+
# Parent spawns a child via submit_task
1361+
await backend.submit_task({"type": "child", "bank_id": "b1"})
1362+
# This line only runs AFTER the child finishes (blocking)
1363+
execution_order.append(f"{task_type}:end")
1364+
1365+
backend = SyncTaskBackend()
1366+
backend.set_executor(mock_executor)
1367+
await backend.initialize()
1368+
1369+
await backend.submit_task({"type": "parent", "bank_id": "b1"})
1370+
1371+
# With SyncTaskBackend the child runs inline, nested inside the parent
1372+
assert execution_order == [
1373+
"parent:start",
1374+
"child:start",
1375+
"child:end",
1376+
"parent:end",
1377+
], f"SyncTaskBackend should execute child inline (blocking parent). Got: {execution_order}"
1378+
1379+
@pytest.mark.asyncio
1380+
async def test_worker_backend_does_not_block_parent_on_child_task(self):
1381+
"""With WorkerTaskBackend, a parent task that calls submit_task for a
1382+
child task is NOT blocked — submit_task is a no-op, so the parent
1383+
completes immediately and the child row stays pending for the poller.
1384+
"""
1385+
execution_order: list[str] = []
1386+
1387+
async def mock_executor(task_dict):
1388+
task_type = task_dict.get("type", "unknown")
1389+
execution_order.append(f"{task_type}:start")
1390+
if task_type == "parent":
1391+
# Parent spawns a child via submit_task — should be a no-op
1392+
await backend.submit_task({"type": "child", "bank_id": "b1"})
1393+
execution_order.append(f"{task_type}:end")
1394+
1395+
backend = WorkerTaskBackend()
1396+
backend.set_executor(mock_executor)
1397+
await backend.initialize()
1398+
1399+
# Simulate the poller calling the executor directly for the parent task
1400+
await mock_executor({"type": "parent", "bank_id": "b1"})
1401+
1402+
# The child should NOT have been executed — only the parent runs
1403+
assert execution_order == [
1404+
"parent:start",
1405+
"parent:end",
1406+
], (
1407+
f"WorkerTaskBackend must not execute child tasks inline. "
1408+
f"Got: {execution_order}"
1409+
)
1410+
1411+
@pytest.mark.asyncio
1412+
async def test_worker_backend_child_task_stays_pending_in_db(self, pool, clean_operations):
1413+
"""End-to-end: when a worker-executed task spawns a child operation,
1414+
the child row stays as 'pending' in async_operations (not executed inline).
1415+
A subsequent poll cycle can then claim and execute it independently.
1416+
"""
1417+
from hindsight_api.worker import WorkerPoller
1418+
from hindsight_api.worker.poller import ClaimedTask
1419+
1420+
bank_id = f"test-worker-{uuid.uuid4().hex[:8]}"
1421+
await _ensure_bank(pool, bank_id)
1422+
1423+
# Pre-create the parent operation (as the poller would claim it)
1424+
parent_op_id = uuid.uuid4()
1425+
parent_payload = {
1426+
"type": "batch_retain",
1427+
"operation_id": str(parent_op_id),
1428+
"bank_id": bank_id,
1429+
}
1430+
await pool.execute(
1431+
"""
1432+
INSERT INTO async_operations (operation_id, bank_id, operation_type, status, task_payload, worker_id, claimed_at)
1433+
VALUES ($1, $2, 'retain', 'processing', $3::jsonb, 'test-worker-1', now())
1434+
""",
1435+
parent_op_id,
1436+
bank_id,
1437+
json.dumps(parent_payload),
1438+
)
1439+
1440+
# Pre-create the child operation row (as _submit_async_operation would)
1441+
child_op_id = uuid.uuid4()
1442+
child_payload = {
1443+
"type": "consolidation",
1444+
"operation_id": str(child_op_id),
1445+
"bank_id": bank_id,
1446+
}
1447+
await pool.execute(
1448+
"""
1449+
INSERT INTO async_operations (operation_id, bank_id, operation_type, status, task_payload)
1450+
VALUES ($1, $2, 'consolidation', 'pending', $3::jsonb)
1451+
""",
1452+
child_op_id,
1453+
bank_id,
1454+
json.dumps(child_payload),
1455+
)
1456+
1457+
# The executor simulates retain: marks parent completed, then calls
1458+
# submit_task for the child (which WorkerTaskBackend should ignore).
1459+
backend = WorkerTaskBackend()
1460+
1461+
async def executor(task_dict):
1462+
# Mark parent as completed
1463+
await pool.execute(
1464+
"UPDATE async_operations SET status = 'completed', completed_at = now() WHERE operation_id = $1",
1465+
parent_op_id,
1466+
)
1467+
# Trigger child task via submit_task (should be no-op for WorkerTaskBackend)
1468+
await backend.submit_task(child_payload)
1469+
1470+
poller = WorkerPoller(
1471+
pool=pool,
1472+
worker_id="test-worker-1",
1473+
executor=executor,
1474+
)
1475+
1476+
# Execute the parent task
1477+
claimed_task = ClaimedTask(
1478+
operation_id=str(parent_op_id),
1479+
task_dict=parent_payload,
1480+
schema=None,
1481+
)
1482+
await poller.execute_task(claimed_task)
1483+
completed = await poller.wait_for_active_tasks(timeout=5.0)
1484+
assert completed, "Parent task did not complete within timeout"
1485+
1486+
# Parent should be completed
1487+
parent_row = await pool.fetchrow(
1488+
"SELECT status FROM async_operations WHERE operation_id = $1",
1489+
parent_op_id,
1490+
)
1491+
assert parent_row["status"] == "completed"
1492+
1493+
# Child should still be pending (NOT executed inline)
1494+
child_row = await pool.fetchrow(
1495+
"SELECT status, worker_id FROM async_operations WHERE operation_id = $1",
1496+
child_op_id,
1497+
)
1498+
assert child_row["status"] == "pending", (
1499+
f"Child task should remain 'pending' for the next poll cycle, "
1500+
f"but got '{child_row['status']}'. This means it was executed inline, "
1501+
f"blocking the parent task."
1502+
)
1503+
assert child_row["worker_id"] is None
1504+
1505+
# Now verify the poller can claim the child on the next cycle
1506+
claimed = await poller.claim_batch()
1507+
child_claimed = [c for c in claimed if c.operation_id == str(child_op_id)]
1508+
assert len(child_claimed) == 1, "Child task should be claimable on next poll"
1509+
1510+
13201511
class TestDynamicTenantDiscovery:
13211512
"""Tests for dynamic tenant discovery via TenantExtension."""
13221513

0 commit comments

Comments
 (0)