Skip to content

ghsyncd crash-loops on startup while a stale budget lease is held #52

Description

@ewhauser

Summary

When ghsyncd dies without running Gate.Close (crash, SIGKILL, OOM), the installation budget lease in installation_budgets remains held until its TTL (default 30s) expires. A restarted process calls budget.NewLeased, receives ErrLeaseHeld, and cmd/ghsyncd treats that as a fatal startup error:

githubGate, err = budget.NewLeased(...)
if err != nil {
    return fmt.Errorf("GitHub budget gate: %w", err)
}

The process exits, the supervisor restarts it, and it crash-loops until the stale lease expires and one attempt steals it.

Observed behavior

Reproducible locally via the loadgen chaos scenario (TestChaosKnobsPassStrictEndToEndAssertions with restarts): each chaos restart logs several

ghsyncd: GitHub budget gate: GitHub installation budget lease is held

lines before a start finally succeeds. During that window no fetch/sweep/drift work runs, so refresh generations stall. The recent queue-isolation work (#48/PR #51) keeps the strict chaos oracle green despite this, but the underlying unavailability window remains: worst case is the full lease TTL, and with a supervisor that backs off restarts (e.g. Kubernetes CrashLoopBackOff) the effective outage extends well beyond the TTL.

This also affects clean rolling deploys when the old process is killed rather than drained: the replacement crash-loops instead of waiting the few seconds until the lease frees.

Expected behavior

ErrLeaseHeld at startup is an expected, transient condition under C-B1/C-O2 singleton semantics, not a configuration error. The starting process should wait in standby and retry acquisition (bounded polling with jitter, responsive to shutdown signals) instead of exiting. Genuinely fatal conditions (bad installation ID, transport/store errors persisting beyond a bound, invalid options) should still fail startup loudly.

Suggested direction

  • In cmd/ghsyncd, on ErrLeaseHeld (and only that error), retry budget.NewLeased on a capped, jittered interval until acquisition succeeds or the process receives its shutdown signal. Log at most once per interval change that the process is standing by for the lease (include the owner name, never the token).
  • Alternatively (or additionally) expose the standby loop from internal/budget so tests can drive it with the fake clock.
  • Preserve the singleton invariant: exactly one active gate per installation; a standby must not admit any GitHub calls before acquisition.
  • Do not shorten or bypass the TTL steal path: token-checked expiry-steal in AcquireInstallationBudgetLease remains the ownership authority (C-C6/C-B1 unchanged — the acquire transaction stays local SQL only).

Acceptance criteria

  • A process started while another live owner holds the lease stands by and becomes the owner promptly after the owner releases (clean handoff) without process exit.
  • A process started after an ungraceful kill becomes the owner promptly after TTL expiry without process exit (no crash-loop).
  • Shutdown signal during standby exits cleanly.
  • Non-ErrLeaseHeld startup errors remain fatal.
  • Tests cover: standby-then-release handoff, standby-then-expiry steal, signal-during-standby, and the fatal path — using the fake clock where applicable, no wall-clock sleeps.
  • The loadgen chaos restart scenario no longer logs fatal GitHub budget gate exits; restarted processes log standby and recover within the lease TTL.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions