bedrock-kv / bedrock / 556aeeac68b301d8150b210001e11c75c615490b

82%

Build Type

push

github

Committed by web-flow

Commit Message

Watch hosted workers, so a dead one stops being reported as running (#222)

Closes bedrock-z2e, under epic bedrock-gu0.

A worker's health was recorded once, when it started, and never
revisited. Nothing monitored the process, so `{:ok, pid}` outlived the
process it named — and there was no monitor, no `trap_exit` and no
`:DOWN` clause anywhere under `service/foreman`: the catch-all
`handle_info/2` would have swallowed one regardless.

That stale value is load-bearing in three places:

| consumer | consequence |
|---|---|
| `do_relay_tsl/2` | layout pushes go to a dead pid — and that push is
how a worker learns it has been **displaced** |
| `do_get_all_running_services/1` | the coordinator keeps being told a
dead process is an available service |
| the health verdict | a fold over these values, so the foreman reports
`:ok` while hosting a corpse |

The third is what made the verdict settled in #218/#219 untrustworthy.
The second was not in the ticket — my failing tests found it.

### The interesting part: `:stopped` is not simply the right answer

Workers are `:transient`, so an abnormal exit is **restarted** under the
same OTP name. Marking a dead worker `:stopped` and leaving it there
would drop a *live* worker out of the roll call — trading a
dead-process-reported-running bug for a live-process-reported-gone bug.

Adopting the replacement at `:DOWN` time doesn't work either: the
`:DOWN` is delivered the instant the process dies, while the supervisor
must first handle its own `EXIT`. It lost that race in every test run.
So adoption is a bounded recheck on a timer, and only a worker still
`:stopped` is adopted — it can never overwrite health the worker
reported for itself.

### What the audit caught, and why my tests missed it

There are **two** adopters, not one. A worker that reports its own
health names a pid, and that report can name a different process than
the one being watched: Olivine's replacement casts `{:ok, self()}` as
soon as startup com... (continued)

Coverage Stats

36 of 41 new or added lines in 3 files covered. (87.8%)

3 existing lines in 1 file now uncovered.

6937 of 8675 relevant lines covered (79.97%)

1036.84 hits per line