Read a filesystem that could not be named as unknown, never as a move (hq ADR 0325 sixth review)

A name read that timed out gave an empty name, which differed from the recorded one and so reset the record: a folder kept while empty asked again, a warranted one was walked again. An unnamed filesystem now leaves the folder exactly as it is, the access is said not known for that look, and it resumes once it is named.
2026-10-12 19:20:53 +02:00
parent 46c16e18fe
commit 2ff40109ac
4 changed files with 87 additions and 6 deletions
@@ -164,6 +164,9 @@ func judgeAccess(a judgedAccess) verdict {
 				What: apply.DiskWords(a.name)}}}
 	case apply.VerdictRoot:
 		return verdict{state: link.StateUnhealthy, reason: apply.RootWords(v.Why)}
+	case apply.VerdictUnknown:
+		// Not a fault and not the operator's: said as not known, and judged again on the next look.
+		return verdict{state: link.StateUnknown, reason: v.Why + "; judged again on the next look"}
 	case apply.VerdictBroken:
 		// Its words carry no path (AccessVerdict says them so): the reason reaches the operator's channel.
 		return verdict{state: link.StateUnhealthy, reason: v.Why}
@@ -189,6 +189,8 @@ const (
 	folderWaitDisk = "wait-disk"
 	// folderRoot is root's data on its placed filesystem: never kept.
 	folderRoot = "root"
+	// folderUnknown is a folder whose filesystem could not be named: left as it is until it is.
+	folderUnknown = "unknown"
 )

 // folderVerdict is the judgement of one folder, with what it read.
