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>
5.9 KiB
| id | status | status_updated | status_note | component | severity | timestamp | last_seen | occurrences | files | tags | related | legacy | |||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| bug-1466 | fixed | 2026-09-24 | controller | unknown | 2026-07-24T15:36:56.329888Z | 2026-09-19T09:00:00Z | 1 |
|
|
true |
bug-1466: test_config_repl.c: test_committed_marker_survives_checkpoint_trim and…
Description
test_config_repl.c: test_committed_marker_survives_checkpoint_trim and test_commit_advance_retry_clears_push_before_commit_fence flaked/failed (persist_checkpoint returned -EBUSY; new_mlsn <= committed_at_old) after wiring config-upgrade-snapshot task 5's WAL_CONFIG_MARKER apply case
Environment
Not recorded (migrated from buglog.json).
Reproduction
Not recorded (migrated from buglog.json).
Root cause
Two related findings from wiring apply_committed_entry's new WAL_CONFIG_MARKER case (persist.c, config-upgrade-snapshot task 5), which for the first time makes applying a config marker synchronously trigger a real checkpoint: (1) persist_apply_pump_kick's dispatch is a CAS race between the calling thread (inside commit_submit's synchronous commit_resolve_pending) and persist_start's independent fsync_worker thread (via commit_on_durable_advance) -- when the fsync thread wins, commit_submit's WAIT mode returns having waited only for COMMIT, not for the marker's now-real APPLY+CHECKPOINT side effect on that other thread, so a fixture's own subsequent persist_checkpoint()/LSN bookkeeping can race the still-in-flight background checkpoint. This race was always latent (applying a marker used to be a no-op) but was invisible until task 5 gave it a real, observable side effect. (2) DEEPER pre-existing architectural finding: persist_write_checkpoint_marker (called by any checkpoint, periodic or marker-triggered) appends its local-only, never-replicated WAL_CHECKPOINT entry by incrementing persist_t's current_lsn directly, WITHOUT going through ha_primary_log_append/raft_log_append -- this violates ha_primary_log_append's own documented co-tracking invariant 'p->current_lsn == s->log_len + 1 always holds' (ha.c:4057). Consequence: config_append_marker's returned lsn (sourced from raft's own s->log_len, which under-counts once a checkpoint has interleaved) becomes unreliable as an exact physical-position identifier for any marker appended after an earlier marker's checkpoint has run -- exactly the scenario Requirement 4.4 (overlapping markers) requires. Verified this does NOT corrupt on-disk data or commit/durability correctness: commit_advance_locked's majority_candidate_locked (commit.c) is driven entirely by persist_t's own durable_lsn/current_lsn, never by raft's s->log_len (that library value is documented as advisory-only for commit-ceiling purposes, ha-term-safety commit-ceiling-optA). The bug is confined to callers trusting the RETURNED lsn from config_append_marker/ha_primary_log_append for precise identity/ordering once a checkpoint has occurred earlier in the WAL. This is pre-existing (the periodic WAL-threshold checkpoint trigger has always had the same bypass) but was never observable before because periodic checkpoints require >=100 WAL records to fire, essentially never happening between two closely-spaced marker appends in a unit test; task 5's marker-triggered checkpoint fires on EVERY marker commit, making it the common case. SEVERITY CORRECTION (2026-07-24, kiro-review of task 5): the original "advisory-only... does NOT affect correctness" framing UNDERSTATES real risk. ha->raft.log_len (the same value as s->log_len) is a direct input to Raft RequestVote freshness comparisons (ha.c:258-261, my_last_log_index; ha.c:401-406, my_last_log_term via persist_entry_term(c, log_len)) -- NOT debug/advisory-only. ha_primary_log_append itself documents (ha.c:4054-4060) that its own LSN-assignment correctness RELIES on p->current_lsn == s->log_len + 1 holding; once a checkpoint marker breaks that invariant, the next primary append receives a library-assigned index that may no longer match its true physical WAL position, so persist_entry_term() lookups keyed by that index can resolve to the wrong WAL entry. Confirmed via kiro-review independent trace, not yet reproduced as a live failure. Pre-existing (not introduced by task 5 -- the periodic WAL-threshold checkpoint trigger already had this bypass; task 5 only made the interleaving common instead of rare). Still correctly out of config-upgrade-snapshot task 5's boundary to fix -- flagging with corrected severity for whoever next owns ha-term-safety/raft-log-replication so it is not deprioritized based on the original advisory-only framing.
Fix
(1) Added t42_wait_applied() (mirrors test_apply_pump.c's tap_wait_applied) and called it from t43_commit_marker and directly in test_committed_marker_survives_checkpoint_trim, polling persist_last_applied_lsn() before any LSN-sensitive step following a marker commit, closing the fsync-thread race. (2) Changed t43_append_marker_uncommitted to return the TRUE physical lsn read directly from persist_t (c->persist.current_lsn - 1) instead of trusting config_append_marker's returned (raft-library-derived, now-unreliable) lsn -- matching how production code that needs precise positioning (persist.c's apply seam, which receives lsn from the WAL entry's own on-disk header) already effectively does it. Did NOT attempt to fix the deeper co-tracking invariant gap in ha.c/raft.c (out of config-upgrade-snapshot task 5's scope -- would require raft-log-replication/ha-term-safety spec owners to add a log_len resync after any local-only checkpoint-marker append on a raft leader); flagged as a follow-up finding in the task 5 status report instead.
Verification
Not recorded (migrated from buglog.json).
References
Not recorded (migrated from buglog.json).