Fix: stop one DB failure from ending the whole monitor - #679
Open
rhaegar325 wants to merge 1 commit into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
=====================================
Coverage 78.2% 78.2%
=====================================
Files 40 40
Lines 8827 8842 +15
Branches 1665 1665
=====================================
+ Hits 6901 6916 +15
Misses 1591 1591
Partials 335 335
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #627
Problem
A
sqlite3.DatabaseError: database disk image is malformedraised whilereconciling the first of 100 sub-jobs killed the monitor 4m42s into a
24h30m walltime (
Exit Status: 1, not a PBS kill). The other 99 outcomes werenever recorded and had to be recovered by parsing
logs/*/cmor_*.err.Four defects turned one bad row into a total loss. All predate this PR; none
are regressions.
Changes
1.
monitor_loop— per-sub-job guardThe per-job body made two unguarded DB calls (
reconcile_one,get_status).Either raising ended the loop for every remaining sub-job. Each sub-job is
now guarded on its own.
A sub-job that cannot be reconciled is dropped, not retried every poll:
anything that gets past
TaskTracker's retry wrapper is not transient, and apermanently broken DB would otherwise pin the loop until walltime. Its
variable, job id, PBS state and exit status go to stderr, so the outcome
survives even when the DB write does not.
2.
monitor_main—finalize_monitormoved into afinallyfinalize_monitorsat aftermonitor_loopin thetrybody, so the onefunction written to reclassify exactly these lost-write rows never ran. It now
runs on every exit path, wrapped so its own failure cannot mask the exception
already on its way out.
Side effect: it also runs on the
SystemExitfromshutdown_handler, whichremoves the sidecar that otherwise blocks the output directory permanently.
3.
finalize_monitor— per-variable guardWithout this, change 2 fixes nothing. A corrupt DB is not uniformly
corrupt. Measured against the real database from the incident:
The unguarded sweep died on the first bad row — reachable or not, it would
have reclassified nothing, written no report, and left the sidecar behind.
Guarded per variable, the same database now yields:
unreadableis a new summary field; unreadable variables are named on stderr.4.
TaskTracker._execute_with_retry/_init_db— correct exception classBoth caught
sqlite3.OperationalError, a subclass ofDatabaseError, so"database disk image is malformed"passed straight through the wrapper. Nowcaught as
DatabaseError. The_TRANSIENTmessage check is unchanged, so theretry path behaves identically; only the blind spot for non-
OperationalErrorDB failures is closed.
Need further investigate and update related to DB and lustre system
Gadi mounts every Lustre filesystem with
localflock:localflockmakesfcntl/flocknode-local: they are not coordinatedacross nodes. SQLite's entire concurrency model rests on POSIX advisory locks,
so ~100 workers on ~100 different compute nodes can each hold an "EXCLUSIVE"
lock on the same file and write simultaneously.
Evidence from the incident database:
100 rows, 86 still readable. All 100 workers ran 13:10:14–13:14:40, ~90 of
them writing within the first 60 seconds; the monitor hit the damaged page at
13:10:44.
A second run (
1pctCO2-01, 66 nodes) shows the milder form of the same fault:integrity_check: ok, but 14 of 100start_timevalues silently lost. SQLitewrites whole pages, not rows — 99 rows share one 4096-byte leaf page, so
two nodes read the page, each edit their own row, and the later write-back
discards the earlier one. Reproduced with 20 writers over strictly disjoint
rows under
nolock=1: 6–10% of writes lost,integrity_checkstillok.What still needs to change
Retries cannot help and
synchronous=FULLcannot help — the fault is locking,not durability. The only real fix is to remove concurrent cross-node writes:
per-variable
logs/<var>/status.json; the monitor ingests these each polland is the sole writer.
--append-variablewritesadd_task+enqueue_monitor_requestfrom the login node while themonitor is running (
batch_cmoriser.py:1381-1397) — it needs a file-basedqueue.
qc/backfill_compliance.pyneeds the same convention.synchronous=OFF(tracking.py:60) can go back toNORMAL; it wasonly turned off to dodge fsync EIO under concurrency.
This is deliberately a separate PR: it changes the worker template, so every
sub-job script changes and it needs a real multi-node PBS run to validate.
Landing it on top of this PR means its new failure modes are already guarded.