bedrock-kv / bedrock / 026c4c55ed565b10e653b23c47c252f7c94c4f90

82%

Build Type

push

github

Committed by web-flow

Commit Message

Settle the foreman's health at spin-up (#218)

Closes bedrock-mxt. **Stacked on #213** — it also edits `do_spin_up/1`.
GitHub will retarget this to `develop` when #213 merges.

`do_spin_up/1` loaded workers from disk, started them, and relayed the
layout — but never recomputed health. `recompute_health/1` is reachable
only from `do_worker_health/3`, `do_remove_worker/2` and
`do_remove_workers/2`, so after a successful boot the state kept the
`:starting` from `State.new_state/1`, however cleanly every worker
started.

The consequence is worst where there is no other path to a verdict: the
only sender of `{:worker_health, ...}` is `Materializer.Olivine.Server`,
and **Shale never calls `Foreman.report_health/3`**. On a node with only
the `:log` capability nothing would ever trigger the sole remaining
recompute, so `wait_for_healthy/2` could not return `:ok` — not slowly,
but never.

### Two corrections the audit forced
- **I dropped the `notify_waiting_for_healthy/1` call.** My
justification for it — "a caller can park before spin-up runs" — is
provably false: this runs in the `:spin_up` `handle_continue`, which
precedes every mailbox message, so the `handle_call` that is the only
writer of `waiting_for_healthy` cannot have run. The list is always `[]`
here. A caller whose request is already queued finds health settled when
it is served. The line was dead code with a false comment; the comment
now explains why no notify is needed.
- **The tests only covered the empty-directory path**, so the headline
scenario was untested. There is now a test with a log worker that really
starts, driving the verdict to `:ok`, plus one where a worker cannot
start and the foreman must not report healthy. Four of five tests fail
against the pre-fix code.

### What `:ok` means here is weaker than the docstring promises
`try_to_start_worker/3` concludes `{:ok, pid}` once the process
registers its name. For Olivine that is after `init/1`, which returns
immediately and defers r... (continued)

Coverage Stats

1 of 1 new or added line in 1 file covered. (100.0%)

2 existing lines in 1 file now uncovered.

6893 of 8696 relevant lines covered (79.27%)

1034.5 hits per line