Fix scrape submission race condition allowing duplicate background tasks
Two rapid POST /scrape requests could both pass the lock check before either background task acquired SCRAPE_LOCK, queuing two sequential scrapes for the same date range. The second would hit Cloudflare after the first already succeeded, causing repeated retries. _SCRAPE_PENDING is set on submission and cleared when the task starts, closing the gap between the 409 check and lock acquisition. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
13618e4471
commit
b47258d47b
1 changed files with 13 additions and 8 deletions
|
|
@ -10,6 +10,8 @@ from sqlalchemy import text
|
||||||
from pydantic import BaseModel
|
from pydantic import BaseModel
|
||||||
import logging
|
import logging
|
||||||
|
|
||||||
|
import threading
|
||||||
|
|
||||||
from database import get_db, SyncSessionLocal
|
from database import get_db, SyncSessionLocal
|
||||||
from auth import get_current_user
|
from auth import get_current_user
|
||||||
from services import proxy as proxy_util
|
from services import proxy as proxy_util
|
||||||
|
|
@ -17,6 +19,11 @@ from services import proxy as proxy_util
|
||||||
router = APIRouter()
|
router = APIRouter()
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
# Set when a background task is submitted; cleared when the task starts executing.
|
||||||
|
# Prevents duplicate scrapes caused by the race between the 409 check and the
|
||||||
|
# background task acquiring SCRAPE_LOCK.
|
||||||
|
_SCRAPE_PENDING = threading.Event()
|
||||||
|
|
||||||
|
|
||||||
# ============================================
|
# ============================================
|
||||||
# REQUEST/RESPONSE MODELS
|
# REQUEST/RESPONSE MODELS
|
||||||
|
|
@ -358,6 +365,8 @@ def run_scrape_sync(from_date: date, to_date: date):
|
||||||
import asyncio
|
import asyncio
|
||||||
from services.booking_scraper import run_manual_scrape, cleanup_stale_batches
|
from services.booking_scraper import run_manual_scrape, cleanup_stale_batches
|
||||||
|
|
||||||
|
_SCRAPE_PENDING.clear() # Task is now running — allow new submissions to queue
|
||||||
|
|
||||||
db = SyncSessionLocal()
|
db = SyncSessionLocal()
|
||||||
try:
|
try:
|
||||||
# Clean up any stale batches before starting
|
# Clean up any stale batches before starting
|
||||||
|
|
@ -407,17 +416,13 @@ async def trigger_manual_scrape(
|
||||||
if not location_result.fetchone():
|
if not location_result.fetchone():
|
||||||
raise HTTPException(status_code=400, detail="No scrape location configured. Set location first.")
|
raise HTTPException(status_code=400, detail="No scrape location configured. Set location first.")
|
||||||
|
|
||||||
# Only one scrape at a time — concurrent Chromium runs cause the page
|
# Only one scrape at a time — concurrent Chromium runs cause page timeouts.
|
||||||
# timeouts that produce partial results
|
# _SCRAPE_PENDING guards the window between submission and lock acquisition.
|
||||||
#
|
|
||||||
# Best-effort early 409: the background task re-acquires the lock and will
|
|
||||||
# no-op (logging "another scrape is running") if it loses a millisecond-
|
|
||||||
# window race, so no double-run can slip through here.
|
|
||||||
from services.booking_scraper import get_lock_status
|
from services.booking_scraper import get_lock_status
|
||||||
if get_lock_status()["locked"]:
|
if get_lock_status()["locked"] or _SCRAPE_PENDING.is_set():
|
||||||
raise HTTPException(status_code=409, detail="A scrape is already running. Try again when it finishes.")
|
raise HTTPException(status_code=409, detail="A scrape is already running. Try again when it finishes.")
|
||||||
|
|
||||||
# Start background task
|
_SCRAPE_PENDING.set()
|
||||||
background_tasks.add_task(run_scrape_sync, from_date, to_date)
|
background_tasks.add_task(run_scrape_sync, from_date, to_date)
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue