mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-11-15 16:28:11 +00:00
fix(db): wait up to 30 s for a busy writer instead of SQLite's 5 s default (#1539)
The users, download history and activity view-state databases open their connections with SQLite's default busy timeout of 5 seconds. A start-up migration, the restart reconcile, or any statement that overlaps a backup or a second process raised "database is locked" after 5 seconds, and the reconcile finished only part of its work. One shared `DB_BUSY_TIMEOUT_SECONDS` (30) in `core/utils.py`, used by all three. The test reads SQLite's `busy_timeout` pragma back from each connection, so it doesn't need a real lock fight. I left `UserDB.initialize()` without a guard on purpose. With 30 seconds the wait covers a backup, and swallowing a schema failure there would hide a worker that can't run. Found in a Claude pass over 1.5.0.
This commit is contained in:
1 parent
2d9491e063
commit
f560b31ee4
5 files changed
+38
-4
No files matched your search
@@ -7,6 +7,7 @@ import threading
|
||||
from typing import Any
|
||||
|
||||
from shelfmark.core.request_helpers import now_utc_iso
|
||||
from shelfmark.core.utils import DB_BUSY_TIMEOUT_SECONDS
|
||||
|
||||
VALID_ACTIVITY_ITEM_TYPES = frozenset({"download", "request"})
|
||||
ADMIN_VIEWER_SCOPE = "admin:shared"
|
||||
@@ -82,7 +83,7 @@ class ActivityViewStateService:
|
||||
self._lock = threading.Lock()
|
||||
|
||||
def _connect(self) -> sqlite3.Connection:
|
||||
conn = sqlite3.connect(self._db_path)
|
||||
conn = sqlite3.connect(self._db_path, timeout=DB_BUSY_TIMEOUT_SECONDS)
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA foreign_keys = ON")
|
||||
return conn
|
||||
|
||||
@@ -16,7 +16,7 @@ from shelfmark.core.request_helpers import (
|
||||
normalize_optional_text,
|
||||
now_utc_iso,
|
||||
)
|
||||
from shelfmark.core.utils import transform_cover_url
|
||||
from shelfmark.core.utils import DB_BUSY_TIMEOUT_SECONDS, transform_cover_url
|
||||
|
||||
logger = setup_logger(__name__)
|
||||
|
||||
@@ -100,7 +100,7 @@ class DownloadHistoryService:
|
||||
self._lock = threading.Lock()
|
||||
|
||||
def _connect(self) -> sqlite3.Connection:
|
||||
conn = sqlite3.connect(self._db_path)
|
||||
conn = sqlite3.connect(self._db_path, timeout=DB_BUSY_TIMEOUT_SECONDS)
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA foreign_keys = ON")
|
||||
return conn
|
||||
|
||||
@@ -21,6 +21,7 @@ from shelfmark.core.request_validation import (
|
||||
validate_request_level_payload,
|
||||
validate_status_transition,
|
||||
)
|
||||
from shelfmark.core.utils import DB_BUSY_TIMEOUT_SECONDS
|
||||
|
||||
logger = setup_logger(__name__)
|
||||
|
||||
@@ -194,7 +195,7 @@ class UserDB:
|
||||
return self._db_path
|
||||
|
||||
def _connect(self) -> sqlite3.Connection:
|
||||
conn = sqlite3.connect(self._db_path)
|
||||
conn = sqlite3.connect(self._db_path, timeout=DB_BUSY_TIMEOUT_SECONDS)
|
||||
conn.row_factory = sqlite3.Row
|
||||
conn.execute("PRAGMA foreign_keys = ON")
|
||||
return conn
|
||||
|
||||
@@ -18,6 +18,11 @@ if TYPE_CHECKING:
|
||||
|
||||
logger = setup_logger(__name__)
|
||||
|
||||
# How long an app database connection waits for another writer before it raises. SQLite's
|
||||
# own default is 5 s, which a start-up migration or a restart reconcile can outlast when a
|
||||
# backup or a second process holds the file.
|
||||
DB_BUSY_TIMEOUT_SECONDS = 30
|
||||
|
||||
|
||||
def normalize_http_url(
|
||||
url: str | None,
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
"""The app databases wait out a busy writer instead of failing at SQLite's 5 s default."""
|
||||
|
||||
import pytest
|
||||
|
||||
from shelfmark.core.activity_view_state_service import ActivityViewStateService
|
||||
from shelfmark.core.download_history_service import DownloadHistoryService
|
||||
from shelfmark.core.user_db import UserDB
|
||||
|
||||
# SQLite's own default is 5000 ms. A start-up migration or a restart reconcile that meets a
|
||||
# writer for a few seconds should wait, not raise.
|
||||
_MIN_BUSY_TIMEOUT_MS = 15_000
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"service_cls",
|
||||
[UserDB, DownloadHistoryService, ActivityViewStateService],
|
||||
)
|
||||
def test_app_database_connections_wait_for_a_busy_writer(tmp_path, service_cls):
|
||||
service = service_cls(str(tmp_path / "users.db"))
|
||||
|
||||
conn = service._connect()
|
||||
try:
|
||||
busy_timeout_ms = conn.execute("PRAGMA busy_timeout").fetchone()[0]
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
assert busy_timeout_ms >= _MIN_BUSY_TIMEOUT_MS
|
||||
Reference in new issue
Block a user