Skip to content

Commit 198714d

Browse files
committed
[feat.db.sigma.D7+] app.job stops being a table nobody writes to, and the first thing it recorded was a rule I had only ever written in comments
The ledger has existed since migration 007 with the state machine, the one-in-flight unique index, v_job_live and reap_orphaned_jobs — and 0 rows, no writer. backend/jobs.py is the writer; field_server calls it around every compute (that file's changes are in 86aff26, swept into a peer commit before I could stage them — the WHY is here and at both call sites). MEASURED, four requests, four correct terminal rows: fields.mep done 0.21 s fields.mep failed UNSUPPORTED "Gasteiger cannot parameterize F/P" (PF6-) fields.qm.homo failed BUDGET "SCF exceeded its 0 s budget" counters at /health: opened 4 · done 1 · failed 3 · write_failed 0 A ledger that only ever records successes is the one that is easiest to lie with, so the failure paths were wired and exercised first. THREE DESIGN POINTS, each of which is a rule this repo already paid for: - A JOB ROW MAY NEVER COST A RESULT. Every failure degrades to "no row" and the field still returns — but NOT silently: the counters are on /health, because a ledger that is quietly not being written looks exactly like a system with no work in it, which is the OFFLINE-vs-EMPTY confusion the ops console exists to refuse. - Rows are inserted as 'running', not 'queued'. In a synchronous service 'queued' would last microseconds, and a state the system cannot be observed in is noise. The state stays valid in the schema for the worker that will use it. - A conflict on job_one_inflight is a MEASUREMENT, not an error: today two identical concurrent requests both compute, so that counter is how often duplicated work actually happens. Guessing that number was never going to justify building the wait-for-result path. AND THE LEDGER IMMEDIATELY CAUGHT SOMETHING. `max_seconds: 0` is documented in four separate comments as "refuse before doing any work, tell me the cost". The HTTP layer had `max(req_seconds, 1.0)`, so zero became one; and run_scf had no pre-flight at all, so the refusal came from the in-loop watchdog after one SCF cycle. The job rows show it: 1.03 s and 1.28 s, error BUDGET. A rule stated in four comments and implemented in none of them is a rule about my intentions. Both fixed: the floor preserves 0 (and clamps negatives to 0 — nonsense is not a request for the default), and run_scf now screens with the model this repo already measured over 47 molecules, 2.8x safety factor included rather than quietly dropped. Zero-budget refusal went 1.30 s -> 0.05 s and now names the estimate and the basis-function count, so the caller knows what to send instead. THE NEW GUARD SHADOWED AN OLD WITNESS, which is the part worth keeping. The in-loop watchdog's test used max_seconds=0.0 — the pre-flight now refuses that before an SCF object exists, so the test went red on a message with no cycle count, and the watchdog it exists to prove was no longer being exercised. A new guard that shadows an older guard's test does not remove the need for the older guard, it removes the EVIDENCE for it, and the suite still reads green everywhere else. Fixed by budgeting 0.05 s against the model's own 0.033 s estimate for benzene (clears the screen, an order of magnitude under a converged SCF), and the pre-flight got a witness of its own. Three more stale expectations repaired, all from another session's real changes, none by weakening the invariant: - the cube-origin units check hardcoded pad=4.0 while the box now GROWS until the contour closes; it reads pad_used_angstrom now, so it tests units instead of padding policy (hardcoding turned every tuning pass into a "unit inversion"); - iso_suggest was replaced by iso_fixed/iso_sized_for; the invariant (the drawn isovalue lies inside the field's range) survives the rename; - the stray-basis-literal scanner convicted MINIMAL_BASES, a NEW and legitimate home for a DIFFERENT fact ("which bases are too small to quote a frontier energy from"). Narrowed by NAME, not by line, so a literal in a function body is still a stray — which is where the original incident lived. Its positive control now plants inside a function, or it would test the exemption. 19/19 physics contracts green (was 17 passed / 2 failed). Also landing the performance probe from earlier today, unrelated but finished: scripts/perf_probe.mjs + docs/PERF.md. Cold load 920 ms measured, bundle 3.14 MB / 902 KB gzip, field-swap medians 189-722 ms across 6 kinds. STOPPED HERE at Ivan's instruction (「暂停先」). Four agents were mid-flight and were killed; their partial files are deliberately NOT in this commit: src/.../facets/{property-cockpit,pharmacophore-designer}/index.ts, src/app/services/ligand-store.ts, scripts/memory_probe.py, docs/probe_runs/. Uncommitted and unverified is the honest state for work that was interrupted.
1 parent 86aff26 commit 198714d

4 files changed

Lines changed: 1409 additions & 14 deletions

File tree

