fix: recover supervised connections when the transport process dies - #568
Merged
sleipnir merged 7 commits intoAug 31, 2026
Merged
Conversation
enilsen16
marked this pull request as ready for review
July 30, 2026 16:18
sleipnir
reviewed
Jul 30, 2026
When an underlying transport process died, GRPC.Client.Connection kept
the dead channel in the load-balancing rotation. Every subsequent RPC
picked the stale channel, fell back to the payload-less virtual handle,
and crashed with a FunctionClauseError in the Gun adapter - permanently,
until the connection process was restarted. Worse, the orchestrator had
no death signal at all for Gun transports: they live under the adapter's
DynamicSupervisor (restart: :temporary), not linked to the orchestrator,
so the log-only :EXIT clauses never fired either.
Connection changes:
- Monitor every successfully connected transport (connect_real_channel).
A :DOWN (or :EXIT, for adapters that link like Mint) from a tracked
conn_pid marks its channel {:failed, reason} and rebalances so pickers
stop seeing it. Deliberate disconnects remove the channel from state
before the signal arrives, so they never match; their trailing
:DOWN :normal is ignored quietly.
- When the last healthy channel is gone, flip established? and schedule
:retry_establish instead of dialing inside the signal handler. Deaths
within 10s of establishing count as flaps and back off exponentially;
stable connections redial immediately.
- :retry_establish adopts channels a background resolver update already
reconnected instead of dialing a duplicate set that would orphan them.
- When other channels remain, request an early re-resolution and run a
self-scheduling repair loop that redials {:failed, _} entries from the
last resolved address set, so static multi-address targets recover the
dead endpoint too.
- Re-establishment reuses the ETS-backed LB state via lb_mod.update/2;
a policy flip disposes the old balancer via a new optional terminate/1
callback on GRPC.Client.LoadBalancing.
Stub changes:
- Re-pick (bounded) when the picked channel's conn_pid is dead, so
rotating policies advance past the dead entry during the rebalance
window.
- With no healthy channel resolvable for a named connection's virtual
handle, fail with UNAVAILABLE instead of handing the adapter an
unusable channel: error tuple for unary/server-streaming, raised
GRPC.RPCError for request-streaming calls (their return value is a
stream). The failure still flows through the interceptor chain and
client_span telemetry. GRPC.Stub.connect/2 channels keep the existing
fallback behavior.
- terminate the old LB only after the replacement initializes, so a failed policy-flip attempt can't leave lb_state pointing at a deleted ETS table (later update calls would crash the connection process); dedup the init/update failure shells into run_lb/3 - on a resolve failure during re-establishment, redial the last known address set under the existing LB instead of silently downgrading a round_robin connection to a single PickFirst endpoint - schedule the repair loop from adopt_established when some addresses failed to dial, so a partially successful establish doesn't strand the failed endpoints when there is no background resolver - flush the sibling :EXIT/:DOWN signal when handling a channel death so link-based adapters (Mint) don't log a spurious unrelated-signal warning for every transport death - validate call options before resolving the channel so configuration errors raise deterministically instead of being masked as UNAVAILABLE while the connection is down - fail fallback_channel with UNAVAILABLE when a payload-carrying channel's conn_pid is dead instead of handing the adapter a dead transport - honor an interceptor-transformed result on the request-streaming UNAVAILABLE path, raising only when the chain still returns an error; dedup the interceptor fold into run_interceptors/2
Expose the previously hard-coded 10s flap window as a :flap_window connection option, validated and defaulted like the other timing knobs.
- guard the optional Resolver.update/2 callback in request_reresolve so a
partial transport death can't crash the connection process when a custom
resolver omits it; handle_cast(:resolve_now) now delegates to the same
helper instead of keeping a divergent copy
- dedup :retry_establish timers behind a retry_scheduled? flag so flap
cycles during an outage can't accumulate concurrent retry loops
- adopt immediately when a resolver update reconnects channels while a
delayed retry is pending, so await_ready/connect track actual recovery
instead of the retry backoff
- liveness-check channels on the :retry_establish adopt path so a
reconnected-then-dead channel whose death signal is still queued can't
trigger a false adopt
- only schedule the repair loop for connections without a background
resolver; resolver ticks already redial failed endpoints
- read codec/compression defaults from the picked channel again: bare
%Channel{ref: name} handles carry none of the connection's config
(keeps option validation ahead of resolution)
- log one warning per RPC when re-picks are exhausted instead of one per
attempt
enilsen16
force-pushed
the
fix/supervised-connection-down-recovery
branch
from
August 18, 2026 15:58
6ebc9b6 to
8a58ebc
Compare
Named Gun connection owners were registered through :global. In a clustered application, nodes using the same channel name and target therefore competed for one cluster-wide owner name. DynamicSupervisor.start_child/2 could report the process on another node as already started. The local connection manager accepted that remote PID, then called Process.alive?/1 while resolving an RPC. Erlang only allows that check for local PIDs, so requests raised ArgumentError: not a local pid instead of returning UNAVAILABLE and reconnecting. Register owners in the existing node-local GRPC.Client.Registry so every node owns and supervises its own transport. Reject remote owner PIDs as unavailable as a defensive fallback, and strengthen the distributed test to prove both nodes can call independently and one disconnect does not affect the other.
Contributor
|
hey @sleipnir any updates on this one? |
sleipnir
approved these changes
Aug 31, 2026
Collaborator
There was a problem hiding this comment.
@enilsen16 Please resolve the conflicts so we can proceed.
…nnection-down-recovery # Conflicts: # grpc/test/grpc/client/connection_test.exs
Collaborator
Collaborator
|
Thank you @enilsen16 |
Contributor
Author
|
Thanks I have a small followup commit :) Will have a PR shortly |
sleipnir
added a commit
that referenced
this pull request
Sep 1, 2026
* fix(client): harden supervised connection recovery edge cases Follow-ups to #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) * chore: remove unnecessary comment in unavailable_result * chore: drop redundant comments added in this PR --------- Co-authored-by: Adriano Santos <solid.sistemas@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a transport process dies,
GRPC.Client.Connectionnever notices: Gun transports live under the adapter's supervisor with no link or monitor back to the connection process. The dead channel stays in the load-balancing rotation, and every RPC that picks it crashes with aFunctionClauseErrorin the Gun adapter — permanently, until the connection process is restarted.What this PR does
:DOWN/:EXITfrom a tracked transport removes its channel from rotation immediately.:flap_window(default 10s) of establishing.{:error, %GRPC.RPCError{status: 14}}(UNAVAILABLE) instead of crashing the caller; request-streaming calls raise, since their return value is a stream. Failures still flow through the interceptor chain and telemetry.await_ready/2andconnect/2succeed as soon as any healthy channel exists, whichever path restored it.API additions
:flap_windowconnection option (ms, default10_000).terminate/1callback onGRPC.Client.LoadBalancing, so re-establishment reuses ETS-backed balancer state instead of leaking one table per reconnect.Edge cases covered
Policy flips dispose the old balancer only after the new one initializes; a resolve failure during redial keeps the last known addresses instead of downgrading the LB policy; the optional
Resolver.update/2callback is guarded; retry timers dedup so outages can't accumulate concurrent retry loops; a reconnected-then-dead channel can't fake readiness; call options are validated even while the connection is down, so config errors raise instead of masking as UNAVAILABLE.