From 38e38174335f767cb6b90f6748ac8a7cd1dbe0c6 Mon Sep 17 00:00:00 2001 From: sinohqb Date: Fri, 14 Aug 2026 14:52:34 +0800 Subject: [PATCH] =?UTF-8?q?fix(intelligent-eval):=20atomic=20CAS=20in=20as?= =?UTF-8?q?sign=5Ftask=20and=20complete=5Ftask=20(resolves=20=C2=A76.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace read-check-write in task_queue.assign_task with UPDATE...WHERE status='pending' and decide on rowcount so two concurrent workers cannot both claim the same task. Also harden complete_task with the same CAS pattern (status='assigned') so a worker + stuck-handler double-complete leaves the DB in one state. The xfail guard in test_worker_task_resilience now passes (4/4). --- .../agenteval/intelligent_eval/task_queue.py | 61 +++++++++++-------- .../test_worker_task_resilience.py | 8 --- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/backend/agenteval/intelligent_eval/task_queue.py b/backend/agenteval/intelligent_eval/task_queue.py index 5bb2749..2c01ef9 100644 --- a/backend/agenteval/intelligent_eval/task_queue.py +++ b/backend/agenteval/intelligent_eval/task_queue.py @@ -10,7 +10,7 @@ OpenClaw workers to pick up. Tasks are prioritized by: from datetime import timedelta from typing import Optional -from sqlmodel import Session, select +from sqlmodel import Session, select, update from agenteval.intelligent_eval.models import IntelligentEvalStatus from agenteval.storage.db import ( @@ -113,39 +113,50 @@ def get_next_task(session: Session) -> Optional[IntelligentEvalTaskQueueDB]: def assign_task(task_id: str, cron_id: str, session: Session) -> bool: - """Assign a task to a cron. + """Atomically assign a pending task to a cron (CAS on status). - Returns: - True if assigned successfully, False if task not found or already assigned + P1 真问题修复(§6.1): use ``UPDATE ... WHERE status='pending'`` and decide + on ``rowcount`` so two concurrent workers cannot both claim the same task. + The previous read-check-write left a race because SQLite + two sessions + could each read ``status=pending`` and each commit. """ - task = session.get(IntelligentEvalTaskQueueDB, task_id) - if task is None or task.status != "pending": - return False - - task.status = "assigned" - task.assigned_cron_id = cron_id - task.assigned_at = utc_now() - task.updated_at = utc_now() + stmt = ( + update(IntelligentEvalTaskQueueDB) + .where(IntelligentEvalTaskQueueDB.id == task_id) + .where(IntelligentEvalTaskQueueDB.status == "pending") + .values( + status="assigned", + assigned_cron_id=cron_id, + assigned_at=utc_now(), + updated_at=utc_now(), + ) + ) + result = session.exec(stmt) session.commit() - return True + return result.rowcount > 0 def complete_task(task_id: str, success: bool, error: Optional[str], session: Session) -> bool: - """Mark a task as completed or failed. + """Atomically mark a task as completed/failed (CAS on status). - Returns: - True if completed successfully, False if task not found + P1 真问题修复(§6.1 审计): guard with ``status='assigned'`` so a + double-complete from worker + stuck-handler leaves the DB in one state. """ - task = session.get(IntelligentEvalTaskQueueDB, task_id) - if task is None: - return False - - task.status = "completed" if success else "failed" - task.completed_at = utc_now() - task.error = error - task.updated_at = utc_now() + terminal = "completed" if success else "failed" + stmt = ( + update(IntelligentEvalTaskQueueDB) + .where(IntelligentEvalTaskQueueDB.id == task_id) + .where(IntelligentEvalTaskQueueDB.status == "assigned") + .values( + status=terminal, + completed_at=utc_now(), + error=error, + updated_at=utc_now(), + ) + ) + result = session.exec(stmt) session.commit() - return True + return result.rowcount > 0 def requeue_stuck_task(eval_id: str, cron_id: str, session: Session) -> bool: """Mark the cron-stuck task as failed and enqueue a retry task. diff --git a/tests/integration/test_worker_task_resilience.py b/tests/integration/test_worker_task_resilience.py index d6a694d..cc23637 100644 --- a/tests/integration/test_worker_task_resilience.py +++ b/tests/integration/test_worker_task_resilience.py @@ -41,14 +41,6 @@ def test_concurrent_assign_only_one_succeeds(db_session): assert task.assigned_cron_id == "cron-A" -@pytest.mark.xfail( - reason=( - "Known race: assign_task lacks atomic CAS — two sessions can both read " - "status=pending and both commit. Tracked in .scratch/v111-architecture-scan.md " - "discoveries. Fix requires atomic UPDATE ... WHERE status=pending in assign_task." - ), - strict=False, -) def test_concurrent_assign_via_two_sessions(tmp_db_path, db_session): task = IntelligentEvalTaskQueueDB( eval_id="eval1", status="pending", priority=1, reason="slot_due"