Retry and report transient-busy LedgerCloseMeta drops during catchup

[tomerweller]

Closes #3801

Summary

crates/history/src/catchup/persist.rs emit_meta persisted the catchup ledger_close_meta row with a bare warn! on failure. A transient SQLITE_BUSY/SQLITE_LOCKED therefore dropped the row silently, and catchup never revisits a ledger — so the hole in the RPC-facing table (getTransactions / getLedgers) was permanent and invisible. #3772 measured 139 lock-loss events in 26 days on the deployed validator with 2 confirmed dropped ledgers.

The write now goes through a bounded transient-busy retry (CATCHUP_META_MAX_ATTEMPTS = 3, 25 ms apart). On exhaustion the write is still abandoned rather than propagated — an error out of emit_meta aborts the whole batch's persist_ledger_history, and the replay retry resumes from the already-advanced in-memory LCL, so propagating would turn a 1-row hole into a ≤64-ledger, 3-table one without repairing the original (structural blocker tracked as #3811). What changes is that the drop is no longer silent: an error! naming the RPC-visible gap, the #3802 counters at site="catchup_meta", and a meta_rows_dropped count carried out on CatchupResult.

Supporting change: the busy/locked predicate moves down to henyey_db::DbError::is_transient_busy, beside its documented sibling is_query_interrupted. crates/history cannot see app's pub(crate) is_transient_db_busy (the dependency runs app → history), so this gives one shared definition of "transient" instead of a second copy that could drift (#3871). crates/app/src/app/persist.rs::is_transient_db_busy becomes a one-line delegate with its pub(crate) visibility and all ~20 call sites unchanged — nothing is widened.

The fd:3 meta callback stays unconditional on every path, exactly as on main: coupling it to the DB outcome would drop frames from the meta stream stellar-rpc/Horizon consume.

Plan reference

Converged Plan, as refined by the crate-boundary resolution comment (option 2: literal counter name at the emit site in crates/history, const + pre-registration in app's catalog; classifier moves to crates/db).

Files changed

• crates/db/src/error.rs — DbError::is_transient_busy() + unit tests.
• crates/app/src/app/persist.rs — is_transient_db_busy delegates; visibility unchanged.
• crates/app/src/metrics.rs — SITE_CATCHUP_META, added to DB_BUSY_SITES and both "site", [...] arrays in metric_catalog!; removed from the "Reserved" doc block; the negative pinning assertion deleted and the pinned array extended.
• crates/history/src/catchup/persist.rs — store_meta_with_busy_retry + MetaStoreOutcome; emit_meta rewired; tests.
• crates/history/src/catchup/mod.rs — meta_rows_dropped: AtomicU32 on CatchupManager (both constructors), meta_rows_dropped() accessor, per-run delta in replay_and_finish.
• crates/history/src/lib.rs — CatchupResult.meta_rows_dropped + Display (mentioned only when non-zero).
• crates/history/Cargo.toml — metrics-exporter-prometheus as a dev-dependency (local recorder for the counter assertions).

Test plan

All run under CARGO_TARGET_DIR=~/data/<session>/do-3801/cargo-target, exit codes captured directly (not through a pipe):

• cargo fmt --all -- --check → rc=0
• cargo clippy --all --all-targets -- -D warnings → rc=0
• cargo test -p henyey-db → rc=0 (125 + 5 passed, 2 ignored)
• cargo test -p henyey-history → rc=0 (567 + 1 + 3 + 7 + 2 + 14 + 1 + 29 passed, 2 ignored)
• cargo test -p henyey-app → rc=0 (1233 + 3 + 1 + 1 + 8 passed, 2 ignored)
• cargo test -p henyey-rpc → rc=0 (233 + 28 + 37 + 1 passed) — reads the affected table
• pre-commit hook (fmt + clippy) ran on the fix commit and passed

Regression test

• Tests:
  • crates/history/src/catchup/persist.rs::test_store_meta_with_busy_retry_retries_transient_busy_then_stores
  • crates/history/src/catchup/persist.rs::test_store_meta_with_busy_retry_gives_up_after_max_attempts_and_counts_drop
  • crates/history/src/catchup/persist.rs::test_store_meta_with_busy_retry_does_not_retry_non_transient
  • crates/history/src/catchup/persist.rs::test_emit_meta_routes_store_through_retry_helper (real DB fault, not the closure seam — stops a correct-but-uncalled helper from passing)
  • crates/history/src/catchup/persist.rs::test_emit_meta_persists_row_and_invokes_callback
  • crates/history/src/lib.rs::test_catchup_result_display_reports_dropped_meta
  • crates/db/src/error.rs::test_is_transient_busy_matches_busy_and_locked, ::test_is_transient_busy_rejects_corrupt_and_non_sqlite
  • crates/app/src/metrics.rs::test_db_busy_site_label_vocabulary_pinned (updated)
• Pre-fix: committed as 189fe99a — verified FAILED:
  • henyey-db: no method named is_transient_busy found for enum DbError
  • henyey-history: cannot find function store_meta_with_busy_retry, cannot find type MetaStoreOutcome, struct CatchupResult has no field named meta_rows_dropped, no method named meta_rows_dropped
  • henyey-app: test_db_busy_site_label_vocabulary_pinned FAILED at runtime — left missing "catchup_meta"
• Post-fix: verified PASSES after 139bb623 (all 31 catchup::persist::tests green, plus the db/app tests above).

Note: 189fe99a was committed with --no-verify because the failing-test commit intentionally does not compile; the fix commit 139bb623 ran the hook normally and passed.

Deviations from plan

• Both the CatchupManager::meta_rows_dropped() accessor (Critic C's minority option) and the CatchupResult.meta_rows_dropped field were implemented, rather than one or the other. The field is the per-run delta from replay_and_finish; the accessor exposes the lifetime total and is what makes the emit_meta-level test assertable without driving a full catchup. Constructor threading was trivial, so nothing was de-scoped.
• The counter names are emitted as raw string literals inside store_meta_with_busy_retry (matching the crates/history/src/catchup/buckets.rs:277 precedent) rather than via a local const, per the crate-boundary resolution comment.
• No PARITY_STATUS.md update: docs/PARITY.md puts Metrics and Logging on the freely-divergeable surface, and ledger_close_meta is a henyey-only table with no stellar-core counterpart. No ledger/bucket hash, result XDR, wire byte, archive format, HTTP or JSON-RPC contract is touched; response completeness strictly improves.

Note for #3806

#3806 (tx_set_gc) is in ready-for-doing and its converged plan touches the same metric_catalog! "site" arrays, the same DB_BUSY_SITES, and the same test_db_busy_site_label_vocabulary_pinned. #3801 was drained first deliberately; #3806 will need to rebase onto this and append tx_set_gc after catchup_meta (its reserved-value assertion is still present and untouched here).

🤖 Generated with Claude Code

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: Cycle 1. The regression tests were verified genuinely failing at the pre-fix commit (189fe99a, rc=101 — one runtime assertion failure plus the expected compile failures), all pass at 139bb623, and the narrow busy/locked classifier survives the crate move byte-for-byte. One non-blocking observation left inline.

Full review

Reviewed at 139bb623 merged with origin/main de2588cf (the PR head is already based on current main — git merge origin/main was a no-op, so nothing here is a stale-base review). Not self-modifying: no .claude/skills/, scripts/lib/, or .github/skills/shared/scripts/ paths.

Test-verification gate — kind: bug-fix (PASS)

The linked issue's ## Triage Report says Kind: bug-fix, so the regression test must provably fail at the pre-fix commit. Verified directly, exit codes captured without a pipe, in ~/data/t267rev/review-pr-3801/reviewer/wt:

At 189fe99a (test-only commit, committed before the fix):

• cargo test -p henyey-app --lib metrics::tests::test_db_busy_site_label_vocabulary_pinned → rc=101, and it is a genuine runtime assertion failure, not a compile error:

  assertion `left == right` failed: the db-busy site vocabulary changed
    left:  [..., "peer_record_update"]
    right: [..., "peer_record_update", "catchup_meta"]
  

• cargo test -p henyey-db --lib error:: → rc=101, E0599: no method named is_transient_busy found for enum DbError.
• cargo test -p henyey-history --lib catchup::persist::tests → rc=101, E0425 store_meta_with_busy_retry, E0433 MetaStoreOutcome, E0560 CatchupResult has no field named meta_rows_dropped, E0599 meta_rows_dropped.

At 139bb623:

• cargo test -p henyey-db --lib error:: → rc=0 (2 passed)
• cargo test -p henyey-history --lib catchup::persist::tests → rc=0 (31 passed)
• cargo test -p henyey-history --lib test_catchup_result_display → rc=0 (1 passed)
• cargo test -p henyey-app --lib metrics::tests → rc=0 (64 passed)

Non-vacuity: the tests are not merely API-existence assertions. test_store_meta_with_busy_retry_retries_transient_busy_then_stores asserts the store closure is invoked twice; on main emit_meta invokes store_ledger_close_meta exactly once inside if let Err(err) = … and warns, so the assertion is the bug. test_emit_meta_routes_store_through_retry_helper uses a real DB fault (DROP TABLE ledger_close_meta) rather than the closure seam, which is what stops a correct-but-uncalled helper from passing the suite.

Classifier move — semantically identical, narrowness preserved (error-handling)

crates/db/src/error.rs:104-116 is character-for-character the same matches! arm as the original at crates/app/src/app/persist.rs:554 on main — DbError::Sqlite(rusqlite::Error::SqliteFailure(rusqlite::ffi::Error { code: DatabaseBusy | DatabaseLocked, .. }, _)). No widening, no extended-code matching, no message-string matching. The load-bearing sentence ("genuine corruption must NEVER be reclassified recoverable") survived the move verbatim, and the doc gained an explicit statement of why it lives on the error type.

test_is_transient_busy_rejects_corrupt_and_non_sqlite pins the negative boundary at seven points: DatabaseCorrupt, SystemIoFailure, Integrity, Xdr, NotFound, QueryBudgetExceeded, and a non-SqliteFailure rusqlite error. That is the right shape for a consensus-safety guard.

crates/app/src/app/persist.rs:560 keeps pub(crate) — not widened to pub/pub(super) — and the &anyhow::Error sibling below it is untouched, so all ~20 call sites and the #3808 record_db_busy_drop_if_transient path are unchanged. No visibility narrowing or dead-code removal anywhere in this diff, so that gate is n/a.

Retry semantics (error-handling) — bounded, correctly classified, no double-count

store_meta_with_busy_retry (crates/history/src/catchup/persist.rs:73-97):

• Bound is finite and small: attempt starts at 1, the retry arm is guarded by attempt < CATCHUP_META_MAX_ATTEMPTS (3), so store() runs at most 3 times. The always-busy test asserts exactly CATCHUP_META_MAX_ATTEMPTS calls, so a future off-by-one turns into a red test rather than a spin.
• Non-transient errors hit the final Err(err) => return MetaStoreOutcome::Failed(err) arm before any sleep or counter — one call, no retry, neither counter touched. test_store_meta_with_busy_retry_does_not_retry_non_transient asserts both the call count and the absence of both series in the render.
• Counter accounting is exact: henyey_db_busy_retry_attempts_total fires once per retried attempt (MAX_ATTEMPTS - 1 = 2 on exhaustion, matching Instrument all 7 crates/app SQLite busy-retry and busy-drop sites #3808's "one per retried attempt, NOT one per episode" HELP text); henyey_db_busy_write_dropped_total fires exactly once, on the abandonment arm only. Both are inside the helper, so policy and telemetry cannot drift apart.
• meta_rows_dropped cannot double-count: emit_meta has exactly three failure arms (DroppedBusy, Failed, to_xdr Err) and each does exactly one fetch_add(1, Relaxed); Stored does none. Nothing increments inside the helper, so a retried-then-stored write contributes zero.
• The per-run delta in replay_and_finish is correct: the meta_dropped_before snapshot at crates/history/src/catchup/mod.rs:626 precedes both emit_meta call sites that can run during the call — the bucket-apply one at mod.rs:646 and the replay one reached via replay.rs:856 — and saturating_sub guards the (practically unreachable) u32 wrap.

Metric vocabulary + test hygiene (test-coverage)

catchup_meta is present in all three required places, which is what test_db_busy_site_series_preregistered_at_zero requires: DB_BUSY_SITES (metrics.rs:333) and both metric_catalog! "site" arrays (metrics.rs:1020 and metrics.rs:1035). A value in the former but missing from either of the latter is not pre-registered and would fail that test — it does not. The reserved-absent assertion for catchup_meta is deleted, not edited; only the tx_set_gc one remains (metrics.rs:2602-2605), which is correct since #3806 has not landed.

Metrics test hygiene is clean: every counter-asserting test in the diff is a synchronous #[test] with the assertions inside metrics::with_local_recorder. No #[tokio::test] anywhere near a counter assertion, which matters because the local recorder is thread-local.

metrics-exporter-prometheus is under [dev-dependencies] in crates/history/Cargo.toml:66, not [dependencies] — the production emit path stays on the global metrics facade with a literal counter name, matching the crates/history/src/catchup/buckets.rs ↔ crates/app/src/metrics.rs precedent.

Behavior preservation

The fd:3 meta callback is still invoked unconditionally after the match, on every path including the to_xdr failure branch — asserted by both test_emit_meta_routes_store_through_retry_helper and test_emit_meta_persists_row_and_invokes_callback. The happy path still writes the row and the stored bytes round-trip against to_xdr(Limits::none()).

Non-blocking (left inline, not gating)

• henyey_history::CatchupResult.meta_rows_dropped currently has no production consumer — see the inline comment on crates/history/src/lib.rs. The operator-visible surface of a drop (the error! line and the site="catchup_meta" counter) does fire, so this is a completeness gap in the programmatic affordance only.
• The end-to-end emit_meta test exercises the non-transient Failed branch through a real DB; the transient retry branch is covered only at the closure seam. The plan states this limitation explicitly and justifies it (with busy_timeout = 30_000 ms a lock held by a test thread is waited out, never surfaced as SQLITE_BUSY), and it matches the accepted Retry transiently-busy maintenance delete chunks #3781 precedent. Honest, not hidden.
• Two 25 ms std::thread::sleeps sit inside a call stack reached from an async fn (replay_one_batch). They are noise against the synchronous store_ledger_close_meta they wrap, which may itself already have blocked for busy_timeout = 30 s, and keeping the helper sync is a hard requirement for the thread-local recorder. Reviewed and accepted.

No blocking concerns.

[tomerweller]

Non-blocking (follow-up material, not a merge gate): this field currently has no production consumer.

crates/app/src/app/catchup_impl.rs:657 rebuilds the app-level CatchupResult (crates/app/src/app/types.rs:225) from output using only ledger_hash / buckets_downloaded / ledgers_applied, so meta_rows_dropped is dropped at the crate boundary and never reaches handle_catchup_result. The Display impl on this struct also has no production caller (crates/app/src/catchup_cmd.rs formats the app CatchupResult, a different type), so the new WARNING: N ledger_close_meta rows DROPPED suffix is currently only exercised by test_catchup_result_display_reports_dropped_meta.

The operator-visible surface of a drop is fine either way — emit_meta's error! line and henyey_db_busy_write_dropped_total{site="catchup_meta"} both fire — so nothing is lost today. But the plan's stated rationale for the field ("catchup completion is a boundary where reacting is safe") is not yet realised: no caller can react, because the value stops here. Threading it onto crates/app's CatchupResult (or logging it in handle_catchup_result) would close the loop.

[tomerweller]

🔍 Reviewer: Risk

Verdict: APPROVE

Summary: Cycle 1. Non-parity PR (no crates/{scp,herder,ledger,tx,overlay} paths), so risk lens. The only material risk is a 3× amplification of an already-pathological catchup stall on a persistently wedged DB — bounded, documented in-code, and explicitly reasoned out of scope in the converged plan. No API, data-format, config, migration or security exposure. One merge-order note reported below.

Full review

Re-read the diff independently at 139bb623 merged with origin/main de2588cf (merge was a no-op — the head is already on current main). Files changed: crates/db/src/error.rs, crates/app/src/app/persist.rs, crates/app/src/metrics.rs, crates/history/{Cargo.toml,src/lib.rs,src/catchup/mod.rs,src/catchup/persist.rs}. Not self-modifying.

perf-regression — catchup stall amplification (reviewed, accepted)

This is the one risk with real teeth, so stating it precisely: each of the up to 3 attempts calls store_ledger_close_meta, which can itself block for the full SQLite busy_timeout = 30_000 ms (crates/db/src/database/mod.rs:20). Worst case per ledger therefore goes from ~30 s to ~90 s (plus 2×25 ms of sleep, negligible). emit_meta runs per-ledger inside replay_via_close_ledger, before the batch's persist_ledger_history, so on a DB wedged for a whole 64-ledger batch the amplification is not capped by the batch-level abort — it is a genuine 3× on that path.

Why this is acceptable rather than blocking:

• The 30 s-per-ledger baseline is already an operationally fatal pathology on main; 32 min vs 96 min for a stuck batch is not the difference between healthy and unhealthy. The node is dead either way, and the alternative (dropping the row) is exactly the silent-data-loss bug this PR exists to fix.
• The observed production shape is episodic, not sustained: SQLite busy_timeout expiry silently drops 113 of 139 writes in 26d — including the retention trims that would relieve the contention, and catchup LedgerCloseMeta #3772 measured 139 lock-loss events across 26 days, i.e. transient contention that a 25 ms-spaced retry resolves on attempt 2, not a multi-second held lock.
• The retry is genuinely bounded — attempt < CATCHUP_META_MAX_ATTEMPTS with CATCHUP_META_MAX_ATTEMPTS = 3 — and the bound is pinned by test_store_meta_with_busy_retry_gives_up_after_max_attempts_and_counts_drop asserting exactly 3 closure calls, so it cannot silently become a spin.
• The trade-off is documented at the constant (crates/history/src/catchup/persist.rs:15-21) and the converged plan calls out the ≈90 s worst case and deliberately defers a per-run circuit breaker.

No new unbounded loop, no new blocking channel send, no new allocation on the hot path. The 25 ms std::thread::sleeps are inside an async call stack but wrap a call that already blocks synchronously for orders of magnitude longer, and the sync signature is load-bearing for the thread-local test recorder.

api-break — contained

henyey_history::CatchupResult gains a public field (crates/history/src/lib.rs:232) and is not #[non_exhaustive], so any struct-literal construction is a source break. In-tree that is exactly two sites (crates/history/src/catchup/mod.rs:668 and the new test), both updated; henyey-history is a workspace-internal, unpublished crate with no external consumers. crates/app's same-named CatchupResult (crates/app/src/app/types.rs:225) is a distinct type and is untouched, so nothing downstream of the app boundary changes shape.

CatchupManager gains an AtomicU32. It has no derives, so no Clone/Copy/Debug impl can break, and both constructors (new, new_with_arcs) plus the CatchupManagerBuilder::build path (which delegates to CatchupManager::new, verified — no struct literal) initialize it. A missed constructor would be a compile error, not a silent zero.

crates/app/src/app/persist.rs::is_transient_db_busy keeps pub(crate) and its signature; the ~20 call sites and the &anyhow::Error sibling are untouched, so #3808's record_db_busy_drop_if_transient path is unaffected. The predicate moving down to henyey-db widens visibility only for a method that is purely a classifier with no side effects.

migration-risk / config-risk — none

No schema change, no new config key, no new CLI flag, no on-disk format change. The retry re-issues the identical INSERT; to_xdr(Limits::none()), build_bucket_apply_meta and emit_meta_ext_v1 are untouched, so the meta bytes written to ledger_close_meta and to the fd:3 stream are byte-identical to main. Response completeness for RPC getTransactions/getLedgers strictly improves — this change can only add rows main would have lost. No rollback hazard: reverting simply restores the old drop behaviour.

ops-risk — improves, with one intentional log-level change

• Metric cardinality: +1 site value across 2 counter families = 2 new series, pre-registered at zero so an un-hit site renders an explicit 0 rather than being absent. test_db_busy_site_series_preregistered_at_zero enforces that automatically for the new value.
• I grepped the repo for henyey_db_busy outside crates/ (alarm scripts, dashboards, ops YAML/JSON/py) and found no external references, so no alarm-surface or dashboard file needs a companion edit. metrics::tests::dashboard_json_validation::* and test_all_counters_described_and_typed pass locally (64/64 in metrics::tests).
• Log level on the DB-failure path goes warn! → error!. That is the intended direction — the whole point is that a permanent RPC-visible gap should not be a warn — and I found no error-log-rate alarm in the repo that this would newly trip. The to_xdr serialization branch stays warn!, which is a slight asymmetry given it increments the same meta_rows_dropped counter, but it is a different (non-DB, deterministic) failure class and not worth churning.
• The fd:3 meta stream is unchanged: the callback still fires unconditionally on every path, including both DB-failure branches and the serialization branch. This is the surface stellar-rpc/Horizon actually consume, and it is asserted by two tests.

regression-risk — covered

The happy path is pinned by test_emit_meta_persists_row_and_invokes_callback (row round-trips through load_ledger_close_meta, callback fires exactly once, meta_rows_dropped() == 0). The wiring is pinned end-to-end by test_emit_meta_routes_store_through_retry_helper using a real DB fault rather than the closure seam. The classifier's narrowness — the actual consensus-safety exposure here, since a widened "transient" would let genuine corruption be treated as recoverable — is pinned negatively at seven points in crates/db/src/error.rs. Full local runs: henyey-db rc=0, henyey-history catchup::persist::tests rc=0 (31), henyey-app metrics::tests rc=0 (64).

supply-chain — none

metrics-exporter-prometheus is added under [dev-dependencies] only (crates/history/Cargo.toml:66), via workspace = true on a version already used by crates/{app,herder,overlay,simulation}. It does not enter the release binary; the production emit path goes through the global metrics facade with a literal counter name.

Merge-order note (reported, not a defect in this PR)

PR #3876 (do/issue-3806, opened from the same base) independently adds an identical DbError::is_transient_busy, an identically-named test, the same persist.rs delegate, and overlapping crates/app/src/metrics.rs catalog/pinning edits. Both show MERGEABLE only because GitHub evaluates each against main in isolation. #3874 is designated to merge first and a collision comment is already on #3876 instructing it to rebase and drop the redundant hunks. Nothing about that duplication is a defect here, and I confirmed #3874 is self-consistent standalone: its test_db_busy_site_label_vocabulary_pinned leaves the tx_set_gc reserved-absent assertion intact and untouched, which is exactly the state #3806 needs to rebase onto.

Pre-existing, not introduced

crates/history/src/catchup/mod.rs:242-246 carries a stale, misattached # Thread Safety doc block claiming CatchupManager is "not Send or Sync" — it actually sits immediately above the MetaCallback type alias, and every CatchupManager field (including MetaCallback = Box<dyn Fn(..) + Send + Sync> and the plain-data CatchupProgress) is Send + Sync. The new field's doc correctly says Send + Sync. AtomicU32 is the safe choice under either reading, so this is a pre-existing doc wart, not a risk introduced here, and not worth blocking or a follow-up.

No blocking concerns.

[tomerweller]

Review: Bounce-Back Cycle 1 — CI red (unrelated), both reviewer lenses APPROVE

Both agent reviewer lenses returned APPROVE. The bounce is CI-only, and the failure is not attributable to this diff.

40/41 checks are green. The single red is test (testnet, core,horizon, …) — the chronic #3768 / #3286 quickstart-testnet hang. Evidence it is pre-existing:

• origin/main is still de2588cf, i.e. this PR's exact base — a rebase is a no-op — and Quickstart run 31745517379 on main @ de2588cf fails on the same job, same step (Run probes through wrapper), with the same horizon-core-up soft-skip → horizon-ingesting hang → 25-minute step timeout.
• Three consecutive same-SHA attempts here (run 31755405144 attempts 1, 2 and 3 — attempt 3 was re-run during this review) reproduce it identically, each ending with the runner reaping orphan timeout / tee / go / test_horizon_ingesting processes (the CI: Quickstart testnet shard STILL hangs at step 8 despite #3273 -k fix — second hang path (orphaned docker-exec holds tee pipe) #3286 signature).

Do not change code in response to this bounce. Full reasoning and the recommended handling are in the bounce comment on #3801.

The one inline comment on crates/history/src/lib.rs is explicitly non-blocking (follow-up-issue material), not required work.

[tomerweller]

Now CONFLICTING — #3876 landed the shared symbols first

Heads up: this PR went to mergeable=false without any push (updated_at still 2026-08-14T01:34:16Z). The cause is external — #3876 merged as 2b198d37 at 2026-08-20T02:49:38Z, and it landed the same three shared pieces this PR also adds.

Verified against origin/main at 1a25a650:

Landed on main by #3876	This PR still adds
crates/db/src/error.rs:103 pub fn is_transient_busy()	crates/db/src/error.rs +112/-0 (same block from scratch)
crates/db/src/error.rs:138 test_is_transient_busy_matches_busy_and_locked, :163 test_is_transient_busy_rejects_non_busy	same test name
crates/app/src/app/persist.rs:546-551 delegate to the canonical henyey_db::DbError::is_transient_busy	crates/app/src/app/persist.rs +8/-11
crates/app/src/metrics.rs catalog/pinning edits	crates/app/src/metrics.rs +17/-11

The issue-#3801 payload itself does not collide. These files are untouched by #3876 and carry the actual fix:

• crates/history/src/catchup/persist.rs +320/-7
• crates/history/src/lib.rs +55/-1
• crates/history/src/catchup/mod.rs +35/-0
• crates/history/Cargo.toml +4/-0

So the rebase is well-scoped: drop the now-duplicate crates/db/src/error.rs, crates/app/src/app/persist.rs, and crates/app/src/metrics.rs portions (all three are on main already), point any local use at the landed canonical is_transient_busy, and keep the crates/history/** payload as-is. Both reviewer lenses had already approved the substance — this is a mechanical de-duplication, not a re-review.

Context: the collision between this PR and #3876 was flagged before either merged, with the recommendation that #3874 land first precisely to avoid this. #3876 merged first instead, so the rebase burden moved here. Note also that a MERGEABLE reading on two PRs that add identical symbols is false comfort — GitHub tests each against main in isolation, which is why both read clean right up until one landed.

No action taken on my side: not merging, not modifying, not re-reviewing. Recording the cause and scope so the rebase is a known quantity.