Coordinators for a led vnode can die permanently without being restarted

[HectorIFC]

Context

A vnode's coordinators can die permanently while the node keeps leading that vnode, reporting healthy, and never starting them again. Self-healing, consumer-group coordination and retention stop for that vnode until leadership changes hands or the manager process itself crashes.

The chain is three pieces that are each individually reasonable.

Malachi.Application.start_vnode_coordinators/1 (lib/malachi/application.ex:742-756) starts a per-vnode Supervisor holding the heal coordinator, the group coordinator and (when a policy is set) retention, and hands it to the DynamicSupervisor with restart: :temporary (:751). The :temporary is deliberate and correct on its own terms: the manager owns the lifecycle, and a DynamicSupervisor that restarted the child by itself could bring back coordinators for a vnode this node no longer leads.

Malachi.Cluster.VnodeCoordinatorManager.reconcile/1 (lib/malachi/cluster/vnode_coordinator_manager.ex:72-79) is level-triggered, which is what makes leadership flaps safe. But the level it compares against is Map.keys(state.running) (:74), a plain vnode_id => pid map (:45) populated in start_vnodes/2 (:81-88) and pruned only in stop_vnodes/2 (:90-98), which runs only when the vnode leaves the led set. Nothing monitors those pids and there is no :DOWN clause in the module.

So when the per-vnode supervisor exits, the dead pid stays in state.running, the vnode stays in the running set, MapSet.difference(desired, running) is empty, and the manager never spawns a replacement. It also keeps reporting the vnode through reconcile_now/1, so the observable state says the coordinators are up.

The per-vnode supervisor is not hypothetical to kill. Its comment (:740-741) says it exists "so a coordinator that crashes is restarted without the manager losing its handle", which holds for a single crash and stops holding at the restart intensity: strategy: :one_for_one with the default 3 restarts in 5 seconds. A coordinator in a persistent bad state, a heal pass that raises on the same corrupt segment every tick for instance, exhausts that and takes its supervisor down with it. That is the case nothing covers.

Reproduction

The manager takes its leading, spawn and stop as functions, so this needs no cluster:

{:ok, manager} =
  VnodeCoordinatorManager.start_link(
    leading: fn -> [:v1] end,
    spawn: fn _vnode -> spawn(fn -> Process.sleep(:infinity) end) end,
    stop: fn _handle -> :ok end,
    interval: 50
  )

[:v1] = VnodeCoordinatorManager.reconcile_now(manager)
# kill the pid that spawn returned, then:
[:v1] = VnodeCoordinatorManager.reconcile_now(manager)

The second call still answers [:v1] and no replacement is spawned, across as many reconciles as you care to run. Confirmed against main with a throwaway test.

Plan

A. Monitor the handle and prune on :DOWN. The manager monitors each pid start_vnodes/2 returns and drops the entry when it goes down, so the next reconcile sees the vnode as not running and starts it again. Smallest change, and it fits the existing design rather than working around it: the map becomes an accurate level for the loop that is already comparing against it. stop_vnodes/2 demonitors so a deliberate stop does not race a :DOWN.

B. Filter running by liveness on each pass. reconcile/1 treats an entry whose pid is dead as absent. Two lines, no new message handling, but it only notices on the next tick rather than at once, and Process.alive?/1 is a lie for a remote pid if these ever become remote.

C. Let the DynamicSupervisor restart it. Drop :temporary. This reintroduces the problem :temporary was chosen to avoid: a restart brings back coordinators for a vnode the node may no longer lead, and now the manager and the supervisor both believe they own the lifecycle. Listed to be rejected explicitly rather than rediscovered.

D. Do nothing. Defensible only if a coordinator exhausting its restart intensity is judged impossible, which is a claim about every current and future coordinator rather than about this code.

Risks and open questions

• Whether a supervisor death should respawn immediately or back off. A coordinator that crash-loops will take its supervisor down again, and an immediate respawn turns a bounded crash loop into an unbounded one one level up. Option A with a plain respawn inherits that; a backoff, or a bounded number of respawns per vnode before leaving it down and logging loudly, may be the better shape.
• Silence is the real defect here, more than the missing restart. A vnode whose coordinators are gone currently looks identical to one whose coordinators are fine. Whatever is chosen, this state should be visible: a log at minimum, and it is a candidate for /ready given that readiness on this branch now means the node can actually do its job.
• Nothing exercises this today. The manager's tests cover leadership arriving and leaving, not a handle dying underneath it.

Verification

• A test that kills the handle and asserts a replacement is started while the vnode is still led, which fails on main (see the reproduction above).
• A test that a deliberate stop does not trigger a respawn, so the fix does not fight stop_vnodes/2.
• Full suite, mix credo --strict, mix dialyzer, mix docs --warnings-as-errors, single-node and 3-node loadtests with zero errors.

Found while reviewing the infrastructure findings from the branch review, alongside the compose and workflow fixes that went in with them.