Skip to content

fix(cluster): treat a full dispatch queue as retryable capacity and pace the startup rebuild - #355

Closed
EnRaiha wants to merge 8 commits into
mainfrom
fix/calvin-backpressure
Closed

EnRaiha wants to merge 8 commits into
mainfrom
fix/calvin-backpressure

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Why

A full per-tenant bridge queue returned Error::Dispatch; the Calvin scheduler
logged it at ERROR and completed the transaction as failed, releasing its locks.
During the startup rebuild the epoch is re-driven, so a full queue became a spin:
33,435 dispatch failed ERRORs in 60 s, CPU pinned, pgwire starved. It
reproduced on the production server with a build derived from main: the node came
up and never served.

What

Commit Change
df9c3b818 Error::DispatchCapacityBusy { tenant_id, inflight, cap } — retryable by type. The dispatcher returns it for the per-tenant cap; the epoch dispatch demotes the log to debug, counts it (dispatch_busy_count), and paces the catch-up drain with busy_backoff (5 ms doubling to a 250 ms cap, deterministic jitter)
ae60781c4 the same handling at every dispatch site: CalvinResolve, commit-resolution, write-version record, and the active epoch dispatch (note_dispatch_busy / note_dispatch_busy_shared)
c5a61d025 RAFT_READY_STALL_TIMEOUT 30 s → 300 s: with a large apply backlog the metadata group needs minutes, and 30 s turned a slow boot into a restart loop
eb4f0b295 DATA_GROUP_RECOVERY_TIMEOUT 60 s → 600 s: restores the value the production config carried before the bound became a hard-coded constant
dbfccd46c drop an issue reference from a touched comment (house preflight)

The gateway maps the new variant to BUSY, so a client still sees a retryable
overload rather than an internal failure, and the tracker entry is cancelled on
the deferred path.

How to test

  • full_queue_is_a_retryable_capacity_condition — new; fails on main (the error
    was Error::Dispatch), passes with the fix
  • full_queue_returns_error — unchanged: a full queue is still an error
  • busy_backoff_is_bounded_and_jittered — bound ≤ 257 ms, jitter varies with
    vShard and epoch

Validation

  • Production deploy of a build carrying these commits: serving, zero
    storm-class ERRORs across the monitored window, SELECT 1 returns, the
    knowledge-graph store is intact (70,978 rows).
  • Before the fix, the same workload on the same server produced the 33k-error
    storm and no service.
  • The startup path was verified with the raised bounds on a backlogged recovery;
    the previous hard-coded bounds aborted the start with
    metadata group applied no entry for 30s and
    data raft group recovery timeout after 60s.

Notes

  • Check C5 (500 non-test lines per changed file) is green without a bypass:
    this branch splits the busy accounting into core/busy.rs and the active
    dispatch into core/active_dispatch.rs (dispatch.rs 543 → 387,
    scheduler.rs 508 → 459 non-test lines), and drops variant docs in
    error/types.rs that restated their display messages, returning it below its
    base count (601 → 586). The enum itself cannot go under 500 without changing
    the public API, so C5 treats it as the pre-existing over-limit file it is:
    no growth.
  • The two raised bounds should become configuration again: issue feat(config): make the startup stall and data-group recovery bounds configurable #354.
  • Files carrying pre-existing issue numbers elsewhere in the tree are untouched.

Fixes #352
Fixes #353

Tradeoffs

Tradeoff: error/types.rs cannot reach 500 non-test lines while its enum stays one type, so C5 treats it as a pre-existing over-limit file that must not grow; the refactor keeps it below its base instead.

Copilot AI lite review requested due to automatic review settings September 20, 2026 09:23

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@EnRaiha EnRaiha added the run-ci Opt this PR into the full test suite; re-add to force a re-run label Sep 20, 2026
@EnRaiha
EnRaiha added this pull request to stack #358 September 20, 2026 11:14
@EnRaiha
EnRaiha removed this pull request from stack #358 September 21, 2026 02:36
…a failure

