Skip to content

Chore/history UI improvements#1156

Merged
frol merged 24 commits into
mainfrom
chore/history_ui_improvements
Jul 20, 2026
Merged

Chore/history UI improvements#1156
frol merged 24 commits into
mainfrom
chore/history_ui_improvements

Conversation

@ktarun1419

@ktarun1419 ktarun1419 commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator
  • Added a one-time Gold history projection after Bronze/Silver backfill completes; subsequent updates are processed incrementally.
  • Added a per-account readiness flag to safely reroute History APIs from legacy data to Gold history.
  • Added PostgreSQL leader election for Apalis jobs so only one application instance runs background workers, with safe leadership handover.
  • Fixed delayed exchange fulfillment and recomputation to maintain correct historical balances.
  • Ensured history rows, counts, and prior balances consistently use the same data source.
  • Fixed token decimal handling and confidential-history filtering.

@ktarun1419
ktarun1419 requested a review from frol July 20, 2026 16:22

@frol-ai frol-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the background-jobs leadership part (jobs/leadership.rs, jobs/mod.rs, the background_job_leader migration, render.yaml) — it's directly relevant to the DB-connection/reliability incident we've been chasing. Overall this is a well-built, correct approach; comments below focus on that infra and its overlap with the other in-flight fixes for the same incident.

The leadership design is solid ✅

  • Using pg_try_advisory_lock as the actual mutex is the right primitive — it's session-scoped, so if the leader's connection dies (exactly our Postgres-crash case) the lock auto-releases and a follower can take over. No split-brain.
  • Generation fencing + rows_affected() == 1 on the heartbeat correctly detects "I lost leadership" and steps down.
  • Graceful drain (stop_runtime, 45s) + render.yaml maxShutdownDelaySeconds: 90 addresses the deploy-overlap connection storm nicely.
  • On HeartbeatFailed it discard()s (close-on-drop) rather than trying to release on a dead connection — correct.
  • Compatible with the pool-lifecycle hardening in #1155: the advisory-lock connection is held checked-out for the leader's lifetime, so it's exempt from idle_timeout/max_lifetime reaping — no premature lock loss. Worth a one-line comment in the code noting that this connection is intentionally never returned to the pool.

Coordinate with the other incident PRs (same files/incident)

This is one of several parallel fixes for the 2026-07-20 stall; they're complementary but touch the same code:

  • #1159 (cron-worker restart backoff) is still needed inside the leader's runtime. Leadership only stops the cron hot-loop after the ~5–10s heartbeat-failure detection window; during that window the CronStream→store pipe error makes the Monitor rebuild cron workers with zero delay, spinning CPU and hammering the DB — which is what cascaded the original outage. Landing #1159 makes that window calm.
  • #1154 (liveness monitor) — you've kept run_liveness_monitor inside the runtime (good). But with leadership's cancellable runtime + heartbeat failover, its std::process::exit(1) is now a heavier hammer than needed; consider having it signal the leadership loop to step down/re-elect instead of killing the process.
  • #1155 pool hardening and #1157 price-sync query cache reduce connection footprint and the heavy query that pressures the DB — orthogonal, keep both.

Packaging concern ⚠️

A major infra change (867-line leadership module + a jobs/mod.rs rewrite + a migration + render.yaml + a new pool connection) is bundled here with "history UI improvements" and unrelated changes (routes/balance_changes.rs +228, token_prices/*, gold/projector.rs, …). That makes it hard to review safely and risky to land/rollback as one unit. Strongly suggest splitting the background-jobs leadership into its own PR so it can be reviewed and merged on its own merits — especially since it will conflict with the merged/in-flight incident PRs and wants a careful rebase.

Happy to rebase my pool/backoff/cache PRs on top of the leadership change (or vice-versa) once we decide the ordering.

@ktarun1419
ktarun1419 marked this pull request as ready for review July 20, 2026 19:46
@frol
frol merged commit 8583dde into main Jul 20, 2026
3 checks passed
@frol
frol deleted the chore/history_ui_improvements branch July 20, 2026 21:52
frol pushed a commit that referenced this pull request Jul 21, 2026
#1161)

Fixes the "multiple account-maintenance tasks Running at once" observed
on the board. They were **not** concurrent — one worker,
`concurrency(1)`, heartbeating fine — but **orphaned `Running` rows**,
one appearing every ~30 min (= the handler `job_timeout`).

## Root cause
`run_maintenance_cycle` processed *every* enabled account each tick, so
the total time was O(accounts). Once that exceeded the 30-min
`job_timeout`, the handler was cancelled mid-cycle and the timed-out
task was left in `Running` — its ack to mark it terminal never landed
(DB instability / cancelled handler). apalis's own orphan-reclaim only
fires when the **worker's** heartbeat goes stale, but the worker was
alive, so these phantom `Running` rows were never reclaimed *or* pruned
and just piled up.

## Fixes
1. **Batch the cycle** (`ACCOUNT_MAINTENANCE_BATCH_SIZE`, default 25).
Dirty accounts sort first (never starved); the rest rotate by
`last_synced_at` across cycles (cron fires every 60s). Worst-case cycle
time = `batch / concurrency × per_account_timeout` ≈ 25/4×120s ≈ 12.5
min — comfortably under `job_timeout`, so cycles finish and ack cleanly.
2. **Reclaim orphaned locks** in `apalis_prune`: flip tasks stuck
`Running` past a threshold to `Killed` (then removed by the retention
sweep). Threshold is at least **2× `job_timeout`**
(`APALIS_STUCK_RUNNING_RECLAIM_SECONDS`), so a genuinely-running task —
bounded by `job_timeout` — is never touched.

## Testing
`cargo fmt --check`, `cargo clippy --lib -- -D warnings`, `cargo check`:
clean. Validated the batch ordering and the reclaim UPDATE against a
live apalis schema (a 2h-stuck `Running` row → `Killed`; a 2-min-old one
left untouched).

Note: with #1156's leader election now merged, only the elected leader
runs these — the batching keeps each leader's maintenance cycle bounded,
and the reclaimer cleans up any locks orphaned during a leadership
transition or DB blip.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01FRWEKUFYCcmGGhwdDYd471

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@akorchyn akorchyn added this to the v1.22.0 milestone Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants