Unwind the hash worker pool instead of abandoning it (closes #6)
All checks were successful
check / check (push) Successful in 1m2s
All checks were successful
check / check (push) Successful in 1m2s
hashPhase returned the moment recordRun failed and left the pool running: the feeder parked forever on a full jobs channel and every worker on a full results channel. Until #4 landed the process exited before that mattered; now that runScan returns an error and unwinds, the goroutines are a real leak. The pool is now an owned hashPool. Its context is derived from the scan's, every blocking send in the feeder and the workers selects on ctx.Done(), the feeder closes jobs on every path out so the workers' range always terminates, and hashPhase defers pool.stop(), which cancels and then drains results until the last goroutine has exited. Draining is the half that matters: a worker already parked on a send cannot observe the cancellation until a receiver frees it. ctx comes from cmd.Context() and is threaded through runScan, syncScan, both worker pools and the database layer as the first parameter throughout, so graceful interrupt handling has a path to hook into rather than a pool to rewrite. The walk pool never leaked, because walkPhase always drains its events to close, but it has the same unbounded-send shape and gets the same treatment, together with a ctx.Err() guard after the walk: a cancelled walk leaves a partial size census, and the update phase would read every file it never reached as vanished and delete its record. Tests drive the scan entry point against a database whose insert trigger aborts, with a fixture large enough that the failure lands partway through the hash phase with more runs queued than either pool channel can hold, and assert that the scan fails instead of hanging and that runtime.NumGoroutine polls back to its pre-scan baseline.
This commit is contained in:
193
scan_test.go
193
scan_test.go
@@ -1,14 +1,18 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"database/sql"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -130,7 +134,7 @@ func collectWalk(t *testing.T, roots []string, oneFS bool,
|
||||
errs int
|
||||
)
|
||||
|
||||
for ev := range startWalk(roots, oneFS, workers) {
|
||||
for ev := range startWalk(t.Context(), roots, oneFS, workers) {
|
||||
if ev.fail {
|
||||
errs++
|
||||
|
||||
@@ -359,7 +363,7 @@ const smokeTreeFiles = 15
|
||||
func syncTree(t *testing.T, db *sql.DB, roots ...string) scanStats {
|
||||
t.Helper()
|
||||
|
||||
st, err := syncScan(db, roots, 4, false)
|
||||
st, err := syncScan(t.Context(), db, roots, 4, false)
|
||||
if err != nil {
|
||||
t.Fatalf("syncScan: %v", err)
|
||||
}
|
||||
@@ -371,7 +375,7 @@ func syncTree(t *testing.T, db *sql.DB, roots ...string) scanStats {
|
||||
func dbRecords(t *testing.T, db *sql.DB) []scanRec {
|
||||
t.Helper()
|
||||
|
||||
recs, err := loadFileRows(db)
|
||||
recs, err := loadFileRows(t.Context(), db)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -815,6 +819,189 @@ func TestScanHardlinkRunFailsTogether(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// injectedWriteFailure is the message the injected database trigger
|
||||
// aborts with, so the test can recognize its own failure in the error
|
||||
// the scan reports.
|
||||
const injectedWriteFailure = "injected write failure"
|
||||
|
||||
// hashLeakFiles is the size of the fixture for the hash-phase failure
|
||||
// test. The batch commit inside the hash phase is what fails, so the
|
||||
// tree must hold more than updateBatchSize files for the failure to
|
||||
// happen at all; the surplus over that is what is still queued when it
|
||||
// does, and it exceeds the depth of both pool channels so that the
|
||||
// workers have nowhere left to put their results. An abandoned pool
|
||||
// therefore parks forever, which is exactly what this test detects.
|
||||
const hashLeakFiles = updateBatchSize + 2*workQueueDepth
|
||||
|
||||
// hashLeakWorkers is the worker count for that scan: a fixed, modest
|
||||
// number keeps the leak deterministic on any machine.
|
||||
const hashLeakWorkers = 4
|
||||
|
||||
// goroutineSettle bounds how long a goroutine count is given to come
|
||||
// back down to its target. Only a failing run ever waits this long.
|
||||
const goroutineSettle = 5 * time.Second
|
||||
|
||||
// goroutinePoll is the interval between goroutine-count samples.
|
||||
const goroutinePoll = 10 * time.Millisecond
|
||||
|
||||
// writeEmptyFiles creates n empty files directly in dir. Zero-length
|
||||
// files are never opened by the hasher — their hashes are constant —
|
||||
// so a fixture this size costs directory entries and no read I/O,
|
||||
// while still queueing n runs through the hash pool.
|
||||
func writeEmptyFiles(t *testing.T, dir string, n int) {
|
||||
t.Helper()
|
||||
|
||||
for i := range n {
|
||||
err := os.WriteFile(
|
||||
filepath.Join(dir, strconv.Itoa(i)), nil, 0o600)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// injectWriteFailure creates a scan database at path carrying the real
|
||||
// schema plus a trigger that aborts every insert. Reads are untouched,
|
||||
// so a scan loads its index and walks normally and then fails on the
|
||||
// first record it tries to commit — a genuine database write failure
|
||||
// partway through the hash phase.
|
||||
func injectWriteFailure(t *testing.T, path string) {
|
||||
t.Helper()
|
||||
|
||||
db, err := openScanDatabase(t.Context(), path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, err = db.ExecContext(t.Context(),
|
||||
"CREATE TRIGGER refuse_insert BEFORE INSERT ON files "+
|
||||
"BEGIN SELECT RAISE(ABORT, '"+injectedWriteFailure+"'); END")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
err = db.Close()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
// baselineGoroutines waits for the goroutine count to stop moving and
|
||||
// returns it. Handles closed by earlier tests take a moment to reap
|
||||
// their driver goroutines, so a single sample would make the baseline
|
||||
// itself flaky.
|
||||
func baselineGoroutines(t *testing.T) int {
|
||||
t.Helper()
|
||||
|
||||
deadline := time.Now().Add(goroutineSettle)
|
||||
last := runtime.NumGoroutine()
|
||||
|
||||
for time.Now().Before(deadline) {
|
||||
time.Sleep(goroutinePoll)
|
||||
|
||||
n := runtime.NumGoroutine()
|
||||
if n == last {
|
||||
return n
|
||||
}
|
||||
|
||||
last = n
|
||||
}
|
||||
|
||||
return last
|
||||
}
|
||||
|
||||
// settledGoroutines polls runtime.NumGoroutine until it is back at or
|
||||
// below want and returns the last count seen. Polling, rather than one
|
||||
// sample after a fixed sleep, is what keeps this from being a race
|
||||
// between the assertion and goroutines that are already exiting.
|
||||
func settledGoroutines(t *testing.T, want int) int {
|
||||
t.Helper()
|
||||
|
||||
deadline := time.Now().Add(goroutineSettle)
|
||||
|
||||
for {
|
||||
n := runtime.NumGoroutine()
|
||||
if n <= want || time.Now().After(deadline) {
|
||||
return n
|
||||
}
|
||||
|
||||
time.Sleep(goroutinePoll)
|
||||
}
|
||||
}
|
||||
|
||||
// TestScanHashWriteFailureUnwindsPool drives the scan entry point
|
||||
// against a database that refuses every write. The hash phase gives up
|
||||
// partway through with thousands of runs still queued, which used to
|
||||
// leave the feeder parked on a full job channel and every worker parked
|
||||
// on a full result channel for the life of the process.
|
||||
func TestScanHashWriteFailureUnwindsPool(t *testing.T) {
|
||||
path := testDBPath(t)
|
||||
t.Setenv(databaseEnv, path)
|
||||
|
||||
dir := t.TempDir()
|
||||
|
||||
writeEmptyFiles(t, dir, hashLeakFiles)
|
||||
injectWriteFailure(t, path)
|
||||
|
||||
base := baselineGoroutines(t)
|
||||
|
||||
var stderr bytes.Buffer
|
||||
|
||||
code := run([]string{
|
||||
cmdScan, "--workers", strconv.Itoa(hashLeakWorkers), dir,
|
||||
}, &stderr)
|
||||
if code != exitFatal {
|
||||
t.Fatalf("run(scan) = %d, want %d; stderr: %s",
|
||||
code, exitFatal, stderr.String())
|
||||
}
|
||||
|
||||
if !strings.Contains(stderr.String(), injectedWriteFailure) {
|
||||
t.Errorf("stderr = %q, want the injected write failure",
|
||||
stderr.String())
|
||||
}
|
||||
|
||||
if got := settledGoroutines(t, base); got > base {
|
||||
t.Errorf("goroutines = %d after the failed scan, want %d back",
|
||||
got, base)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSyncScanCancelledWalkKeepsRecords checks the other half of the
|
||||
// cancellation path: a scan whose context is already cancelled stops
|
||||
// with that error instead of treating its truncated walk as the truth
|
||||
// and deleting the record of every file it never reached.
|
||||
//
|
||||
//nolint:paralleltest // counts goroutines: must not run beside others
|
||||
func TestSyncScanCancelledWalkKeepsRecords(t *testing.T) {
|
||||
dir := buildSmokeTree(t)
|
||||
db := openTestDB(t)
|
||||
|
||||
syncTree(t, db, dir)
|
||||
|
||||
before := recordPaths(dbRecords(t, db))
|
||||
|
||||
ctx, cancel := context.WithCancel(t.Context())
|
||||
cancel()
|
||||
|
||||
base := baselineGoroutines(t)
|
||||
|
||||
_, err := syncScan(ctx, db, []string{dir}, 4, false)
|
||||
if !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("syncScan on a cancelled context = %v, want %v",
|
||||
err, context.Canceled)
|
||||
}
|
||||
|
||||
if got := recordPaths(dbRecords(t, db)); !slices.Equal(got, before) {
|
||||
t.Errorf("records = %q after a cancelled scan, want %q",
|
||||
got, before)
|
||||
}
|
||||
|
||||
if got := settledGoroutines(t, base); got > base {
|
||||
t.Errorf("goroutines = %d after the cancelled scan, want %d back",
|
||||
got, base)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHashRuns(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user