23tae commented on code in PR #67900:
URL: https://github.com/apache/airflow/pull/67900#discussion_r3353378453
##########
airflow-core/src/airflow/api_fastapi/core_api/routes/public/backfills.py:
##########
@@ -239,18 +254,33 @@ def create_backfill(
session,
fallback=True,
)
+ backfill_obj = None
try:
- backfill_obj = _create_backfill(
- dag_id=backfill_request.dag_id,
- from_date=from_date,
- to_date=to_date,
- max_active_runs=backfill_request.max_active_runs,
- reverse=backfill_request.run_backwards,
- dag_run_conf=backfill_request.dag_run_conf,
- triggering_user_name=user.get_name(),
- reprocess_behavior=backfill_request.reprocess_behavior,
- run_on_latest_version=resolved_run_on_latest,
- )
+ for _attempt in range(_MAX_SQLITE_LOCK_RETRIES):
+ try:
+ backfill_obj = _create_backfill(
+ dag_id=backfill_request.dag_id,
+ from_date=from_date,
+ to_date=to_date,
+ max_active_runs=backfill_request.max_active_runs,
+ reverse=backfill_request.run_backwards,
+ dag_run_conf=backfill_request.dag_run_conf,
+ triggering_user_name=user.get_name(),
+ reprocess_behavior=backfill_request.reprocess_behavior,
+ run_on_latest_version=resolved_run_on_latest,
+ )
+ break
+ except OperationalError as e:
+ if "database is locked" not in str(e).lower():
+ raise
+ if _attempt == _MAX_SQLITE_LOCK_RETRIES - 1:
+ raise HTTPException(
+ status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
+ detail="Database is locked. Backfill creation is not
supported on SQLite "
+ "under concurrent access. Please use PostgreSQL or
MySQL.",
+ )
+ time.sleep(_SQLITE_LOCK_RETRY_DELAY_SECONDS[_attempt])
Review Comment:
This `time.sleep` is unreachable. The loop raises a 503 error on the final
attempt (`_attempt == 2`) before reaching here.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_backfills.py:
##########
@@ -352,6 +353,69 @@ def test_create_backfill_with_depends_on_past(
== "Dag has tasks for which depends_on_past=True. You must
set reprocess behavior to reprocess completed or reprocess failed."
)
+ def test_create_backfill_database_locked(self, session, dag_maker,
test_client):
+ """Regression test for issue #66726: SQLite 'database is locked'
returns 503 after retries."""
+ with dag_maker(session=session, dag_id="TEST_DAG_1", schedule="0 * * *
*") as dag:
+ EmptyOperator(task_id="mytask")
+ session.scalars(select(DagModel)).all()
+ session.commit()
+
+ from_date = pendulum.parse("2024-01-01")
+ to_date = pendulum.parse("2024-02-01")
+
+ data = {
+ "dag_id": dag.dag_id,
+ "from_date": to_iso(from_date),
+ "to_date": to_iso(to_date),
+ "max_active_runs": 5,
+ "run_backwards": False,
+ "dag_run_conf": {},
+ }
+
+ with mock.patch(
+
"airflow.api_fastapi.core_api.routes.public.backfills._create_backfill",
+ side_effect=OperationalError("statement", "params", "database is
locked"),
+ ) as mock_create:
+ response = test_client.post("/backfills", json=data)
Review Comment:
Mocking the entire `_create_backfill` to test the retry loop hides how the
transaction and session behaves during an actual DB lock. It makes the test
effectively meaningless since the actual domain logic is completely bypassed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
