fix(daemon): a 503 is an answer, not a wedge — stop restarting a healthy

service under load

GH #1419 Issue 3b (Steve postmortem): the supervisor cycled nexus-service
after 3 consecutive failed health checks caused purely by CPU-bound local
embedding, not by any failure — severing a client mid-batch.

MECHANISM, verified on both sides of the wire. GET /health is implemented
engine-side as dataSource.getConnection() + SELECT 1 (HealthHandler.java), so
it takes a HikariCP pool connection. The supervisor probed it with a 2s
timeout and exited for an OS restart after 3 consecutive failures at 1s — a
3-second grace window. Under indexing load the pool is contended, /health
blocks past 2s, the counter fills, and the service is restarted. The probe
that decides whether to KILL the service competes for the exact resource
saturation exhausts, so the loop is self-amplifying: load -> restart ->
in-flight clients severed -> retries -> more load.

THE DEEPER DEFECT: _service_healthy returned a bare bool, collapsing
"answered 503" and "did not answer at all" into one False. Those are opposite
pieces of evidence. A 503 proves the process is ALIVE and talking — its
dependency is unhappy — and restarting it cannot fix a down database while it
certainly does sever clients. Only silence is consistent with a wedge.

So the probe is now tri-state (HealthProbe OK / UNREADY / UNKNOWN) and ONLY
UNKNOWN advances the restart counter. UNREADY resets it, declines to stamp the
lease (consumers should not be routed to a service saying it cannot serve),
and logs. _service_healthy survives as a boolean for the STARTUP readiness
gate, where UNREADY and UNKNOWN really are equivalent.

BOUNDED, AND HONESTLY SO. Widening the probe budget hits an invariant: the
probe blocks the heartbeat thread, so a tick costs _HEALTH_TIMEOUT +
heartbeat interval and the lease TTL must be >= 3x that
(test_ttl_exceeds_worst_case_heartbeat_tick). With TTL 15s that caps the probe
at exactly 4.0s. A first attempt at 20s tripped that test — the supervisor
would have lost its OWN lease mid-probe, trading a spurious restart for a
vanished endpoint. 4.0s x 4 beats = ~16s of total silence before restart, up
from ~6s, at the invariant boundary with no margin left.

That mitigates the saturation case rather than closing it: a pool whose own
connectionTimeout is 30s can still outlast a 4s probe. Closing it needs a
dependency-free engine liveness endpoint and/or the probe off the heartbeat
thread — filed as nexus-hubc0, engine half batched into v0.1.55. The
503-vs-timeout distinction shipped here is what actually protects the
DB-unhappy case, and it stands on its own.

9 new tests, each RED first, driving a REAL loopback server on port 0 whose
status is switchable rather than stubbing the probe — a classifier that never
classifies would otherwise pass. Two existing heartbeat tests re-pointed at
the tri-state seam; the eight startup-gate stubs keep the boolean by design.

Bead: nexus-7f7gb
Follow-up: nexus-hubc0

Author: Hellblazer

src/nexus/daemon/storage_service_daemon.py

@@ -65,6 +65,7 @@
 import subprocess
 import threading
 import time
+from enum import Enum
 from pathlib import Path
 from typing import Any, Callable, Optional
 
@@ -103,6 +104,25 @@
 _LIBC = _pdeathsig.LIBC
 _set_pdeathsig_preexec = _pdeathsig.set_pdeathsig_preexec
 
+class HealthProbe(Enum):
+    """Outcome of a ``GET /health`` probe (nexus-7f7gb).
+
+    Three states, because two were not enough to make a restart decision:
+
+    - ``OK``      — 200. Serving.
+    - ``UNREADY`` — answered with a non-200 status. The process is ALIVE and
+      talking; its dependency is unhappy. Restarting cannot fix that and does
+      sever in-flight clients, so this never counts toward a restart.
+    - ``UNKNOWN`` — no answer at all (timeout, refused, unset port). The only
+      evidence consistent with a wedged process, and therefore the only state
+      that advances the restart counter.
+    """
+
+    OK = "ok"
+    UNREADY = "unready"
+    UNKNOWN = "unknown"
+
+
 #: Path suffix of the spawn lock file inside config_dir.
 _SPAWN_LOCK_FILE: str = "storage_service_spawn.lock"
 
