nfs-server: the look below a share runs beside the pass, so the status is written every pass (issue 1971)

A walk of minutes inside the pass held the status back until the module was found unhealthy and the assigner held the version. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig
2026-11-05 09:39:24 +01:00
parent 211186edd7
commit b0bdcd02c9
5 changed files with 119 additions and 24 deletions
@@ -105,8 +105,10 @@ type Machine struct {
 	// ownerOf reads a file's owner from its inode; nil is the inode as the kernel says it. Tests set it
 	// so the folders they make stand for a non-root owner, whoever runs them.
 	ownerOf func(info os.FileInfo) (uid, gid int)
-	// clean is when a look below each real path last found no backup repository (LookKept).
-	clean   map[string]time.Time
+	// Beside runs a look below a share beside the pass; nil is a goroutine. Tests run it in place.
+	Beside func(func())
+	// looks is the last look below each real path (LookKept).
+	looks   map[string]*lookBelow
 	cleanMu sync.Mutex
 	// walk is the running pass's context, which ends the look below a share when the process stops.
 	walk   context.Context
@@ -32,7 +32,7 @@ func newFake(t *testing.T) *fake {
 }

 func (f *fake) machine() *Machine {
-	return &Machine{Root: f.root, UID: f.uid, owners: f.owners, ownerOf: asTheLibrarysOwner, Now: func() time.Time { return f.now }, Run: func(_ context.Context, name string, args ...string) Ran {
+	return &Machine{Root: f.root, UID: f.uid, owners: f.owners, ownerOf: asTheLibrarysOwner, Beside: func(f func()) { f() }, Now: func() time.Time { return f.now }, Run: func(_ context.Context, name string, args ...string) Ran {
 		line := strings.Join(append([]string{name}, args...), " ")
 		f.ran = append(f.ran, line)
 		if (line == "exportfs -ra" || line == "sudo -n exportfs -ra") && f.onExportfs != nil {
@@ -61,9 +61,8 @@ func (m *Machine) Pass(ctx context.Context, last *Status) Status {
 	if _, note := m.Backup(); note != "" {
 		s.Warnings = append(s.Warnings, note)
 	}
-	m.setWalk(ctx)
+	m.setWalk(ctx) // stays set: a look beside the pass ends when the process does
 	exports, refused, said := m.Judge(c, roster, wants)
-	m.setWalk(nil)
 	if ctx.Err() != nil {
 		// Stopping: write nothing and reload nothing, so Stopped starts at once.
 		s.Problems = append(s.Problems, "the pass was ended by the process stopping; nothing was written")
@@ -432,3 +432,54 @@ func TestAHangingCommandStillLetsStoppedRemoveTheFileInTime(t *testing.T) {
 		t.Errorf("the export file stays after an unassignment whose commands hung")
 	}
 }
+
+// Issue 1971: the look below a share runs beside the pass. A pass whose walk has not finished still writes
+// its status, refuses the share until a look has finished, keeps believing a clean look while it is renewed,
+// and a walk ended by the process stopping leaves no result.
+func TestTheLookBelowRunsBesideThePassAndNeverHoldsItsStatus(t *testing.T) {
+	f := newFake(t)
+	f.mountinfo("/")
+	f.dir("/srv/clean/films/one")
+	f.file(ConfigFile, grantsConfig("clean=/srv/clean", "clean=ws:rw"))
+	f.roster("ws 10.0.0.3")
+	f.wants(map[string][]Want{"ws": {{Share: "clean", Access: "rw"}}})
+	m := f.machine()
+	var walks []func()
+	m.Beside = func(w func()) { walks = append(walks, w) } // the walk is still running
+	now := time.Date(2026, 11, 5, 5, 0, 0, 0, time.UTC)
+	m.Now = func() time.Time { return now }
+
+	s := m.Pass(context.Background(), nil)
+	if !s.At.Equal(now) || f.read(StatusFile) == "" || len(walks) != 1 {
+		t.Fatalf("the pass waited for the walk: at %v, walks %d", s.At, len(walks))
+	}
+	if !strings.Contains(refusedWhy(s, "clean"), "not been looked through yet") {
+		t.Errorf("a share whose look is unfinished is exported: %+v", s.Refused)
+	}
+	m.Pass(context.Background(), &s)
+	if len(walks) != 1 {
+		t.Errorf("a second walk started while one runs: %d", len(walks))
+	}
+	walks[0]()
+	if s = m.Pass(context.Background(), &s); refusedWhy(s, "clean") != "" {
+		t.Fatalf("the finished look was not used: %+v", s.Refused)
+	}
+
+	now = now.Add(LookKept)
+	s = m.Pass(context.Background(), &s)
+	if len(walks) != 2 || refusedWhy(s, "clean") != "" {
+		t.Errorf("the renewal must run beside the pass and the clean look stand: walks %d, %+v", len(walks), s.Refused)
+	}
+	ctx, cancel := context.WithCancel(context.Background())
+	m.Pass(ctx, &s)
+	cancel()
+	f.restic("/srv/clean/films/one/backup")
+	walks[1]()
+	if s = m.Pass(context.Background(), &s); refusedWhy(s, "clean") != "" {
+		t.Errorf("a walk ended by the stop left a result: %+v", s.Refused)
+	}
+	walks[len(walks)-1]()
+	if s = m.Pass(context.Background(), &s); !strings.Contains(refusedWhy(s, "clean"), "/srv/clean/films/one/backup") {
+		t.Errorf("the repository was not found by the next look: %+v", s.Refused)
+	}
+}
@@ -244,33 +244,72 @@ const (
 )

 // LookKept is how long a look below a share that found no backup repository is believed. The look walks up
-// to WalkLimit folders, which on a large library is too much for every 30 s pass; a repository put under a
+// to WalkLimit folders, which on a large library takes minutes on a cold disk; a repository put under a
 // share is found within this long. A look that found one, or was cut short, is never kept: the share stays
 // refused and is looked at again on the next pass.
 const LookKept = time.Hour

-// repositoryBelow finds a backup repository within WalkDepth levels below dir. complete is false when
-// the look stopped at its bound before it saw everything.
-func (m *Machine) repositoryBelow(dir string) (found string, complete bool) {
-	now := time.Now()
-	if m.Now != nil {
-		now = m.Now()
+// lookBelow is the last look below one folder, and whether another runs now.
+type lookBelow struct {
+	found    string
+	complete bool
+	at       time.Time
+	done     bool // a look has finished; until then there is no result
+	running  bool
+}
+
+// repositoryBelow answers the last look below dir for a backup repository, and starts the next one beside
+// the caller when there is none, or it was not clean, or LookKept has passed. The pass therefore never waits
+// for the walk (issue 1971: a walk of minutes held the status's time back until the module was found
+// unhealthy). pending is true while no look has finished yet; the share is refused until then, and while a
+// kept clean look is being renewed the old result stands. complete is false when the look stopped at its
+// bound or with the process.
+func (m *Machine) repositoryBelow(dir string) (found string, complete, pending bool) {
+	m.cleanMu.Lock()
+	r := m.looks[dir]
+	if r == nil {
+		if m.looks == nil {
+			m.looks = map[string]*lookBelow{}
+		}
+		r = &lookBelow{}
+		m.looks[dir] = r
+	}
+	fresh := r.done && r.found == "" && r.complete && m.clock().Sub(r.at) < LookKept
+	start := !fresh && !r.running
+	if start {
+		r.running = true
+	}
+	m.cleanMu.Unlock()
+	if start {
+		walk := func() {
+			f, c := m.walkBelow(dir)
+			m.cleanMu.Lock()
+			defer m.cleanMu.Unlock()
+			r.running = false
+			if w := m.walking(); w != nil && w.Err() != nil {
+				return // ended by the process stopping: no result, the old one stands
+			}
+			r.found, r.complete, r.at, r.done = f, c, m.clock(), true
+		}
+		if m.Beside != nil {
+			m.Beside(walk)
+		} else {
+			go walk()
+		}
 	}
 	m.cleanMu.Lock()
 	defer m.cleanMu.Unlock()
-	if at, kept := m.clean[dir]; kept && now.Sub(at) < LookKept {
-		return "", true
+	if !r.done {
+		return "", false, true
 	}
-	found, complete = m.walkBelow(dir)
-	if found == "" && complete {
-		if m.clean == nil {
-			m.clean = map[string]time.Time{}
-		}
-		m.clean[dir] = now
-	} else {
-		delete(m.clean, dir)
+	return r.found, r.complete, false
+}
+
+func (m *Machine) clock() time.Time {
+	if m.Now != nil {
+		return m.Now()
 	}
-	return found, complete
+	return time.Now()
 }

 // walkBelow is the look itself, breadth first, bounded by WalkDepth and WalkLimit.
@@ -411,11 +450,15 @@ func (m *Machine) Judge(c Config, roster map[string]string, wants map[string][]W
 			no(fmt.Sprintf("The share %s's folder %s%s is inside the backup repository %s, and backups are never exported.", s.Name, real, via, inside))
 			continue
 		}
-		holds, complete := m.repositoryBelow(real)
+		holds, complete, pending := m.repositoryBelow(real)
 		if holds != "" {
 			no(fmt.Sprintf("The share %s's folder %s%s holds a backup repository (%s), and backups are never exported: share the folders beside it instead.", s.Name, real, via, holds))
 			continue
 		}
+		if pending {
+			no(fmt.Sprintf("The share %s's folder %s%s has not been looked through yet for a backup repository (the look runs beside the pass and takes a while on a large folder), so it is not exported yet.", s.Name, real, via))
+			continue
+		}
 		if !complete {
 			no(fmt.Sprintf("The share %s's folder %s%s holds more folders than the module looks through (%d levels, at most %d folders), so it could not make sure no backup repository is under it: share smaller folders.", s.Name, real, via, WalkDepth, m.limit()))
 			continue