A full per-tenant bridge queue returned Error::Dispatch, which the Calvin
scheduler logged at ERROR and completed as a failed transaction, releasing its
locks. During the startup rebuild the epoch is re-driven, so a full queue spun:
one ERROR and one failed transaction per attempt, the queue never drained, CPU
pinned, and pgwire starved.

- add Error::DispatchCapacityBusy { tenant_id, inflight, cap }: the request was
  not enqueued and nothing was applied, so it is retryable by type
- the bridge dispatcher returns it for the per-tenant cap
- the scheduler demotes the log to debug, counts it (dispatch_busy_count), and
  paces the catch-up drain with a bounded jittered backoff (busy_backoff, 5 ms
  doubling to a 250 ms cap) instead of spinning
- classify the variant like a dispatch failure and map it to BUSY for clients

Tests: full_queue_is_a_retryable_capacity_condition (new; failed before the
fix), full_queue_returns_error (unchanged), busy_backoff_is_bounded_and_jittered.

Issue 352.
The first change classified a full per-tenant queue as retryable and paced the
epoch dispatch, but four other sites still logged ERROR and completed the work
as failed: the CalvinResolve dispatch, the commit-resolution dispatch, the
write-version record dispatch, and the second (active) epoch dispatch. On a
large WAL replay those sites kept the catch-up loop busy and the readiness gate
never opened.

- note_dispatch_busy(&mut self, ..): counts the event and arms the bounded
  drain backoff; used by the epoch, CalvinResolve and commit-resolution sites
- note_dispatch_busy_shared(&self, ..): counts only (no arming) for the
  one-way write-version record dispatch, which holds only a shared borrow
- each site demotes the log to debug on the retryable class

Issue 352.
…startup

The readiness gate failed startup when the metadata group applied no entry for
30 s. With a large apply backlog the group needs minutes, so a slow boot became
a restart loop (2026-09-20 incident; issue 352). Raise the bound to 300 s and
keep failing a group that never applies.

Follow-up: expose the bound as configuration.
The configurable [server] data_group_recovery_timeout_ms (600 s in production)
was replaced by a hard-coded 60 s. On a backlogged recovery the data groups need
longer: group 2 reached 261 of 262 committed entries inside 60 s and startup
aborted into a restart loop (2026-09-20 deploy attempts of the HEAD+fixes
builds). Restore the previous operator value as the constant; a follow-up should
expose it as configuration again.
The house preflight requires the issue number to live in the PR, not in the
code, and this branch touches the file.
The Calvin determinism gate forbids unmarked Instant::now() in the write path.
The backoff deadline and the drain gate read the wall clock for pacing only —
scheduler observability, never Calvin WAL data. Comment-only change.
…into modules

The scheduler and dispatch files grew past the 500-line house limit. Move
the capacity-busy accounting and backoff into core/busy.rs and the active
dependent-read dispatch into core/active_dispatch.rs, and route the static
dispatch busy path through the shared helper instead of duplicating it.
Each removed doc line repeated the variant's #[error] text, so the message
is now the single source for what the variant says. The enum had also grown
past its base line count; this returns it below the unchanged-file bound.
@EnRaiha
EnRaiha force-pushed the fix/calvin-backpressure branch from 7700837 to cbae048 Compare September 21, 2026 14:00
@farhan-syah

Copy link
Copy Markdown
Member

Closing. On a capacity-busy dispatch, static_dispatch.rs and active_dispatch.rs still call on_txn_complete. That releases the locks and marks the position applied, but the transaction never ran on that replica. Demoting the log to debug makes that loss invisible instead of fixing it. The two startup bounds are progress/stall bounds. The failures cited (no applied entry for 30 s; 261 of 262 entries) are stalls, not slow replays. No [server] data_group_recovery_timeout_ms field ever existed in the repo history. The maintainers will fix this in-tree: a busy dispatch keeps the transaction pending and re-dispatches it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci Opt this PR into the full test suite; re-add to force a re-run

Projects

None yet

3 participants