@@ -115,8 +135,26 @@
 #: After SIGTERM, wait this long before escalating to SIGKILL.
 _GRACEFUL_STOP_TIMEOUT: float = 5.0
 
-#: Short HTTP timeout for /health probes.
-_HEALTH_TIMEOUT: float = 2.0
+#: HTTP timeout for /health probes.
+#:
+#: nexus-7f7gb (GH #1419 Issue 3b): was 2.0s, BELOW the contention window it
+#: had to survive. ``GET /health`` is implemented engine-side as
+#: ``dataSource.getConnection()`` + ``SELECT 1`` (HealthHandler.java), so it
+#: takes a HikariCP pool connection — and the pool's own connectionTimeout is
+#: 30s. Under CPU-bound indexing the pool is contended, /health blocks well
+#: past 2s, and a probe budget that small made "the pool is busy"
+#: indistinguishable from "the process is wedged". The probe that decides
+#: whether to KILL the service must not be tighter than the resource it
+#: contends for.
+#:
+#: CEILING (test_ttl_exceeds_worst_case_heartbeat_tick): the probe BLOCKS the
+#: heartbeat thread, so the tick can take _HEALTH_TIMEOUT + heartbeat interval
+#: and the lease TTL must be >= 3x that. With TTL 15s that caps this at 4.0s.
+#: Raising it further requires either a larger TTL (which widens the
+#: post-restart stale-endpoint window, nexus-om64x) or moving the probe OFF
+#: the heartbeat thread — see the follow-up bead. 4.0s is the largest value
+#: the current invariant allows, not the value the problem wants.
+_HEALTH_TIMEOUT: float = 4.0
 
 #: The storage-service lease TTL is a SUBSTRATE parameter — it lives in the shared
 #: primitive (``service_registry.TIER_TTLS["storage_service"]``, resolved via
@@ -133,11 +171,18 @@
 #: stuck-but-alive states (connection-pool exhaustion, GC pause, internal
 #: deadlock) that are the most common partial-failure mode — and which the OS
 #: watchdog (RDR-175) cannot catch on its own, since it only sees process
-#: death. 3 beats at 1s interval = a 3s grace window before exit — large
-#: enough to absorb transient GC pauses, small enough to recover quickly from
-#: real deadlocks. RDR-175 retired the in-process respawn mechanism; this
+#: death.
+#:
+#: nexus-7f7gb: was 3, which with the old 2s probe gave a ~3s grace window —
+#: an ordinary indexing burst crossed it and cycled the service mid-batch,
+#: severing in-flight clients (which then retried, adding load: the loop was
+#: self-amplifying). Only UNKNOWN beats (no answer AT ALL) advance this
+#: counter now; a 503 means the service ANSWERED and is not a wedge. 4 beats
+#: x ~5s tick = ~20s of total silence before the supervisor exits for
+#: an OS restart — long enough that saturation cannot masquerade as death,
+#: short enough that a real deadlock is still caught. RDR-175 retired the in-process respawn mechanism; this
 #: DETECTION is retained but its action is now exit-for-OS-restart, not respawn.
-_MAX_UNHEALTHY_HEARTBEATS: int = 3
+_MAX_UNHEALTHY_HEARTBEATS: int = 4
 
 
 # ── Errors ─────────────────────────────────────────────────────────────────────
@@ -644,19 +689,43 @@ def _spawn_service(self) -> tuple[subprocess.Popen[bytes], int]:
         )
         return proc, port
 
-    def _service_healthy(self, port: int | None = None) -> bool:
-        """Return True iff the service /health endpoint returns HTTP 200."""
+    def _probe_service_health(self, port: int | None = None) -> "HealthProbe":
+        """Classify ``GET /health`` into :class:`HealthProbe`.
+
+        nexus-7f7gb: the predecessor returned a bare bool, collapsing
+        "answered 503" and "did not answer at all" into one False. Those are
+        OPPOSITE pieces of evidence about liveness — a 503 proves the process
+        is alive and answering, while silence is the only thing suggesting a
+        wedge — and treating them alike is what let an indexing burst kill a
+        healthy service.
+        """
         _port = port if port is not None else self._service_port
         if _port <= 0:
-            return False
+            return HealthProbe.UNKNOWN
         url = f"http://{_SERVICE_HOST}:{_port}/health"
         try:
+            import urllib.error  # noqa: PLC0415 — deferred import — branch-local
             import urllib.request  # noqa: PLC0415 — deferred import — platform/heavy dep loaded only on the path that needs it
             req = urllib.request.Request(url, method="GET")
             with urllib.request.urlopen(req, timeout=_HEALTH_TIMEOUT) as resp:
-                return resp.status == 200
-        except Exception:  # noqa: BLE001 — best-effort reachability probe; returns False on any error
-            return False
+                return HealthProbe.OK if resp.status == 200 else HealthProbe.UNREADY
+        except urllib.error.HTTPError:
+            # An HTTP status IS an answer: the process is up and serving, its
+            # dependency is not. Never a restart reason — restarting cannot
+            # fix a down database and does sever in-flight clients.
+            return HealthProbe.UNREADY
+        except Exception:  # noqa: BLE001 — no answer: timeout, refused, anything
+            return HealthProbe.UNKNOWN
+
+    def _service_healthy(self, port: int | None = None) -> bool:
+        """Back-compat boolean: True iff /health answered 200.
+
+        Retained for the STARTUP readiness gate, which genuinely wants "is it
+        serving yet" and for which UNREADY and UNKNOWN are equivalent. The
+        HEARTBEAT path must use :meth:`_probe_service_health` — there, the
+        distinction is the whole point.
+        """
+        return self._probe_service_health(port) is HealthProbe.OK
 
     def _pg_reachable(self) -> bool:
         """Return True iff the Postgres port accepts TCP."""
@@ -947,13 +1016,30 @@ def heartbeat_once(self) -> tuple[bool, bool]:
             return False, False  # process exited; signal the run loop to exit
 
         service_alive = _pid_is_alive(self._proc.pid)
-        service_ok = self._service_healthy()
+        probe = self._probe_service_health()
+        # nexus-7f7gb: UNREADY (answered non-200) is NOT a wedge — only total
+        # silence counts toward the restart threshold.
+        service_ok = probe is HealthProbe.OK
         pg_ok = self._pg_reachable()
 
         if not service_alive:
             self._consecutive_unhealthy_heartbeats = 0
             return False, pg_ok
 
