feat: monitors, UniFi and Traefik keep collecting through a storage stall (#5995)

The same Holding as the SNMP poll, for the rest of Steward's own
collectors:

- Monitors: the list is read within 10 s or the last one read stands in;
  a start the database doesn't take is kept in memory, so a monitor isn't
  checked twice; each result is held and stored at the time it was
  checked. A stall shows on the monitors as the checks it saw, not a gap.
- UniFi and Traefik: each poll's writes are held (hold_write) and run
  oldest first in one transaction; every reading is stamped with its scrape.
- A held round the database refuses (as opposed to can't be reached) is
  stored alone and let go, so it can't hold up every round after it.

The Prometheus scrape is left as it is: its one target, Fabled Archives,
keeps its own database on the same NFS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

steward/core/holding.py

@@ -80,13 +80,42 @@ class Holding:
                              "held (the database may still finish it)", self.name, len(batch),
                              self.deadline_s)
                 return
-            except Exception:
-                logger.exception("%s: storing %d held round(s) failed; they stay held",
+            except Exception as exc:
+                if unreachable(exc):
+                    logger.warning("%s: the database can't be reached (%s); %d round(s) stay held",
+                                   self.name, exc, len(batch))
+                    return
+                # A round the database refuses would otherwise be retried, and
+                # refused, ahead of every round after it: store each on its own
+                # and let go of the ones refused.
+                logger.exception("%s: storing %d held round(s) failed; storing each alone",
                                  self.name, len(batch))
-                return
-            last = batch[-1][0]
-            while self._held and self._held[0][0] <= last:
-                self._held.popleft()
+                for seq, item in batch:
+                    try:
+                        await asyncio.wait_for(self.store([item]), self.deadline_s)
+                    except Exception as one:
+                        if isinstance(one, TimeoutError) or unreachable(one):
+                            return
+                        logger.error("%s: a round collected was refused and is dropped: %s",
+                                     self.name, one)
+                    self._drop_through(seq)
+                continue
+            self._drop_through(batch[-1][0])
+
+    def _drop_through(self, seq: int) -> None:
+        while self._held and self._held[0][0] <= seq:
+            self._held.popleft()
+
+
+def unreachable(exc: BaseException) -> bool:
+    """Whether a store failed because the database couldn't be reached, as
+    opposed to refusing what it was given."""
+    from sqlalchemy.exc import DBAPIError, InterfaceError, OperationalError
+
+    from steward.database import DatabaseUnavailable
+    if isinstance(exc, (OSError, DatabaseUnavailable, InterfaceError, OperationalError)):
+        return True
+    return isinstance(exc, DBAPIError) and bool(exc.connection_invalidated)
 
 
 def holding(app, name: str, store: Callable[[list], Awaitable[None]], limit: int) -> Holding:
@@ -97,3 +126,21 @@ def holding(app, name: str, store: Callable[[list], Awaitable[None]], limit: int
     if name not in holdings:
         holdings[name] = Holding(name, store, limit)
     return holdings[name]
+
+
+async def hold_write(app, name: str, write: Callable[[object], Awaitable[None]],
+                     limit: int = 120, wait_s: float = 20.0) -> bool:
+    """Hold one collected round as `write(session)` and store it: the held
+    writes run oldest first, in one transaction. For a collector whose round
+    is easiest said as the writes it makes. True when it was stored within
+    `wait_s`."""
+    held = holding(app, name, store=lambda writes: _run_writes(app, writes), limit=limit)
+    held.hold(write)
+    return await held.flush(wait_s)
+
+
+async def _run_writes(app, writes: list) -> None:
+    async with app.db_sessionmaker() as session:
+        async with session.begin():
+            for write in writes:
+                await write(session)

steward/monitors/scheduler.py

@@ -5,15 +5,24 @@ enabled monitors and their intervals fresh (a changed interval applies at the
 next tick), stamps the due ones with the time they were due to start, and
 starts each check on its own. A slow check (a 10 s timeout) holds up nobody
 else, and a monitor still being checked is not started twice.
+
+A stall of the database doesn't stop the checks (#5995). The monitor list
+is read within MONITORS_READ_S, or the last one read stands in; a stamp the
+database doesn't take in that time is kept in memory instead; and each
+result is held and stored, oldest first and at the time it was checked, once
+the database answers. So a stall shows on the monitors as the checks it saw,
+not as a gap.
 """
 from __future__ import annotations
 import asyncio
 import logging
 from datetime import datetime, timezone
+from types import SimpleNamespace
 
 from sqlalchemy import select, update
 
 from steward.core.alerts import MetricBatch
+from steward.core.holding import holding
 from steward.models.monitors import Monitor, MonitorResult
 from .runner import _BLANK, run_monitor
 
@@ -25,17 +34,30 @@ DISPATCH_TICK_SECONDS = 5
 # A check past this is recorded down rather than left holding its monitor:
 # far beyond any probe's own timeout, so it only ever catches a hang.
 CHECK_DEADLINE_SECONDS = 120
+# How long the monitor list may take to read, or the stamp to be taken,
+# before the last list read and the stamp kept in memory stand in.
+MONITORS_READ_S = 10.0
+# How long a check waits for its result to be stored before it lets its
+# monitor go; the result stays held until the database takes it.
+STORE_WAIT_S = 20.0
+# Results held while the database doesn't answer: an hour of a minute's
+# checks for 60 monitors.
+HELD_RESULTS = 3600
 
 
-def is_due(monitor, now: datetime, global_interval: int) -> bool:
-    """Whether a monitor's interval has elapsed since its last scheduled start.
+def is_due(monitor, now: datetime, global_interval: int,
+           started: datetime | None = None) -> bool:
+    """Whether a monitor's interval has elapsed since its last scheduled start:
+    the stamp stored, or `started`, the one kept in memory when the database
+    didn't take it.
 
     Half a tick of slack, so a check stamped at t isn't missed at t+interval
     by a few milliseconds of tick jitter and pushed back a whole tick."""
-    if monitor.last_checked_at is None:
+    last = max((t for t in (monitor.last_checked_at, started) if t is not None), default=None)
+    if last is None:
         return True
     interval = monitor.check_interval_seconds or global_interval
-    elapsed = (now - monitor.last_checked_at).total_seconds()
+    elapsed = (now - last).total_seconds()
     return elapsed >= interval - DISPATCH_TICK_SECONDS / 2
 
 
@@ -46,6 +68,39 @@ def _state(app) -> tuple[set, set]:
     return app._monitor_checks
 
 
+def _memory(app) -> SimpleNamespace:
+    """The monitors as last read, and the starts the database hasn't taken."""
+    if not hasattr(app, "_monitor_memory"):
+        app._monitor_memory = SimpleNamespace(monitors=None, started={})
+    return app._monitor_memory
+
+
+async def _enabled_monitors(app) -> list:
+    memory = _memory(app)
+
+    async def read() -> list:
+        async with app.db_sessionmaker() as session:
+            return list((await session.execute(
+                select(Monitor).where(Monitor.enabled.is_(True)))).scalars())
+
+    try:
+        memory.monitors = await asyncio.wait_for(read(), MONITORS_READ_S)
+    except Exception as exc:
+        if memory.monitors is None:
+            logger.warning("Monitors not read (%s); nothing to check yet", exc or type(exc).__name__)
+            return []
+        logger.warning("Monitors not read within %.0f s (%s); checking the %d last read",
+                       MONITORS_READ_S, exc or type(exc).__name__, len(memory.monitors))
+    return memory.monitors
+
+
+async def _stamp(app, ids: list[str], now: datetime) -> None:
+    async with app.db_sessionmaker() as session:
+        await session.execute(
+            update(Monitor).where(Monitor.id.in_(ids)).values(last_checked_at=now))
+        await session.commit()
+
+
 async def run_due_monitors(app) -> None:
     """Start a check for every enabled Monitor that is due and not already
     being checked. Returns once they are started, not finished.
@@ -55,19 +110,23 @@ async def run_due_monitors(app) -> None:
     global_interval = int(app.config.get("MONITORS_POLL_INTERVAL", 60))
     now = datetime.now(timezone.utc)
     in_flight, tasks = _state(app)
+    memory = _memory(app)
 
-    async with app.db_sessionmaker() as session:
-        monitors = list((await session.execute(
-            select(Monitor).where(Monitor.enabled.is_(True))
-        )).scalars())
-        due = [m for m in monitors if m.id not in in_flight and is_due(m, now, global_interval)]
-        if not due:
-            return
-        # Stamp the scheduled start before checking: the next run is due one
-        # interval from now, however long this check takes.
-        await session.execute(
-            update(Monitor).where(Monitor.id.in_([m.id for m in due])).values(last_checked_at=now))
-        await session.commit()
+    monitors = await _enabled_monitors(app)
+    due = [m for m in monitors if m.id not in in_flight
+           and is_due(m, now, global_interval, memory.started.get(m.id))]
+    if not due:
+        return
+    # Stamp the scheduled start before checking: the next run is due one
+    # interval from now, however long this check takes. Kept in memory too,
+    # for when the database doesn't take it.
+    for m in due:
+        memory.started[m.id] = now
+    try:
+        await asyncio.wait_for(_stamp(app, [m.id for m in due], now), MONITORS_READ_S)
+    except Exception as exc:
+        logger.warning("Monitor starts not stored within %.0f s (%s); kept in memory",
+                       MONITORS_READ_S, exc or type(exc).__name__)
 
     for m in due:
         in_flight.add(m.id)
@@ -77,18 +136,31 @@ async def run_due_monitors(app) -> None:
 
 
 async def _check_and_record(app, monitor: Monitor) -> None:
-    """Run one monitor's check and store its result and metrics."""
+    """Run one monitor's check, hold its result, and store it."""
     in_flight, _ = _state(app)
     try:
         try:
             res = await asyncio.wait_for(run_monitor(monitor), CHECK_DEADLINE_SECONDS)
         except asyncio.TimeoutError:
             res = dict(_BLANK, error_msg=f"Check did not finish within {CHECK_DEADLINE_SECONDS} s")
-        check_time = datetime.now(timezone.utc)
-        async with app.db_sessionmaker() as session:
-            async with session.begin():
+        held = holding(app, "Monitor checks", store=lambda rounds: _store(app, rounds),
+                       limit=HELD_RESULTS)
+        held.hold((monitor.id, monitor.type, monitor.name, datetime.now(timezone.utc), res))
+        await held.flush(STORE_WAIT_S)
+    except Exception:
+        logger.exception("Monitor %r check errored", monitor.name)
+    finally:
+        in_flight.discard(monitor.id)
+
+
+async def _store(app, checks: list) -> None:
+    """Write held check results, oldest first, each at the time it was checked."""
+    async with app.db_sessionmaker() as session:
+        async with session.begin():
+            batch = MetricBatch()
+            for monitor_id, mtype, name, check_time, res in checks:
                 session.add(MonitorResult(
-                    monitor_id=monitor.id,
+                    monitor_id=monitor_id,
                     checked_at=check_time,
                     is_up=res["is_up"],
                     response_ms=res["response_ms"],
@@ -99,20 +171,9 @@ async def _check_and_record(app, monitor: Monitor) -> None:
                     error_msg=res["error_msg"],
                 ))
                 # Alert pipeline — keyed by monitor type (source) + name.
-                batch = MetricBatch()
-                batch.add(
-                    source_module=monitor.type,
-                    resource_name=monitor.name, metric_name="is_up",
-                    value=1.0 if res["is_up"] else 0.0,
-                )
+                batch.add(source_module=mtype, resource_name=name, metric_name="is_up",
+                          value=1.0 if res["is_up"] else 0.0, recorded_at=check_time)
                 if res["response_ms"] is not None:
-                    batch.add(
-                        source_module=monitor.type,
-                        resource_name=monitor.name, metric_name="response_ms",
-                        value=res["response_ms"],
-                    )
-                await batch.record(session)
-    except Exception:
-        logger.exception("Monitor %r check errored", monitor.name)
-    finally:
-        in_flight.discard(monitor.id)
+                    batch.add(source_module=mtype, resource_name=name, metric_name="response_ms",
+                              value=res["response_ms"], recorded_at=check_time)
+            await batch.record(session)