‎backend/jobs.py‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
"""app.job lifecycle writes — the seam that was built empty.
2+
3+
Migration 007 shipped the table, the CHECK-enforced state machine, the
4+
one-in-flight unique index, `app.v_job_live` and `app.reap_orphaned_jobs()`.
5+
Nothing ever wrote to it: `SELECT count(*) FROM app.job` read 0 for as long as it
6+
existed, and a seam with no writer that reads as finished is how a schema rots.
7+
This module is the writer.
8+
9+
WHAT IT DOES NOT DO, stated so nobody reads more into it: compute is still
10+
SYNCHRONOUS per HTTP request. This does not make the service asynchronous and it
11+
does not add a worker. It makes every long compute VISIBLE and RECONNECTABLE
12+
while it runs, which is the precondition for a queue rather than the queue
13+
itself — and per ROADMAP.md P1 the queue then lands as different writers to a
14+
table that already refuses illegal states.
15+
16+
Three properties that are the actual content:
17+
18+
① A JOB ROW MAY NEVER COST A RESULT. Every failure here degrades to "no job
19+
row" and the field still computes and still returns. But NOT silently: the
20+
counters below are exposed at /health and through the ops surface, because a
21+
ledger that is quietly not being written looks exactly like a system with no
22+
work in it — and "the queue is empty" vs "nobody is recording" is the same
23+
OFFLINE-vs-EMPTY confusion the ops console was built to refuse.
24+
25+
② THE STATE MACHINE IS THE SCHEMA'S, NOT MINE. queued has no started_at; a
26+
terminal state has finished_at; failed has an error_code that is not
27+
CANCELLED; done has seconds. Those are CHECK constraints, so a wrong
28+
transition raises instead of storing a lie. This module does not re-implement
29+
them and deliberately does not pre-validate — a duplicated rule drifts, and
30+
the copy is never the one that gets corrected.
31+
32+
③ A CONFLICT ON `job_one_inflight` IS A MEASUREMENT, NOT AN ERROR. That index
33+
makes an identical (method, input, params) impossible to have twice in
34+
flight. Today two identical concurrent requests both compute — nothing
35+
queues, nothing waits — so the conflict is how often that duplicated work
36+
actually happens. It is counted, the request proceeds, and the number is the
37+
evidence for building the wait-for-result path. Guessing at that number was
38+
never going to justify the work.
39+
"""
40+
from __future__ import annotations
41+
42+
import threading
43+
import uuid
44+
from typing import Any, Callable
45+
46+
# Counted rather than logged-and-forgotten. Same reasoning as the persist
47+
# counters: a number nobody can read is a log line with extra steps.
48+
_counters = {
49+
'opened': 0, # rows inserted
50+
'done': 0, # reached state='done'
51+
'failed': 0, # reached state='failed'
52+
'inflight_conflict': 0, # an identical job was ALREADY running (see ③)
53+
'write_failed': 0, # the ledger write itself failed; compute unaffected
54+
'last_error': None, # type: ignore[dict-item]
55+
}
56+
_lock = threading.Lock()
57+
58+
# app.job_error (migration 007) is a SUBSET of contracts/errors.json — it omits
59+
# the codes a job cannot carry. Mapping here rather than at the call site so
60+
# there is one place to look when a new code appears; an unmapped code becomes
61+
# INTERNAL rather than raising, because losing the ROW to an enum mismatch would
62+
# be a worse outcome than losing the code's precision.
63+
_JOB_ERROR = {
64+
'PARSE', 'UNCONVERGED', 'UNPARAMETERIZED', 'BUDGET',
65+
'UNSUPPORTED', 'TOO_LARGE', 'INTERNAL', 'CANCELLED',
66+
}
67+
68+
69+
def counters() -> dict:
70+
with _lock:
71+
return dict(_counters)
72+
73+
74+
def _bump(key: str, error: str | None = None) -> None:
75+
with _lock:
76+
_counters[key] = _counters[key] + 1
77+
if error is not None:
78+
_counters['last_error'] = error
79+
80+
81+
def job_error_code(code: str | None) -> str:
82+
"""Map an envelope error code onto app.job_error, never raising."""
83+
if code in _JOB_ERROR:
84+
return code
85+
return 'INTERNAL'
86+
87+
88+
class JobLedger:
89+
"""Writes job rows through a caller-supplied connection factory.
90+
91+
`connect` is injected rather than imported so this module has no opinion
92+
about how the service connects, and so a test can hand it a factory that
93+
raises — which is the only way to prove property ① instead of asserting it.
94+
"""
95+
96+
def __init__(self, connect: Callable[[], Any], worker: str) -> None:
97+
self._connect = connect
98+
self.worker = worker
99+
100+
# ── lifecycle ────────────────────────────────────────────────────────
101+
def open(self, *, method_row_id: str, input_sha256: bytes, params: dict,
102+
budget_seconds: float | None = None, est_seconds: float | None = None,
103+
compound_id: str | None = None,
104+
conformer_hash: bytes | None = None) -> str | None:
105+
"""Insert a 'running' row and return its id, or None.
106+
107+
Inserted directly as RUNNING with started_at set: in a synchronous
108+
service the row would be 'queued' for microseconds, and a state the
109+
system cannot actually be observed in is noise in the ledger. The queued
110+
state stays valid in the schema for the worker that will use it.
111+
112+
None means "no row" — the DB was unreachable, the write failed, or an
113+
identical job is already in flight. In every case the caller proceeds.
114+
"""
115+
# budget_seconds has a CHECK (> 0), and 0 is a legitimate request here
116+
# ("refuse immediately, tell me the cost"), so 0 must arrive as NULL
117+
# rather than violate the constraint and lose the whole row.
118+
budget = float(budget_seconds) if budget_seconds else None
119+
if budget is not None and budget <= 0:
120+
budget = None
121+
try:
122+
import json
123+
with self._connect() as conn, conn.cursor() as cur:
124+
cur.execute(
125+
"INSERT INTO app.job (method_row_id, state, input_sha256, params, "
126+
" budget_seconds, est_seconds, compound_id, conformer_hash, "
127+
" worker, started_at) "
128+
"VALUES (%s, 'running', %s, %s, %s, %s, %s, %s, %s, now()) "
129+
'RETURNING id',
130+
(method_row_id, input_sha256, json.dumps(params), budget,
131+
est_seconds, compound_id, conformer_hash, self.worker))
132+
row = cur.fetchone()
133+
_bump('opened')
134+
return str(row[0]) if row else None
135+
except Exception as e: # noqa: BLE001
136+
# A unique-violation on job_one_inflight is the DEDUP signal, not a
137+
# fault. Distinguished by SQLSTATE 23505 rather than by message text
138+
# so a locale or a version change cannot turn a measurement into an
139+
# error count.
140+
if getattr(getattr(e, 'sqlstate', None), 'strip', None) and e.sqlstate == '23505':
141+
_bump('inflight_conflict')
142+
elif '23505' in str(getattr(e, 'sqlstate', '')) or 'job_one_inflight' in str(e):
143+
_bump('inflight_conflict')
144+
else:
145+
_bump('write_failed', f'{type(e).__name__}: {e}')
146+
return None
147+
148+
def done(self, job_id: str | None, *, seconds: float,
149+
field_cube_id: str | None = None, peak_rss_mb: int | None = None) -> None:
150+
if job_id is None:
151+
return
152+
try:
153+
with self._connect() as conn, conn.cursor() as cur:
154+
cur.execute(
155+
"UPDATE app.job SET state = 'done', finished_at = now(), "
156+
' seconds = %s, field_cube_id = %s, peak_rss_mb = %s '
157+
"WHERE id = %s AND state = 'running'",
158+
(round(float(seconds), 3), field_cube_id, peak_rss_mb, job_id))
159+
_bump('done')
160+
except Exception as e: # noqa: BLE001
161+
_bump('write_failed', f'{type(e).__name__}: {e}')
162+
163+
def failed(self, job_id: str | None, *, code: str, detail: str,
164+
seconds: float | None = None) -> None:
165+
"""Terminal-failed. `code` is mapped onto app.job_error, never raising.
166+
167+
A CANCELLED code is stored as state='cancelled', because the schema's
168+
job_failed_has_code says failed ⇔ a code that is not CANCELLED — the two
169+
are different facts about the world and the constraint knows it.
170+
"""
171+
if job_id is None:
172+
return
173+
mapped = job_error_code(code)
174+
state = 'cancelled' if mapped == 'CANCELLED' else 'failed'
175+
try:
176+
with self._connect() as conn, conn.cursor() as cur:
177+
cur.execute(
178+
'UPDATE app.job SET state = %s, finished_at = now(), '
179+
' seconds = %s, error_code = %s, error_detail = %s '
180+
"WHERE id = %s AND state = 'running'",
181+
(state, None if seconds is None else round(float(seconds), 3),
182+
mapped, detail[:2000], job_id))
183+
_bump('failed')
184+
except Exception as e: # noqa: BLE001
185+
_bump('write_failed', f'{type(e).__name__}: {e}')
186+
187+
# ── startup ──────────────────────────────────────────────────────────
188+
def reap(self) -> int:
189+
"""Fail every row this worker left in flight when it died.
190+
191+
Without this, a restart leaves rows 'running' forever and
192+
`app.v_job_live`'s age_seconds grows without bound — the ledger would
193+
report work that no process is doing, which is worse than no ledger
194+
because it reads as a hung system.
195+
"""
196+
try:
197+
with self._connect() as conn, conn.cursor() as cur:
198+
cur.execute('SELECT app.reap_orphaned_jobs(%s)', (self.worker,))
199+
row = cur.fetchone()
200+
return int(row[0]) if row else 0
201+
except Exception as e: # noqa: BLE001
202+
_bump('write_failed', f'{type(e).__name__}: {e}')
203+
return 0
204+
205+
206+
def make_worker_name(pid: int, producer_version: str) -> str:
207+
"""Identify the process AND the code it is running.
208+
209+
A bare pid is reusable after a restart, so a reap keyed on it can either
210+
miss rows or claim another process's. Including the producer version means a
211+
reap after a code change cannot silently adopt the previous generation's
212+
in-flight rows as its own.
213+
"""
214+
return f'fields/{pid}/{producer_version}'
215+
216+
217+
def new_uuid() -> str:
218+
return str(uuid.uuid4())

0 commit comments

Comments
 (0)