Merge pull request 'A check's store needs no durability; a starting holder removes what earlier holders left (hq issue 306)' (#133) from fix/a-check-store-needs-no-durability into main

cmd/mesh-builder/holder.go

@@ -274,6 +274,22 @@ func (h *holder) removeContainers(id string) (int, error) {
 	return builder.RemoveContainersOf(cleanup, h.remove, id)
 }
 
+// removeLeftBehind removes, once, what earlier holders on this machine left — called before anything is
+// taken, so none of it is this holder's — and says what it did. A runtime that cannot be asked is said
+// and the holder takes work all the same: what is left is waste, not a reason to stop building.
+func (h *holder) removeLeftBehind() int {
+	cleanup, stop := context.WithTimeout(context.Background(), killRemoves)
+	defer stop()
+	n, err := builder.RemoveLeftBehind(cleanup, h.remove)
+	switch {
+	case err != nil:
+		fmt.Fprintf(os.Stderr, "could not look for containers earlier builds left on %s: %v\n", h.on, err)
+	case n > 0:
+		fmt.Fprintf(os.Stderr, "removed %d container(s) earlier builds left on %s\n", n, h.on)
+	}
+	return n
+}
+
 // returned records that the build's work ended, and says whether a kill came first — only then is
 // the build killed; an error it ended with on its own is its own outcome.
 func (h *holder) returned(r *running) bool {

cmd/mesh-builder/holder_test.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"strings"
 	"testing"
 
@@ -146,3 +147,26 @@ func TestAKillArrivingAfterTheBuildEndedIsRefusedAndAnUnannouncedKillSaysSo(t *t
 		t.Fatalf("kill said %q (%v)", said, err)
 	}
 }
+
+// **Issue 306**: a holder starting removes the containers earlier builds left on its machine, through
+// the runner a kill uses, and a runtime that cannot be asked does not stop it.
+func TestAHolderStartingRemovesWhatEarlierBuildsLeftHere(t *testing.T) {
+	h := newHolder("novox", link.TheBuildMachine, t.TempDir(), nil)
+	var removed []string
+	h.remove = func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		removed = append(removed, name+" "+strings.Join(args, " "))
+		if args[0] == "ps" {
+			return "c1 build-1\nc2 build-1\nc3 check-here-2\n", nil
+		}
+		return "", nil
+	}
+	if n := h.removeLeftBehind(); n != 2 || len(removed) != 2 || removed[1] != "docker rm -f c1 c2" {
+		t.Fatalf("removed %d: %v", n, removed)
+	}
+	h.remove = func(context.Context, string, string, ...string) (string, error) {
+		return "", errors.New("no runtime")
+	}
+	if n := h.removeLeftBehind(); n != 0 {
+		t.Errorf("a runtime that cannot be asked removed %d", n)
+	}
+}

cmd/mesh-builder/main.go

@@ -129,6 +129,9 @@ func run() error {
 	if h.Paused() {
 		fmt.Fprintf(os.Stderr, "paused (kept in %s): taking no build until resumed\n", pausedFile(workspace))
 	}
+	// **What an earlier holder here left goes before anything is taken** (novox/hq issue 306): a holder
+	// recreated mid-check left the check's store and bus running, and the ask went to another machine.
+	h.removeLeftBehind()
 	machine := link.MachineOverNATSWith(js, on, seat, link.MachineOptions{Paused: h.Paused})
 	defer machine.Close()
 

internal/builder/check.go

@@ -346,7 +346,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 	}()
 	labelled := Labelled(run, spec.ID)
 	store, err := throwaway(ctx, labelled, run, spec.ID+"-store", StoreImage(f.Versions.Store), 5432,
-		[]string{"-e", "POSTGRES_PASSWORD=check"}, nil)
+		[]string{"-e", "POSTGRES_PASSWORD=check"}, StoreSettings)
 	if err != nil {
 		return CheckVerdict{}, err
 	}
@@ -848,6 +848,16 @@ func StoreImage(version string) string {
 	return "postgres:" + major + "-alpine"
 }
 
+// StoreSettings are the throwaway store's server settings: **no durability** (novox/hq issue 306). The
+// store lives for one check and is removed after it, so nothing it writes has to survive a crash —
+// and a test suite that creates a database per test asks the disk to flush thousands of times. On a
+// machine whose disk the mesh's own store, bus and forge already keep busy — the control node — each
+// flush waited its turn: the store-bound packages of the controller's suite ran five to seven times
+// slower there than on any other holder, while those that touch no store ran alike, and its check ran
+// past the suite's thirty minutes every time it landed there. The release stays the mesh's; only what
+// a crash would need is switched off.
+var StoreSettings = []string{"-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"}
+
 // BusImage is the bus a check stands on: the release the mesh's bus server runs.
 func BusImage(version string) string {
 	v := strings.TrimPrefix(strings.TrimSpace(version), "v")

internal/builder/check_test.go

@@ -33,6 +33,43 @@ func TestTheStoreAndBusAChecksStandsOnAreTheOnesTheMeshRuns(t *testing.T) {
 	}
 }
 
+// **Issue 306**: the throwaway store is raised without durability — it lives for one check — so a suite
+// that makes a database per test does not wait on the disk of a busy machine for every flush.
+func TestTheThrowawayStoreFlushesNothing(t *testing.T) {
+	said := strings.Join(StoreSettings, " ")
+	for _, want := range []string{"-c fsync=off", "-c synchronous_commit=off", "-c full_page_writes=off"} {
+		if !strings.Contains(said, want) {
+			t.Errorf("the store is raised with %q, not %q", said, want)
+		}
+	}
+	if os.Getenv("MESH_TEST_DOCKER") != "1" {
+		t.Skip("MESH_TEST_DOCKER=1: the settings are proven on a raised store")
+	}
+	id := fmt.Sprintf("check-store-%d", time.Now().UnixNano())
+	labelledRun := Labelled(Command, id)
+	t.Cleanup(func() { _, _ = RemoveContainersOf(context.Background(), Command, id) })
+	if _, err := throwaway(t.Context(), labelledRun, Command, id+"-store", StoreImage("17.11"), 5432,
+		[]string{"-e", "POSTGRES_PASSWORD=check"}, StoreSettings); err != nil {
+		t.Fatal(err)
+	}
+	if err := waitFor(t.Context(), Command, id+"-store", []string{"pg_isready", "-U", "postgres"}); err != nil {
+		t.Fatal(err)
+	}
+	for setting, want := range map[string]string{"fsync": "off", "synchronous_commit": "off", "full_page_writes": "off"} {
+		var got string
+		for try := 0; try < 10; try++ {
+			out, err := exec.Command("docker", "exec", id+"-store", "psql", "-U", "postgres", "-tAc", "SHOW "+setting).CombinedOutput()
+			if got = strings.TrimSpace(string(out)); err == nil {
+				break
+			}
+			time.Sleep(500 * time.Millisecond)
+		}
+		if got != want {
+			t.Errorf("%s is %q on the raised store", setting, got)
+		}
+	}
+}
+
 // aRepository is a git repository holding these files, committed, and its head.
 func aCheckedRepository(t *testing.T, files map[string]string) (string, string) {
 	t.Helper()

internal/builder/kill.go

@@ -76,3 +76,53 @@ func RemoveContainersOf(ctx context.Context, run Runner, id string) (int, error)
 	}
 	return len(ids), nil
 }
+
+// A holder that stops mid-build leaves its containers running (novox/hq issue 306).
+//
+// A build's containers are removed by the build itself, by a kill, or by the next delivery of the same
+// ask — but only on the machine that delivery reaches. A holder recreated by a rollout while a check
+// ran left the check's throwaway store and bus running, the ask went to another machine, and nothing
+// on the first one ever looked at them again: eleven pairs across four machines in two days, each a
+// postgres that had written its share of the disk. A holder starting has taken nothing yet, so every
+// container labelled with an ask of the seat is one an earlier holder here left, and is removed then.
+
+// RemoveLeftBehind removes every container on this machine labelled with an ask of the build seat —
+// an id of the seat's shape, `build-<n>` — and says how many. Called by a holder before it takes
+// anything, so none of them can be a build it runs. A container labelled with any other id — a check
+// a person runs by hand (`check-here-…`), a test's — is not the seat's, and is left.
+func RemoveLeftBehind(ctx context.Context, run Runner) (int, error) {
+	out, err := run(ctx, "", "docker", "ps", "-a", "--filter", "label="+BuildLabel,
+		"--format", `{{.ID}} {{.Label "`+BuildLabel+`"}}`)
+	if err != nil {
+		return 0, err
+	}
+	var ids []string
+	for _, line := range strings.Split(out, "\n") {
+		fields := strings.Fields(line)
+		if len(fields) == 2 && anAskOfTheSeat(fields[1]) {
+			ids = append(ids, fields[0])
+		}
+	}
+	if len(ids) == 0 {
+		return 0, nil
+	}
+	if _, err := run(ctx, "", "docker", append([]string{"rm", "-f"}, ids...)...); err != nil {
+		return 0, err
+	}
+	return len(ids), nil
+}
+
+// anAskOfTheSeat is whether an id is of the shape the controller gives the seat's asks: `build-` and the
+// moment it was asked, in nanoseconds.
+func anAskOfTheSeat(id string) bool {
+	n, ok := strings.CutPrefix(id, "build-")
+	if !ok || n == "" {
+		return false
+	}
+	for _, c := range n {
+		if c < '0' || c > '9' {
+			return false
+		}
+	}
+	return true
+}

internal/builder/kill_test.go

@@ -101,3 +101,40 @@ func TestAKillRemovesTheContainersLabelledWithTheBuild(t *testing.T) {
 		t.Errorf("ran %v", ran)
 	}
 }
+
+// **Issue 306**: a holder starting removes what earlier holders on its machine left — every container
+// labelled with an ask of the seat — and nothing labelled otherwise.
+func TestAStartingHolderRemovesWhatEarlierHoldersLeftAndNothingElse(t *testing.T) {
+	var ran [][]string
+	run := func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		ran = append(ran, append([]string{name}, args...))
+		if args[0] == "ps" {
+			return "c1 build-1791418000320743455\nc2 build-1791418000320743455\n" +
+				"c3 check-here-1791418000320743455\nc4 check-test-17\nc5 build-x\nc6 build-\n\nc7 build-1791319895953076871\n", nil
+		}
+		return "", nil
+	}
+	n, err := RemoveLeftBehind(context.Background(), run)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if n != 3 {
+		t.Errorf("removed %d", n)
+	}
+	if len(ran) != 2 || !reflect.DeepEqual(ran[1], []string{"docker", "rm", "-f", "c1", "c2", "c7"}) {
+		t.Fatalf("ran %v", ran)
+	}
+	if !strings.Contains(strings.Join(ran[0], " "), "--filter label=mesh.build") {
+		t.Errorf("listed with %v", ran[0])
+	}
+
+	// Nothing left behind: nothing removed, and no rm asked.
+	ran = nil
+	quiet := func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		ran = append(ran, append([]string{name}, args...))
+		return "c3 check-here-1\n", nil
+	}
+	if n, err := RemoveLeftBehind(context.Background(), quiet); err != nil || n != 0 || len(ran) != 1 {
+		t.Errorf("with nothing of the seat's: %d %v %v", n, err, ran)
+	}
+}