bedrock-kv / bedrock / 5a520c967f063d01f1e728970e1aa01f9eeb534f

82%

Build Type

push

github

Committed by web-flow

Commit Message

Make recomputing health and waking waiters one act (#219)

Closes bedrock-n5h, under epic bedrock-gu0. **Stacked on #218**, itself
stacked on #213. GitHub retargets as each merges.

`do_remove_worker/2` and `do_remove_workers/2` recomputed the verdict
but never called `notify_waiting_for_healthy/1`. Only
`do_worker_health/3` paired them — by hand.

So removing the last failing worker flipped health to `:ok` and told
nobody. A caller parked in `wait_for_healthy/2` (default timeout
`:infinity`) slept through the exact moment its condition became true.

### The audit found the same gap in the opposite direction

`do_new_worker/4` is a **fourth** writer of `t.workers` and recomputed
nothing at all. A fresh node folds an empty worker set to `:ok`, so
adding a worker that then fails to start left the foreman reporting
healthy while hosting a dead one — and `do_wait_for_healthy/2`
short-circuits `:ok` immediately. Nothing corrected it, since recompute
is otherwise reachable only from a worker's own health cast and Shale
never sends one.

I had written a docstring claiming *"every health change goes through
here"* while only three of four writers did. Rather than weaken the
claim, this PR makes it true: `settle_health/1` now covers all four.

### Why a helper rather than four fixed call sites

The pairing stops being something a call site can forget. Forgetting it
is precisely how three of the four came to differ from each other.

Spin-up uses it too, though no waiter can exist there — it runs in the
`:spin_up` `handle_continue`, which precedes every mailbox message. The
notify is a no-op today and is there anyway, because "provably no
waiters" is a property of the current call ordering rather than of that
code.

### Verification

2821 tests, 0 failures. `mix compile --warnings-as-errors`, format,
credo and dialyzer all clean. Two of the four original tests fail
against the pre-fix code; the `do_new_worker` test fails against the
un-fixed version of that path... (continued)

Coverage Stats

5 of 6 new or added lines in 1 file covered. (83.33%)

4 existing lines in 1 file now uncovered.

6927 of 8698 relevant lines covered (79.64%)

1025.0 hits per line