Skip to content

Health endpoint inert on State backend: make generic launch path sole owner of the readiness gate (follow-up to #1239) #1311

Description

@zancas

Background

PR #1239 ("Add liveness and readiness HTTP endpoints") relocated the
"wait for initial sync to complete" gate out of the backend's spawn
and into the generic indexer launch path, so the HTTP health server
(/livez, /readyz) can come up during the multi-hour initial scan
instead of only after it finishes.

The new generic orchestration in Indexer::launch_inner
(packages/zainod/src/indexer.rs) is:

1. IndexerService::<Service>::spawn(cfg)   // → Service::spawn(cfg)  (backend-specific)
2. spawn health server                      // generic, for any Service
3. loop until Service::status() == Ready    // generic readiness gate

For steps 2–3 to mean anything, step 1 must honor an invariant:
ZcashService::spawn returns promptly with sync running in a background
task (status Syncing); it must not block until Ready itself.

The problem this issue tracks

That invariant is enforced only by a prose comment, not by the type system —
and PR #1239 left it satisfied for one backend but not the other:

FetchService::spawn StateService::spawn
pre-#1239 blocks until Ready blocks until Ready
post-#1239 returns immediately (Syncing) still blocks until Ready

FetchService::spawn had its internal wait-loop removed
(packages/zaino-state/src/backends/fetch.rs), but
StateService::spawn still contains its copy
(packages/zaino-state/src/backends/state.rs, the
// wait for sync to complete loop).

Because IndexerService::spawn (packages/zaino-state/src/indexer.rs) is a
thin wrapper that just awaits Service::spawn, for the State backend
step 1 blocks until full initial sync — so step 2 (start health server) is
only reached after the scan is already done:

  • /livez and /readyz are not reachable during the initial scan on the
    State backend (connection-refused), which is the precise failure the health
    endpoint exists to prevent (a k8s liveness probe can kill+restart the pod in
    a loop during a long sync).
  • The two backends, which were consistent before Add liveness and readiness HTTP endpoints #1239 (both blocked
    internally), are now divergent — the divergence is introduced by Add liveness and readiness HTTP endpoints #1239,
    not pre-existing.

This is not a regression for the State backend in isolation (it behaves as it
always did), but the PR's feature is silently inert for it.

Why we are not just deleting the State loop

The minimal fix — delete the wait-loop from StateService::spawn so the
single generic gate governs both backends — works, but it leaves the root
cause in place: the "spawn must not block on readiness" invariant remains a
convention a future backend (or a future edit) can silently violate again.
We're tracking the deeper fix here instead and landing #1239 as-is.

Proposed fix (lighter design)

Make the generic launch path the sole owner of the readiness gate, and
remove the means and motive for a backend to re-introduce one:

  1. Keep ZcashService::spawn async (construction genuinely needs .await:
    validator connect, get_info, NodeBackedChainIndex::new — and the long
    pole, the sync loop, is already backgrounded by
    NodeBackedChainIndex::new).
  2. Document the non-blocking invariant on the trait method itself
    (packages/zaino-state/src/indexer.rs, ZcashService::spawn), not in a
    per-backend comment: spawn returns as soon as the service is constructed
    and background sync has started, status Syncing; the caller
    (Indexer::launch_inner) owns the readiness gate; a setup/sync failure is
    surfaced via status() == CriticalError.
  3. Remove the wait-loop from StateService::spawn so neither backend
    implements the gate; both flow through the one generic gate, and a
    sync-time CriticalError is caught there via status() exactly as for
    FetchService.

Net: backends differ only in how they construct + start sync, never in
whether/where they gate on readiness.

Heavier alternative considered and rejected

A Spawnable trait with a synchronous fn spawn(config) -> Result<Self, SpawnError> would make the readiness-gate-in-spawn a compile error (a fn
can't .await, so the loop { … sleep().await } is unwriteable), pushing all
async construction into a background task and routing setup errors through
CriticalError (which the gate already consumes).

Rejected for now because the cost/benefit is poor here specifically: the only
genuinely long async work (the sync loop) is already off the startup path,
so a sync spawn would defer the cheap construction (connect + build index
handle) too, forcing the service inner behind an Arc<ArcSwapOption<_>> and
making get_subscriber() (called before Ready by the health wiring) race
construction. We can revisit if a third backend or a recurring drift makes the
hard compile-time guarantee worth that machinery.

Acceptance

  • ZcashService::spawn's non-blocking-until-Ready contract is documented
    on the trait method.
  • StateService::spawn no longer contains a wait-until-Ready loop.
  • /livez and /readyz are reachable during initial sync on both
    backends (a manual check or a startup test that probes before Ready).

Follow-up to #1239.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions