Repair the lint blocker that let the store fix land on a red battery, and the failure path it uncovered: the branch written to fail loudly waited on a reader that a live helper never ends
This commit is contained in:
parent
7aded660ed
commit
3c0ecd5502
1 changed files with 29 additions and 9 deletions
|
|
@ -2,6 +2,7 @@ package store
|
|||
|
||||
import (
|
||||
"bufio"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"math"
|
||||
|
|
@ -44,10 +45,12 @@ import (
|
|||
|
||||
const (
|
||||
killLoopEnv = "TM_STORE_KILL_LOOP_DB"
|
||||
// killLoopRoundEnv scopes the helper's request hashes to its round. Checkpoints are idempotent by
|
||||
// request_hash (ON CONFLICT DO NOTHING), so two rounds that generated the same hash would settle
|
||||
// once between them, leave the count flat, and make the increment check below report a durability
|
||||
// failure that never happened. The round makes each round's writes unambiguously its own.
|
||||
// killLoopRoundEnv scopes the helper's request hashes to its round. The rest of a hash is the
|
||||
// helper's pid and a counter that restarts at zero every round, so two rounds can only collide when
|
||||
// the OS hands the second helper the pid the first one had. Checkpoints are idempotent by
|
||||
// request_hash (ON CONFLICT DO NOTHING), so that collision would settle once between the two rounds,
|
||||
// leave the count flat, and make the increment check below report a durability failure that never
|
||||
// happened. The round makes each round's writes unambiguously its own.
|
||||
killLoopRoundEnv = "TM_STORE_KILL_LOOP_ROUND"
|
||||
// killLoopReady is printed by the helper exactly once, after its FIRST settle+checkpoint has
|
||||
// committed. It is the whole liveness signal: everything the parent asserts afterwards is about work
|
||||
|
|
@ -111,12 +114,23 @@ type killHelper struct {
|
|||
drained <-chan struct{}
|
||||
}
|
||||
|
||||
// killAndReap SIGKILLs the helper and collects it in the order os/exec requires.
|
||||
// killAndReap SIGKILLs a helper that is expected to be running and collects it in the order os/exec
|
||||
// requires. A helper that has already exited means the round lost its subject before the kill landed,
|
||||
// so that is named rather than absorbed.
|
||||
func (h *killHelper) killAndReap(t *testing.T, round int) {
|
||||
t.Helper()
|
||||
if err := h.cmd.Process.Signal(syscall.SIGKILL); err != nil {
|
||||
t.Fatalf("round %d: SIGKILL: %v", round, err)
|
||||
}
|
||||
h.reap()
|
||||
}
|
||||
|
||||
// reap leaves nothing of the helper behind. The signal is best-effort because reap also serves the
|
||||
// paths where the helper may already have exited on its own; what it guarantees is that a live helper
|
||||
// is stopped BEFORE the reader is waited on. The reader drains until EOF, and a process still running
|
||||
// never produces one — so waiting for it first would hang the test where it means to fail it.
|
||||
func (h *killHelper) reap() {
|
||||
_ = h.cmd.Process.Signal(syscall.SIGKILL)
|
||||
<-h.drained // every read from the pipe has completed…
|
||||
_ = h.cmd.Wait() // …and only now may Wait close it
|
||||
}
|
||||
|
|
@ -156,7 +170,14 @@ func startKillHelper(t *testing.T, round int, dbPath string) *killHelper {
|
|||
}
|
||||
}
|
||||
if !seen {
|
||||
ready <- fmt.Errorf("helper exited without ever committing a checkpoint (scanner: %v)", sc.Err())
|
||||
// A scanner failure and a clean EOF are different facts about the helper — one is a broken
|
||||
// read, the other a process that ran and said nothing — and the reader keeps them apart so
|
||||
// the parent's diagnosis names which one happened.
|
||||
if err := sc.Err(); err != nil {
|
||||
ready <- fmt.Errorf("the helper's stdout could not be read to the end: %w", err)
|
||||
} else {
|
||||
ready <- errors.New("the helper's stdout reached a clean EOF without the commit marker")
|
||||
}
|
||||
}
|
||||
// An over-long line would have stopped the scanner above; keep draining regardless so a stalled
|
||||
// pipe can never be mistaken for a stalled store.
|
||||
|
|
@ -166,12 +187,11 @@ func startKillHelper(t *testing.T, round int, dbPath string) *killHelper {
|
|||
select {
|
||||
case err := <-ready:
|
||||
if err != nil {
|
||||
<-drained
|
||||
_ = cmd.Wait()
|
||||
h.reap()
|
||||
t.Fatalf("round %d: %v — there was nothing for SIGKILL to land on, so everything asserted afterwards would be vacuous", round, err)
|
||||
}
|
||||
case <-time.After(killLoopReadyWait):
|
||||
h.killAndReap(t, round)
|
||||
h.reap()
|
||||
t.Fatalf("round %d: the helper did not commit a first checkpoint within %s — the kill would have landed on a process that had done nothing, and everything asserted afterwards would be vacuous",
|
||||
round, killLoopReadyWait)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue