Skip to content

PeerStorage.health() only checks that the watcher reference is non-null — a watcher whose iterator dies silently is never detected, so restartWorthy never fires #67

Description

@konsone

Body

Summary

PeerStorage.health() reports liveness as:

override health(): PeerHealth {
    const ok = !!(this.watcherDeno || this.watcher);
    return { name: this.config.name, type: "storage", ok, detail: ok ? "watching" : "starting", backendUp: true, restartWorthy: false };
}

This only checks that the watcher object reference is non-null — not that the watcher is actually still delivering events. In startDenoFsWatch():

this.watcherDeno = Deno.watchFs(lP, { recursive: true });
for await (const event of this.watcherDeno) {
    this.processFile(event);
}

if the for await loop ends — the native watcher's async iterator returns, throws, or the process otherwise stops iterating (e.g. an OS-level fs-event hiccup, or the Too many open files crash path described in #14) — this.watcherDeno is not reset to undefined. The stale reference keeps health().ok at true indefinitely.

Since Peer.probeHealth() derives restartWorthy from base.ok:

const restartWorthy = backendUp && (Date.now() - this._notOkSince > Peer.RESTART_GRACE_MS);

a storage peer that silently stopped watching never reports ok: false, so restartWorthy never becomes true and the container-restart safety net never engages. The bridge keeps running, keeps reporting "watching" in the health file, and simply stops dispatching any local file change until someone manually restarts the container — with no signal that anything is wrong.

The chokidar path (useChokidar: true) has the identical defect, since ok there is also just !!(this.watcher).

Reproduction

  1. Start the bridge with a storage peer (useChokidar: false).
  2. Cause the native watcher's iterator to end without an explicit .close() call (e.g. hit the fd-limit crash in Too many open files (os error 24) #14, or interrupt/kill the process feeding fs events in a way that ends the async iterator but doesn't throw past startDenoFsWatch()'s caller).
  3. Modify a file under the watched path. It is never dispatched.
  4. Check health() / the heartbeat file — it still reports ok: true, "watching".
  5. Restarting the container recovers the file via the startup scanOfflineChanges scan, confirming the watcher (not something downstream) was the dead component.

Related, but distinct from

Suggested direction

Track watcher liveness explicitly instead of !!ref — e.g. reset this.watcherDeno/this.watcher to undefined (or flip a dedicated alive flag) as soon as the watch loop/iterator ends for any reason, so health() reflects reality and restartWorthy can actually trip. Optionally pair this with a periodic reconciliation pass (diff the watched tree against a persisted inventory) so a missed event is caught within a bounded interval even before a restart happens, rather than only on next container start.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions