Rongsong Shen
d26ff8870d
The buglog was one 27K-line JSON file whose fields covered only symptom,
root cause and fix, as free text: 2% of entries named the code version a
bug was found on, 12% the build profile, 36% a way to reproduce, and 58% of
the file was OpenWolf hook auto-detections ("Missing guard clause",
"Function not marked async" on .md files).
- .claude/skills/buglog/bug-record.md: the record standard. Eight
sections (Summary as the title, Description, Environment, Reproduction;
Root cause, Fix, Verification, References once fixed), each with the
content it must contain -- e.g. Environment needs the OS, the libc the
failing binary loads (from its ELF interpreter: glibc 2.42 in the nix
shell vs 2.40 on the host), toolchain, build profile and commit.
- .wolf/bugs/<id>.md: 500 records migrated from buglog.json; every field
verified to round-trip. They carry legacy: true and an explicit "not
recorded" where the old log had no such field; component derived from
file paths, severity from keywords (else unknown), fix_commits from
hashes in status_note/fix that git resolves (91 of 463 fixed).
- .wolf/buglog-archive.json: the 710 hook auto-detections, verbatim.
- .wolf/buglog.json is now an index generated from the records, keeping
the fields OpenWolf's hooks and `openwolf bug search` read.
- scripts/buglog.py rewritten: new (template + Environment collected from
the machine and --binary), check, status (refuses a status the record
doesn't support; fixed needs Root cause, Fix, Verification and
--commit hashes git resolves), note, recur, set, show, list, triage,
env, index. 29 unittest cases in scripts/tests/test_buglog.py.
- scripts/git-hooks/pre-commit runs `check --staged`: incomplete staged
records, a legacy record hand-changed to fixed without the fix-time
sections, a new record claiming legacy: true, or a stale index refuse
the commit.
- scripts/gitea-sync.py reads the records: issue body = the record's
sections plus linked fix commits (origin remote, GITEA_COMMIT_REPO to
override), and a body hash so an issue's text is refreshed when the
record changes.
- bug-1866 rewritten to the full standard as the worked example.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
6.7 KiB
| id | status | status_updated | status_note | component | severity | timestamp | last_seen | occurrences | files | tags | related | legacy | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| bug-1694 | fixed | 2026-09-24 | controller | unknown | 2026-08-25T00:00:00.000Z | 2026-09-19T09:00:00Z | 1 |
|
|
|
true |
bug-1694: xce-audit-invariants finding (dimension 1, async I/O thread-role): checkpoint_task_fn…
Description
xce-audit-invariants finding (dimension 1, async I/O thread-role): checkpoint_task_fn (src/controller/persist.c) was the last delaymgr-triggered path still calling its full blocking body (persist_write_checkpoint_marker's pwrite+fdatasync, write_snapshot()'s unbounded open/write walk over every job/queue/host frame held under apply_pump_lock, trim_wal_segments, rotate_snapshots' unlink) directly on the delaymgr timer thread -- the same single thread that drives heartbeats and election timeouts cluster-wide (ha-async-io.md thread-role contract: delaymgr must only trigger work, never block). Triggered from the WAL-append hot path via schedule_checkpoint()/delay_task() whenever wal_record_count crosses max_wal_records. Every sibling long-blocking-I/O path (snapshot send, config-push, reconnect) had already been offloaded by raft-drive-async; checkpoint was the one documented remaining gap.
Environment
Not recorded (migrated from buglog.json).
Reproduction
Not recorded (migrated from buglog.json).
Root cause
checkpoint_task_fn called persist_checkpoint() inline with no offload mechanism at all -- unlike every other delaymgr-triggered blocking path in the codebase. Investigated whether the obvious fix (vtask_enq_classified(pool, task, VTASK_LIVE_BLOCK), the pattern used by persist_apply_pump_kick/commit.c's dispatch_continuation) was safe here: it is NOT -- persist_apply_pump_run() (which ALSO acquires apply_pump_lock, across its entire per-entry walk) is itself dispatched onto that SAME vtaskmgr N-thread pool via persist_apply_pump_kick()'s vtask_enq_classified(..., VTASK_LIVE_BLOCK). Putting checkpoint's genuinely unbounded write_snapshot() (held under apply_pump_lock) on the identical N-thread pool would risk a worker thread parked on apply_pump_lock for the whole snapshot-write duration, starving the pool's liveness-critical capacity (commit continuations, apply-pump drains) -- exactly the failure mode ha-async-io.md's own table entry warns about ("vtaskmgr must NOT do: long/unbounded blocking I/O (snapshot transfer, config-push) -- starves the pool"). The correct destination, per the codebase's OWN established distinction (re-verified by reading all 4 reference sites plus grepping every offload flag), is the SLOW LANE (slowlane_submit) -- the same mechanism snapshot-send (ha_snapshot_do_send -> snapshot_lane_fn, g_drive_async_snapshot_offload, default ON) and config-push (configpush_lane_fn, g_drive_async_configpush_offload, default ON) already use, and NOT the vtask pool. Confirms the task brief's suspicion that checkpoint was harder than a simple vtask wrap -- it's a slow-lane fix, not a vtask-pool fix.
Fix
FIXED. checkpoint_task_fn is now trigger-only: it releases the checkpoint_in_progress CAS slot, then submits the real work (via a new run_checkpoint_body()/checkpoint_lane_fn()) to the cluster's slow lane (slowlane_submit(&persist->cluster->slowlane, key=1, checkpoint_lane_fn, persist)), mirroring snapshot_lane_fn/configpush_lane_fn/catchup_lane_fn/demotion_lane_fn exactly. New feature flag g_persist_checkpoint_offload (persist_set_checkpoint_offload_for_test/persist_checkpoint_offload_enabled, persist.h/persist.c) defaults ON (1) -- unlike the still-experimental catchup/demotion offloads (default OFF pending an unrelated real test-fixture hang, bug-1429), this reuses an ALREADY-PROVEN-SAFE call: persist_checkpoint() already runs on this same slow lane in production today via ha_snapshot_do_send()->snapshot_lane_fn (snapshot offload also default ON), so apply_pump_lock's acquisition from a slow-lane thread is exercised in production every day, not new surface. persist_checkpoint()'s own atomic_exchange guard (checkpoint_in_progress) remains the single-flight barrier preventing double-run with the snapshot-send path, exactly as it already does today. Verified no existing controller_unit_test fixture depends on the delaymgr trigger path completing synchronously (test_ha_core_safety.c deliberately pins max_wal_records=1u<<30 specifically to prevent ever exercising it in a live-cluster fixture) -- ruling out the catchup/demotion-class 'uninitialized slow lane + a synchronous downstream waiter hangs' hazard, since checkpoint's trigger is pure fire-and-forget. Inline-fallback + WARN on genuine slowlane_submit failure only (not -EAGAIN/-EEXIST, both non-failure per slowlane's own documented contract) -- a scheduled checkpoint is never silently dropped. New regression suite test_persist_checkpoint_offload.c (3 tests, mirrors test_ha_liveness.c's reconnect/configpush offload gate-probe pattern): test_checkpoint_offload_trigger_returns_promptly (GREEN: trigger returns <200ms while the real body is observably parked on a slow-lane worker thread, not the caller's), test_checkpoint_inline_blocks_delaymgr_thread_RED (flag OFF reproduces the pre-fix inline-blocking behavior on demand, permanent regression guard), test_checkpoint_no_cluster_runs_inline (defensive NULL-cluster fallback). RED-phase independently confirmed by temporarily forcing checkpoint_task_fn's pre-fix always-inline behavior (a one-line if (1) || probe) and rebuilding: the GREEN test then hung indefinitely (process had to be kill -9'd, PID confirmed via ps as the stuck controller_unit_test binary) -- proving the test genuinely detects the violation, not a tautology. Probe reverted (git diff --stat confirmed clean re-application of the real fix) and the full suite re-run GREEN: 1184 Tests, identical 2 pre-existing failures both runs (test_conf_trim_keeps_only_n_versions Expected 2 Was 3, test_conf_get_saved_tag_no_config Expected 0 Was , test_config_repl.c, both matching bug-1561's well-documented overlayfs fdatasync-EIO artifact signature exactly -- 0 NEW failures. Live HA smoke test (real checkpoint trigger against a running deployed cluster under load) was scoped out given budget already spent on the lock-order investigation, which was this fix's primary risk -- flagged explicitly in the final report rather than silently skipped, as a reasonable follow-up soak.
Verification
Not recorded (migrated from buglog.json).
References
- Related: bug-1429