Skip to content

Add systemd watchdog to the main service - #31

Merged
vpetersson merged 2 commits into
masterfrom
feat/service-watchdog
Apr 23, 2026
Merged

Add systemd watchdog to the main service#31
vpetersson merged 2 commits into
masterfrom
feat/service-watchdog

Conversation

@vpetersson

@vpetersson vpetersson commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Restart=on-failure already handles process crashes, but recent outages have included cases where the main process stays up while the event loop is wedged — nothing for systemd to restart. This PR wires the app into systemd's native watchdog protocol so a stuck event loop automatically kills + restarts the service.

How it works

  • pypi-tea.service becomes Type=notify with WatchdogSec=30s.
  • On lifespan startup (after httpx / Redis / extraction pool are ready), the app sends READY=1 to $NOTIFY_SOCKET — systemd only considers the service up once this arrives.
  • A background asyncio task pings WATCHDOG=1 every ~15s (half of WATCHDOG_USEC). If the event loop blocks — GIL-held sync call, deadlock, OOM thrashing — the ping stops and systemd terminates the process after 30s; Restart=on-failure brings it back.
  • All guarded on NOTIFY_SOCKET being set, so local dev, tests, and non-systemd deployments are completely unaffected.

Why Type=notify (option B) over an external curl-based watchdog (option A)

  • Detects event-loop hangs directly, not just endpoint unreachability.
  • No extra unit/timer to manage.
  • Uses the standard protocol every systemd-aware operator already understands (systemctl status shows watchdog state).

Additional hardening

  • StartLimitIntervalSec=600 + StartLimitBurst=10 cap flapping: if the service restarts ≥10 times in 10 min, systemd stops retrying and marks it failed (visible via systemctl status, paging alerts, etc.). Without this, a genuinely broken service could silently restart forever.
  • NotifyAccess=main restricts who can send to the notify socket.

Files

  • src/pypi_tea/app.py — READY / WATCHDOG wiring in the lifespan context manager; clean shutdown of the watchdog task.
  • deploy/pypi-tea.serviceType=notify, WatchdogSec=30s, NotifyAccess=main, StartLimit tuning.
  • pyproject.toml — adds sdnotify>=0.3.2 (pure Python, no build deps) and the mypy override; bumps to 0.6.1 (stacked on top of Refactor refresh into arq-backed worker queue #30's 0.6.0).

Non-goals

  • This does NOT add a deeper healthcheck (e.g. also pinging Redis before each WATCHDOG=1). The ping proves the event loop is alive, which covers the dominant hang mode we've seen. A deeper probe can be layered later if needed.

Test plan

  • uv run ruff check src/ tests/ — clean
  • uv run ruff format src/ tests/ — no changes
  • uv run mypy src/ — clean (strict)
  • uv run pytest tests/ -v — 79 passed, 5 skipped (unchanged)
  • App still starts correctly when NOTIFY_SOCKET is unset (covered by existing test suite)
  • Deploy to a staging host and verify:
    • systemctl status pypi-tea shows "active (running)" and "Status: Ready"
    • Forcing a hang (e.g. kill -STOP $(pidof uvicorn)) triggers a restart within ~30s
    • journalctl -u pypi-tea logs the "systemd watchdog active" line on startup

Compatibility with #30

This stacks on #30 — the PR base is refactor/arq-worker-queue, not master. Diff shown here is the watchdog delta on top of the arq refactor. GitHub will auto-retarget to master once #30 merges.

🤖 Generated with Claude Code

@vpetersson
vpetersson force-pushed the feat/service-watchdog branch from e61abe1 to 5e58bc1 Compare April 23, 2026 14:33
@vpetersson
vpetersson changed the base branch from master to refactor/arq-worker-queue April 23, 2026 14:33
@vpetersson
vpetersson requested a review from Copilot April 23, 2026 14:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR integrates the FastAPI app with systemd’s native sd_notify watchdog protocol so systemd can restart the service when the event loop becomes unresponsive, improving resilience beyond Restart=on-failure.

Changes:

  • Add READY=1 notification on successful lifespan startup and a background watchdog ping loop (WATCHDOG=1) guarded by NOTIFY_SOCKET.
  • Update the main systemd unit to Type=notify with WatchdogSec=30s and restart flapping limits.
  • Add sdnotify dependency (and mypy ignore override) and bump project version.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

File Description
src/pypi_tea/app.py Sends systemd readiness notification and runs an asyncio watchdog ping task during app lifespan.
deploy/pypi-tea.service Switches to Type=notify, enables watchdog, and adds start-limit tuning.
pyproject.toml Adds sdnotify dependency, mypy override, and bumps version to 0.6.1.
uv.lock Locks the new sdnotify dependency and updates the package version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/pypi_tea/app.py Outdated
Comment thread src/pypi_tea/app.py Outdated
Comment thread src/pypi_tea/app.py Outdated
Comment thread deploy/pypi-tea.service Outdated
Comment thread pyproject.toml
Restart=on-failure already handles crashes, but we've had outages where
the process stays up while the event loop is wedged — nothing for
systemd to restart.  Use Type=notify + WatchdogSec so a stuck loop
stops the WATCHDOG=1 pings and systemd kills + restarts us.

- app.py: send READY=1 after lifespan startup completes and spawn a
  background task that pings WATCHDOG=1 every half of WATCHDOG_USEC.
  Guarded on NOTIFY_SOCKET so local dev and tests are unaffected.
- pypi-tea.service: Type=notify, WatchdogSec=30s, NotifyAccess=main,
  plus StartLimitIntervalSec/Burst so a genuinely broken service does
  eventually give up instead of hot-looping forever.
- Add sdnotify dep and the corresponding mypy stub override.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vpetersson
vpetersson force-pushed the feat/service-watchdog branch from 5e58bc1 to 5110ebd Compare April 23, 2026 16:17
- Extract _maybe_start_watchdog() helper.  Parses WATCHDOG_USEC defensively
  (bad value logs a warning and disables the watchdog instead of crashing
  startup) and honours WATCHDOG_PID: if systemd set it and the value does
  not match os.getpid(), pings would be ignored anyway so we skip starting
  the task and log a warning.
- Reword pypi-tea.service comment from "every ~15s" to "at roughly half
  the configured WatchdogSec interval" so it stays accurate if WatchdogSec
  is overridden via drop-in.
- New tests/test_watchdog.py covers: READY=1 + periodic WATCHDOG=1 when
  env is set, quiet path when NOTIFY_SOCKET is absent, and each
  disable-watchdog reason (invalid WATCHDOG_USEC, WATCHDOG_PID mismatch,
  WATCHDOG_USEC=0).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vpetersson
vpetersson changed the base branch from refactor/arq-worker-queue to master April 23, 2026 21:22
@vpetersson
vpetersson merged commit 49cfd34 into master Apr 23, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants