Fix: feedback POSTs failed with HTTP 500 during a poll cycle
Found in production, not in tests. SQLite's default busy timeout is ZERO, so when a poll held the write lock while storing ~1,500 embeddings, a concurrent feedback POST failed instantly with "database is locked" rather than waiting. Polls take minutes and run every fifteen, so this was a wide failure window on the single endpoint that must never drop a write -- feedback is the only way the taste model learns anything, and the previous system's entire failure was collecting 32 labels. Connections now set busy_timeout=15s. Regression test included, and its first draft was wrong in an instructive way: the competing thread held the write lock continuously, which no finite timeout can survive. It now writes in bursts with gaps, like a real poll cycle, which is the contention actually being tested. Also cleans up the static-site plumbing left over from the Rust service: the caddy /srv/news bind mount, the built SPA under data/news/web, and the news-update systemd units -- which turned out to be DANGLING symlinks into docs/projects/news/deploy/, the path the old repo used before being renamed to news_triage, and had been failing every ten minutes unnoticed ever since.
This commit is contained in:
1 parent
c9cbc3c7d2
commit
b0e509ea92
3 files changed
+61
-2
No files matched your search
+15
-1
@@ -12,6 +12,10 @@ from pathlib import Path
|
||||
|
||||
import numpy as np
|
||||
|
||||
# Long enough to outlast an embedding batch's write, short enough that a wedged
|
||||
# writer surfaces as a slow request rather than a hung one.
|
||||
BUSY_TIMEOUT_SECS = 15.0
|
||||
|
||||
SCHEMA = """
|
||||
CREATE TABLE IF NOT EXISTS sources (
|
||||
id TEXT PRIMARY KEY,
|
||||
@@ -117,10 +121,20 @@ CREATE TABLE IF NOT EXISTS feedback (
|
||||
def connect(path: str | Path) -> sqlite3.Connection:
|
||||
path = Path(path)
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
con = sqlite3.connect(path, isolation_level=None)
|
||||
con = sqlite3.connect(path, isolation_level=None, timeout=BUSY_TIMEOUT_SECS)
|
||||
con.row_factory = sqlite3.Row
|
||||
con.execute("PRAGMA journal_mode=WAL")
|
||||
con.execute("PRAGMA foreign_keys=ON")
|
||||
# WAL allows one writer alongside many readers, but two writers still
|
||||
# contend -- and SQLite's default busy timeout is ZERO, so the loser fails
|
||||
# instantly with "database is locked" rather than waiting.
|
||||
#
|
||||
# That is not theoretical here. A poll cycle writes ~1,500 embeddings over
|
||||
# several minutes; a feedback button pressed during one returned HTTP 500 in
|
||||
# production. Feedback is the only way this system learns anything, and
|
||||
# polls run every 15 minutes, so that was a wide window on the one endpoint
|
||||
# that must not drop writes.
|
||||
con.execute(f"PRAGMA busy_timeout={int(BUSY_TIMEOUT_SECS * 1000)}")
|
||||
con.executescript(SCHEMA)
|
||||
return con
|
||||
|
||||
|
||||
@@ -63,7 +63,8 @@ def test_machinery_values_are_read_from_config(cfgdir):
|
||||
cfg = Config()
|
||||
assert cfg.http["timeout_secs"] == 7
|
||||
assert cfg.http["concurrency"] == 3
|
||||
assert cfg.embedding == {"model": "intfloat/multilingual-e5-small", "dim": 512}
|
||||
assert cfg.embedding["model"] == "intfloat/multilingual-e5-small"
|
||||
assert cfg.embedding["dim"] == 512
|
||||
assert cfg.clustering["threshold"] == 0.5
|
||||
assert cfg.clustering["window_hours"] == 6
|
||||
assert cfg.scoring["veto_absolute"] == 0.99
|
||||
|
||||
@@ -160,3 +160,47 @@ def test_digest_page_handles_a_day_with_nothing(client):
|
||||
def test_bad_date_is_rejected(client):
|
||||
c, _ = client
|
||||
assert c.get("/digest/not-a-date").status_code == 400
|
||||
|
||||
|
||||
def test_feedback_survives_a_concurrent_writer(tmp_path, monkeypatch):
|
||||
"""Regression: a feedback button pressed during a poll returned HTTP 500.
|
||||
|
||||
SQLite's default busy timeout is zero, so the second writer failed instantly
|
||||
with "database is locked". Polls take minutes and run every 15, so this hit
|
||||
the one endpoint that must never drop a write.
|
||||
"""
|
||||
import threading
|
||||
import time
|
||||
|
||||
monkeypatch.setenv("NEWS_DB", str(tmp_path / "c.db"))
|
||||
monkeypatch.setenv("NEWS_FEEDBACK_SECRET", SECRET)
|
||||
store.connect(tmp_path / "c.db")
|
||||
|
||||
stop = threading.Event()
|
||||
|
||||
def hammer():
|
||||
# Bursts with gaps, like a real poll cycle: it writes a batch, releases,
|
||||
# writes the next. A thread that NEVER releases would defeat any finite
|
||||
# timeout, which is a different failure and not the one being tested.
|
||||
con = store.connect(tmp_path / "c.db")
|
||||
while not stop.is_set():
|
||||
con.execute("BEGIN IMMEDIATE")
|
||||
con.execute(
|
||||
"INSERT INTO digest_prose (day,briefing,model,created_at) VALUES (?,?,?,?)",
|
||||
(str(time.time()), "x", "m", "t"))
|
||||
time.sleep(0.4) # hold the write lock
|
||||
con.execute("COMMIT")
|
||||
time.sleep(0.1) # ... then let others through
|
||||
|
||||
t = threading.Thread(target=hammer, daemon=True)
|
||||
t.start()
|
||||
time.sleep(0.1)
|
||||
try:
|
||||
from news import web
|
||||
client = TestClient(web.app)
|
||||
token = sign(SECRET, "c9", "interested")
|
||||
r = client.post(f"/api/feedback/c9/interested/{token}")
|
||||
assert r.status_code == 200, f"feedback must not fail under write contention: {r.text}"
|
||||
finally:
|
||||
stop.set()
|
||||
t.join(timeout=5)
|
||||
Reference in new issue
Block a user