Skip to content

fix(client): harden supervised connection recovery edge cases - #583

Merged
sleipnir merged 4 commits into
elixir-grpc:masterfrom
enilsen16:fix/connection-recovery-followups
Sep 1, 2026
Merged

fix(client): harden supervised connection recovery edge cases#583
sleipnir merged 4 commits into
elixir-grpc:masterfrom
enilsen16:fix/connection-recovery-followups

Conversation

@enilsen16

Copy link
Copy Markdown
Contributor

Follow-ups to #568 — a set of recovery edge cases found while reviewing that change after merge.

Connection (lib/grpc/client/connection.ex)

  • Stale retry timers: schedule_retry deduped on a boolean but never cancelled a pending timer, so a transport death after a resolver-driven recovery could sit out the remainder of an old long backoff (up to ~2min) instead of redialing at the newly computed delay. Now stores the timer ref, cancels/reschedules, and adopt_established cancels any pending retry.
  • Dead-channel adoption race: rebalance_after_reconcile adopted on connected != [] without the channel_alive? check the :retry_establish handler applies for exactly this race, so a channel that died with its :DOWN still queued could produce a false :connected (waiters released, telemetry emitted) until the :DOWN unwound it.
  • Stale LB state for pickers: the LB state returned by lb_mod.update/2 was only kept in GenServer state; pick_channel reads :persistent_term, which was only re-put in establish/1. A behaviour-compliant LB that returns fresh state from update/2 (rather than mutating ETS in place) left pickers on the stale state indefinitely. Now republished when the state changes (no-op for the built-ins).
  • Backoff ladder reset after flaps: adopt_established reset retry_attempt to 0 but not flaps, so an establish failure right after a flap-death restarted the dial ladder at the shortest delay — the backoff could shrink after a failure. handle_channel_down now seeds retry_attempt from the flap count.
  • Silent resolver failures: non-{:ok, _} returns from resolver.update/2 were silently discarded (the removed handle_cast(:resolve_now) used to assert the match). Now logged, keeping the previous resolver state.

Stub (lib/grpc/stub.ex)

  • Crash instead of UNAVAILABLE: fallback_channel's catch-all passed a payload like %{conn_pid: nil} — the documented return of Connection.disconnect/1 — straight to the adapter, crashing on a nil pid. Any payload carrying :conn_pid is now liveness-checked (nil/dead/remote → UNAVAILABLE); only payloads without the key pass through.
  • Pid-less adapters treated as dead: resolve_channel used Map.get(payload, :conn_pid), conflating a missing key with nil, while channel_alive? treats the same shape as alive — so a healthy connection over an adapter that exposes no transport pid could never serve an RPC from the stub side. Now aligned with channel_alive?.
  • Failure results bypassed connection config: unavailable_result ran the caller handle's (often empty) interceptor list with struct-default codec/compressor, while the healthy path uses the picked channel's config — so bare %Channel{ref: name} handles skipped the connection's interceptors on failures only. Now resolves config via Connection.get_channel/1 and honors the caller's codec/compressor opts.
  • Wasted re-picks on non-rotating policies: the bounded re-pick loop burned all attempts on the identical dead channel under PickFirst (plus a Logger.warning per attempt). The loop now bails when the policy returns the same pid again.

Verification

mix test: 10 doctests, 360 tests, 0 failures (2 pre-existing skips). mix format --check-formatted clean on both files.

enilsen16 and others added 2 commits August 31, 2026 09:33
Follow-ups to elixir-grpc#568:

- cancel pending retry timers on adopt/reschedule so a stale long-backoff
  timer can't delay redial after a fresh transport death
- guard rebalance_after_reconcile adoption with channel_alive?, matching
  the :retry_establish handler, to avoid adopting an already-dead channel
- republish LB state to :persistent_term when lb_mod.update/2 returns a
  new state, so pickers don't read stale state from behaviour-compliant LBs
- seed retry_attempt from the flap count so a dial failure after a
  flap-death continues the backoff ladder instead of restarting it
- log non-ok resolver update/2 results instead of silently ignoring them
- liveness-check fallback channels carrying conn_pid (e.g. the
  documented disconnect/1 return) and return UNAVAILABLE instead of
  crashing in the adapter
- align resolve_channel with channel_alive? on payloads without conn_pid
  so pid-less adapters aren't treated as permanently dead
- resolve connection config in unavailable_result so failure results run
  the connection's interceptors with the caller's codec/compressor
- stop re-picking when the LB policy returns the same channel (PickFirst)
Comment thread grpc/lib/grpc/stub.ex Outdated
@enilsen16
enilsen16 force-pushed the fix/connection-recovery-followups branch from da2eff3 to a5bfa20 Compare September 1, 2026 19:56
@sleipnir
sleipnir merged commit b0d9998 into elixir-grpc:master Sep 1, 2026
7 checks passed
@sleipnir

sleipnir commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Thank @enilsen16

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.

2 participants