lohitkolluri commented on code in PR #67900:
URL: https://github.com/apache/airflow/pull/67900#discussion_r3353471965
##########
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:
You're right that the retry loop added complexity. I've removed it entirely
— both create_backfill and create_backfill_dry_run now catch OperationalError
directly and return 503 with no retries. Much cleaner, thanks for the
suggestion.
##########
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:
Agreed. Since the retry loop is removed, the test now just verifies that
OperationalError with 'database is locked' → HTTP 503. The call_count assertion
is gone — the mock is minimal and the endpoint behavior is what matters.
--
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]