+        if probe is HealthProbe.UNREADY:
+            # Answered, therefore alive. Reset the wedge counter, and do NOT
+            # stamp the lease — consumers should not be routed to a service
+            # telling us it cannot serve. It simply is not a restart reason.
+            self._consecutive_unhealthy_heartbeats = 0
+            _log.warning(
+                "storage_service_unready",
+                pg_ok=pg_ok,
+                port=self._service_port,
+                msg="/health answered non-200 — alive, dependency unhappy; "
+                    "not counting toward restart",
+            )
+            return True, pg_ok
+
         if not service_ok:
             self._consecutive_unhealthy_heartbeats += 1
             _log.warning(

tests/daemon/test_health_probe_tristate.py

@@ -0,0 +1,219 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""nexus-7f7gb (GH #1419 Issue 3b): a 503 must not restart the service.
+
+Steve Harris watched the supervisor cycle ``nexus-service`` after 3 consecutive
+failed health checks caused purely by CPU-bound local-embedding load, not by
+any failure. A client mid-batch loses its connection when that happens.
+
+MECHANISM (verified on both sides of the wire):
+
+* ``HealthHandler.java`` implements ``GET /health`` as
+  ``dataSource.getConnection()`` + ``SELECT 1`` — it takes a HikariCP pool
+  connection.
+* The supervisor probes it with ``_HEALTH_TIMEOUT`` and exits for an OS
+  restart after ``_MAX_UNHEALTHY_HEARTBEATS`` consecutive failures.
+
+So the probe that decides whether to KILL the service competes for the very
+resource that saturation exhausts. Under indexing load the pool is contended,
+``/health`` blocks past the probe timeout, the counter fills, and the service
+is restarted — which severs in-flight clients, which makes them retry, which
+adds load. Self-amplifying.
+
+THE DEEPER DEFECT the fix addresses: ``_service_healthy`` returned a bare bool,
+collapsing "answered 503" and "did not answer at all" into one False. Those are
+opposite pieces of evidence:
+
+* 503  — the process ANSWERED. It is demonstrably alive; the DB is unhappy.
+         Restarting cannot fix a down database and does sever clients.
+* timeout / refused — the process may genuinely be wedged. This is the only
+         evidence that justifies a restart.
+
+Only UNKNOWN advances the restart counter. Hal decision 2026-07-24. The
+correct long-term fix is a dependency-free engine liveness endpoint, queued
+for the v0.1.55 engine batch.
+"""
+from __future__ import annotations
+
+import http.server
+import threading
+
+import pytest
+
+
+@pytest.fixture
+def health_server():
+    """Real loopback server on port 0 whose /health status is switchable."""
+    state = {"status": 200, "delay": 0.0}
+
+    class _H(http.server.BaseHTTPRequestHandler):
+        def do_GET(self) -> None:  # noqa: N802 — stdlib callback name
+            if state["delay"]:
+                import time
+
+                time.sleep(state["delay"])
+            body = b'{"status":"ok"}'
+            self.send_response(state["status"])
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+
+        def log_message(self, *_a: object) -> None:
+            pass
+
+    httpd = http.server.HTTPServer(("127.0.0.1", 0), _H)
+    threading.Thread(target=httpd.serve_forever, daemon=True).start()
+    yield httpd, state
+    httpd.shutdown()
+    httpd.server_close()
+
+
+def _probe(port: int):
+    from nexus.daemon.storage_service_daemon import StorageServiceSupervisor
+
+    sup = StorageServiceSupervisor.__new__(StorageServiceSupervisor)
+    sup._service_port = port
+    return sup._probe_service_health()
+
+
+class TestProbeClassification:
+    def test_200_is_ok(self, health_server) -> None:
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        httpd, _state = health_server
+        assert _probe(httpd.server_address[1]) is HealthProbe.OK
+
+    def test_503_is_unready_not_unknown(self, health_server) -> None:
+        """THE fix. The service answered — it is alive. Classifying this as
+        UNKNOWN is what let a DB blip and a load spike both kill a healthy
+        process."""
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        httpd, state = health_server
+        state["status"] = 503
+        assert _probe(httpd.server_address[1]) is HealthProbe.UNREADY
+
+    def test_timeout_is_unknown(self, health_server, monkeypatch) -> None:
+        """A probe that never came back is the ONLY evidence of a wedge."""
+        import nexus.daemon.storage_service_daemon as ssd
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        httpd, state = health_server
+        state["delay"] = 1.5
+        monkeypatch.setattr(ssd, "_HEALTH_TIMEOUT", 0.25)
+        assert _probe(httpd.server_address[1]) is HealthProbe.UNKNOWN
+
+    def test_refused_connection_is_unknown(self) -> None:
+        import socket
+
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        s = socket.socket()
+        s.bind(("127.0.0.1", 0))
+        port = s.getsockname()[1]
+        s.close()  # nothing listening now
+        assert _probe(port) is HealthProbe.UNKNOWN
+
+    def test_unset_port_is_unknown(self) -> None:
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        assert _probe(0) is HealthProbe.UNKNOWN
+
+
+class TestRestartAccounting:
+    """The counter must advance ONLY on UNKNOWN."""
+
+    def _sup(self, probe_result):
+        from unittest.mock import MagicMock
+
+        import nexus.daemon.storage_service_daemon as ssd
+
+        sup = ssd.StorageServiceSupervisor.__new__(ssd.StorageServiceSupervisor)
+        sup._consecutive_unhealthy_heartbeats = 0
+        sup._service_port = 1
+        sup._pg_port = 2
+        sup._proc = MagicMock()
+        sup._proc.poll.return_value = None
+        sup._proc.pid = 4242
+        sup._supervisor = MagicMock()
+        sup._supervisor.fenced = False   # healthy path re-stamps and checks this
+        sup._scope = "test-scope"        # only read when fenced, but keep it real
+        sup._probe_service_health = lambda: probe_result
+        sup._pg_reachable = lambda: True
+        return sup
+
+    def test_repeated_503_never_reaches_the_restart_threshold(
+        self, monkeypatch,
+    ) -> None:
+        """Steve's shape. Twenty consecutive 503s must not kill a process that
+        answered twenty times."""
+        import nexus.daemon.storage_service_daemon as ssd
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        monkeypatch.setattr(ssd, "_pid_is_alive", lambda _pid: True)
+        sup = self._sup(HealthProbe.UNREADY)
+
+        for _ in range(20):
+            keep_going, _pg = sup.heartbeat_once()
+            assert keep_going, "a service answering 503 must never be killed"
+        assert sup._consecutive_unhealthy_heartbeats == 0
+
+    def test_sustained_unknown_still_triggers_the_restart(
+        self, monkeypatch,
+    ) -> None:
+        """The stuck-but-alive class the threshold exists for must still be
+        caught — this fix must not disarm wedge detection."""
+        import nexus.daemon.storage_service_daemon as ssd
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        monkeypatch.setattr(ssd, "_pid_is_alive", lambda _pid: True)
+        sup = self._sup(HealthProbe.UNKNOWN)
+
+        outcomes = [
+            sup.heartbeat_once()[0]
+            for _ in range(ssd._MAX_UNHEALTHY_HEARTBEATS)
+        ]
+        assert outcomes[-1] is False, "sustained no-answer must still restart"
+
+    def test_an_ok_beat_resets_the_counter(self, monkeypatch) -> None:
+        import nexus.daemon.storage_service_daemon as ssd
+        from nexus.daemon.storage_service_daemon import HealthProbe
+
+        monkeypatch.setattr(ssd, "_pid_is_alive", lambda _pid: True)
+        sup = self._sup(HealthProbe.UNKNOWN)
+        sup.heartbeat_once()
+        assert sup._consecutive_unhealthy_heartbeats == 1
+
+        sup._probe_service_health = lambda: HealthProbe.OK
+        sup.heartbeat_once()
+        assert sup._consecutive_unhealthy_heartbeats == 0
+
+
+def test_probe_timeout_respects_the_lease_ttl_invariant() -> None:
+    """The probe budget is CAPPED, not free.
+
+    Originally this asserted ``_HEALTH_TIMEOUT >= 10.0`` on the reasoning that
+    the probe must outlast pool contention. That was wrong and the existing
+    invariant test caught it: the probe BLOCKS the heartbeat thread, so a tick
+    costs ``_HEALTH_TIMEOUT + heartbeat_interval`` and the lease TTL must be at
+    least 3x that (test_ttl_exceeds_worst_case_heartbeat_tick). A 20s probe
+    made the supervisor's OWN lease age out mid-probe — trading a spurious
+    restart for a vanished endpoint, which is the same outage wearing a
+    different hat.
+
+    So the honest statement is: 4.0s is the largest value the current design
+    permits, and it is NOT large enough to outlast a saturated HikariCP pool
+    (connectionTimeout 30s). The tri-state fix stands on its own; closing the
+    remaining gap needs the probe off the heartbeat thread or a dependency-free
+    engine liveness endpoint. Both are tracked.
+    """
+    import nexus.daemon.storage_service_daemon as ssd
+    from nexus.daemon.service_registry import DEFAULT_HEARTBEAT_INTERVAL, ttl_for_tier
+
+    worst_tick = ssd._HEALTH_TIMEOUT + DEFAULT_HEARTBEAT_INTERVAL
+    assert ttl_for_tier("storage_service") >= 3 * worst_tick, (
+        "probe timeout raised past what the lease TTL can absorb — the "
+        "supervisor would lose its own lease while probing"
+    )
+    # Still meaningfully wider than the 2s/3-beat window that cycled Steve's
+    # service: 4 beats of total silence at ~5s per tick.
+    assert ssd._HEALTH_TIMEOUT * ssd._MAX_UNHEALTHY_HEARTBEATS >= 16.0