@@ -249,6 +251,13 @@ func assessFolder(ctx context.Context, path string, rec store.KeptFolder, taken
 	// Another filesystem at the same mount point (another pool, another disk) is a move too. A record from an earlier
 	// build names its filesystem another way, and is not compared: the next keep names it this way.
 	here := PlacedID(pl.Mount)
+	// **A filesystem that cannot be named now is unknown, never a move** (a name read that did not answer, a
+	// filesystem with no UUID, id or source): the folder holds what it is — nothing walked, nothing newly kept,
+	// nothing reset — and is judged again when it is named. One never kept is so never kept: it fails closed.
+	if here == "" {
+		return folderVerdict{kind: folderUnknown, mount: pl.Mount, unnamed: true,
+			why: "its filesystem could not be named this time (no UUID, filesystem id or source answered)"}
+	}
 	moved := pl.Moved || (rec.Fs != "" && rec.Mount == pl.Mount && rec.Placed != here)
 	facts, err := readFolder(path)
 	if err != nil {
@@ -257,11 +266,6 @@ func assessFolder(ctx context.Context, path string, rec store.KeptFolder, taken
 	if why := rootOwned(facts.UID, facts.GID); why != "" {
 		return folderVerdict{kind: folderRoot, mount: pl.Mount, why: "it is " + why}
 	}
-	// A filesystem that cannot be named is never kept: what is there could be anyone's (§8).
-	if here == "" {
-		return folderVerdict{kind: folderWaitKeep, mount: pl.Mount, moved: moved, unnamed: true,
-			why: "its filesystem cannot be named (no UUID, no filesystem id, no source)"}
-	}
 	// The controller's word that the folder was taken holds for the filesystem the warrant was carried out on: a
 	// folder moved to another is not taken, and asks again (§8).
 	takenHere := taken && (rec.TakenOn == here || (rec.TakenOn == "" && !moved))
@@ -520,6 +524,10 @@ func applyAccessFolder(ctx context.Context, r *declaration.Access, in inputs) (O
 		note += "; its grant is no longer acted on: the mesh puts no account in a group for data (novox/hq ADR 0325 §5)"
 	}
 	switch v.kind {
+	case folderUnknown:
+		out.Action = "waiting"
+		out.Detail = v.why + ": the folder is left exactly as it is, and judged again on the next apply" + note
+		return out, nil
 	case folderWaitDisk:
 		out.Action = "waiting"
 		out.Detail = "the disk that holds it is not there: nothing is made, and nothing mounts it until it is " +
@@ -792,6 +800,8 @@ const (
 	VerdictWaitDisk = "wait-disk"
 	VerdictRoot     = "root"
 	VerdictBroken   = "broken"
+	// VerdictUnknown is a folder whose filesystem could not be named this look: neither kept nor faulted.
+	VerdictUnknown = "unknown"
 	// VerdictNobody is a folder no identity uses: nothing is kept, and only a missing disk is said.
 	VerdictNobody = "nobody"
 )
@@ -813,6 +823,8 @@ func AccessVerdict(ctx context.Context, path string, taken, used bool) AccessJud
 		return AccessJudgement{Kind: VerdictBroken, Why: pathless(v.err)}
 	case v.kind == folderWaitDisk:
 		return AccessJudgement{Kind: VerdictWaitDisk}
+	case v.kind == folderUnknown:
+		return AccessJudgement{Kind: VerdictUnknown, Why: v.why}
 	case !used:
 		return AccessJudgement{Kind: VerdictNobody}
 	case v.kind == folderRoot:
@@ -179,6 +179,8 @@ func reconcileOne(ctx context.Context, path string, f store.KeptFolder) []string
 			why = pathless(v.err)
 		case v.kind == folderRoot:
 			why = RootWords(v.why)
+		case v.kind == folderUnknown:
+			why = v.why
 		case v.moved || v.kind == folderWaitKeep:
 			why = "what is at it now is not what the mesh kept there"
 		}
@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"

@@ -1370,7 +1371,7 @@ func TestNamingAHungFilesystemIsOneReadPerMount(t *testing.T) {
 		<-release
 		return ""
 	}
-	defer func() { close(release); MountUUID, statBound = was, wasBound }()
+	defer func() { close(release); flightsDone(t); MountUUID, statBound = was, wasBound }()
 	for i := 0; i < 5; i++ {
 		if id := PlacedID("/a/dead/mount"); id != "" {
 			t.Fatalf("a filesystem that does not answer is named %s", id)
@@ -1382,3 +1383,66 @@ func TestNamingAHungFilesystemIsOneReadPerMount(t *testing.T) {
 		t.Fatalf("%d reads of one mount were started while the first hung", started)
 	}
 }
+
+// Review (sixth): a name read that does not answer is unknown, never a move — a folder kept while empty and filled
+// since is not asked about again, a warranted one is not walked again, and nothing of either record is reset.
+func TestAFilesystemNotNamedThisTimeIsNeverAMove(t *testing.T) {
+	uuid, fsid, source := "6f1c0a4e-1b2d-4c3e-8f00-aa11bb22cc33", "0000080100000000", "/dev/sdb1"
+	_, lib, access := poolNamed(t, &uuid, &fsid, &source)
+	// The name's reads hang past their bound while hanging is set; installed once, before anything reads it.
+	var hanging atomic.Bool
+	release := make(chan struct{})
+	named := MountUUID
+	MountUUID = func(p string) string {
+		if hanging.Load() {
+			<-release
+		}
+		return named(p)
+	}
+	takeIt(t, lib, access)
+	taken := strings.Replace(access, `"taken":false`, `"taken":true`, 1)
+	before, _ := keptRecord(lib)
+
+	wasBound := statBound
+	statBound = 20 * time.Millisecond
+	hanging.Store(true)
+	report, _, err := applyAccesses(t, store.State{}, taken)
+	hanging.Store(false)
+	close(release)
+	flightsDone(t)
+	statBound = wasBound
+	if err != nil {
+		t.Fatal(err)
+	}
+	if o := outcomeOf(report, "radarr.media"); o.Action != "waiting" || !strings.Contains(o.Detail, "could not be named") {
+		t.Fatalf("a folder whose filesystem was not named this time is not said so: %+v", o)
+	}
+	after, _ := keptRecord(lib)
+	if !after.Kept || after.Placed != before.Placed || after.TakenOn != before.TakenOn || len(after.Left) != 0 ||
+		!sameUsers(after.Users, before.Users) {
+		t.Fatalf("the record was reset by a name not read:\nbefore %+v\nafter  %+v", before, after)
+	}
+
+	// Named again: kept as it was, with nothing asked and nothing walked.
+	waitFor(t, "the hung read returned", func() bool { return PlacedID(filepath.Dir(lib)) == before.Placed })
+	report, _, err = applyAccesses(t, store.State{}, taken)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if o := outcomeOf(report, "radarr.media"); o.Action == "waiting" {
+		t.Fatalf("named again, the folder asks again: %+v", o)
+	}
+	if r, _ := keptRecord(lib); r.Act == nil || r.Act.Act != "take" {
+		t.Errorf("the folder was walked again: %+v", r.Act)
+	}
+}
+
+// flightsDone waits until no read of a hung test is in flight: it goes on reading the stand-ins after its release.
+func flightsDone(t *testing.T) {
+	t.Helper()
+	waitFor(t, "the hung reads returned", func() bool {
+		inFlight.Lock()
+		defer inFlight.Unlock()
+		return len(inFlight.reads) == 0
+	})
+}