diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d7e8744..64c5e90 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -74,7 +74,7 @@ graph TB - **Session Management**: User session data - **Cache Layer**: Application-level caching - **Queue Backend**: Background job processing -- **Scheduler Heartbeat**: `queue:heartbeat` key written every minute to verify the scheduler container is alive +- **Queue Worker Heartbeat**: `queue:heartbeat` key, written by a job dispatched onto the `webhooks` queue every minute, so it's only updated when an actual queue worker (not just the scheduler) is alive and processing jobs ### 3. Background Processing @@ -83,13 +83,14 @@ graph TB // Job Types - SendWebhook: Handle webhook HTTP POST with HMAC signing, timing, and retry logic - ProcessWebhookRetries (command): Pick up failed deliveries ready for retry -- QueueHeartbeat (command): Write alive timestamp to Redis every minute +- QueueHeartbeat (command): Dispatch a WriteQueueHeartbeat job onto the `webhooks` queue every minute +- WriteQueueHeartbeat (job): Write alive timestamp to Redis when a queue worker processes it ``` #### Scheduler - **Laravel Scheduler**: Cron-like job scheduling via `routes/console.php` - **Retry Logic**: `webhooks:process-retries` runs every minute — exponential backoff delays configured via `WEBHOOK_BACKOFF_DELAYS` -- **Health Heartbeat**: `queue:heartbeat` runs every minute; the `/health` endpoint reports `stale` if no heartbeat within 2 minutes +- **Health Heartbeat**: `queue:heartbeat` runs every minute, dispatching a job onto the `webhooks` queue; `/health/detailed` reports `stale` if a queue worker hasn't processed one within 2 minutes ## 📊 Data Models & Relationships @@ -310,7 +311,7 @@ GET /health // HTTP 200 if healthy, 503 if any service is degraded ``` -`queue_worker` is `stale` (and the response is 503) if the scheduler heartbeat in Redis is older than 2 minutes, indicating the scheduler container is down. +`queue_worker` is `stale` (and the response is 503) if the queue worker heartbeat in Redis is older than 2 minutes, indicating a queue worker isn't processing jobs — whether because the worker process died or the scheduler stopped dispatching the heartbeat job. ### Error Handling - **Graceful Degradation**: Fallback mechanisms for service failures diff --git a/CLAUDE.md b/CLAUDE.md index f919020..cf22557 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,7 +66,7 @@ The scheduler (`routes/console.php`) runs `webhooks:process-retries` every minut ### Health check -`GET /health` is unauthenticated (for load balancer/orchestrator probes) and returns only `{"status", "timestamp"}` with a 200/503 code — no service breakdown, to avoid disclosing internal infrastructure details to anonymous callers. `GET /health/detailed` requires `auth:sanctum` and returns the full breakdown: `database`, `redis`, and `queue_worker` status, plus loaded PHP extensions. `queue_worker` is `stale` (503) when the Redis key `queue:heartbeat` is older than 2 minutes. The `QueueHeartbeat` artisan command writes this key every minute via the scheduler. +`GET /health` is unauthenticated (for load balancer/orchestrator probes) and returns only `{"status", "timestamp"}` with a 200/503 code — no service breakdown, to avoid disclosing internal infrastructure details to anonymous callers. `GET /health/detailed` requires `auth:sanctum` and returns the full breakdown: `database`, `redis`, and `queue_worker` status, plus loaded PHP extensions. `queue_worker` is `stale` (503) when the Redis key `queue:heartbeat` is older than 2 minutes. The scheduler runs the `QueueHeartbeat` artisan command every minute, which dispatches a `WriteQueueHeartbeat` job onto the `webhooks` queue; that job (not the command itself) writes the key, so staleness reflects whether an actual queue worker is alive and processing jobs, not just whether the scheduler container is ticking. ### Key models diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 39f00d8..973d893 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -398,7 +398,7 @@ Authorization: Bearer } ``` -`queue_worker` is `unknown` if the application has never been fully started (no heartbeat key in Redis yet). The heartbeat is written by `php artisan queue:heartbeat`, which the scheduler container runs every minute. +`queue_worker` is `unknown` if the application has never been fully started (no heartbeat key in Redis yet). The scheduler container runs `php artisan queue:heartbeat` every minute, which dispatches a job onto the `webhooks` queue; the heartbeat key is only written once an actual queue worker processes that job, so staleness reflects worker liveness, not just the scheduler being up. ### Docker Health Check Add to Dockerfile: diff --git a/WEBHOOK_README.md b/WEBHOOK_README.md index f0bf2c7..546c9c4 100644 --- a/WEBHOOK_README.md +++ b/WEBHOOK_README.md @@ -366,7 +366,7 @@ sudo supervisorctl start webhook-worker:* ## Commands - `php artisan webhooks:process-retries` — Process failed webhook deliveries that are ready for retry -- `php artisan queue:heartbeat` — Write a heartbeat timestamp to Redis (run by the scheduler every minute; used by the `/health` endpoint to verify the scheduler is alive) +- `php artisan queue:heartbeat` — Dispatch a job onto the `webhooks` queue that writes a heartbeat timestamp to Redis when a queue worker processes it (run by the scheduler every minute; used by the `/health/detailed` endpoint to verify a queue worker, not just the scheduler, is alive) ## Development diff --git a/app/Console/Commands/QueueHeartbeat.php b/app/Console/Commands/QueueHeartbeat.php index 9474e3e..4db8bed 100644 --- a/app/Console/Commands/QueueHeartbeat.php +++ b/app/Console/Commands/QueueHeartbeat.php @@ -2,17 +2,21 @@ namespace App\Console\Commands; +use App\Jobs\WriteQueueHeartbeat; use Illuminate\Console\Command; -use Illuminate\Support\Facades\Redis; class QueueHeartbeat extends Command { protected $signature = 'queue:heartbeat'; - protected $description = 'Write a heartbeat timestamp to Redis so the health check can verify the scheduler is running'; + protected $description = 'Dispatch a job onto the webhooks queue so the health check can verify a live queue worker (not just the scheduler) is running'; public function handle(): void { - Redis::set('queue:heartbeat', now()->timestamp); + // Dispatched rather than written directly: the heartbeat key must + // only update once an actual queue worker processes this job, so + // staleness reflects worker liveness instead of just the scheduler + // (which runs this command) being alive. See WriteQueueHeartbeat. + WriteQueueHeartbeat::dispatch(); } } diff --git a/app/Jobs/WriteQueueHeartbeat.php b/app/Jobs/WriteQueueHeartbeat.php new file mode 100644 index 0000000..4b49abf --- /dev/null +++ b/app/Jobs/WriteQueueHeartbeat.php @@ -0,0 +1,36 @@ +onQueue('webhooks'); + } + + public function handle(): void + { + Redis::set('queue:heartbeat', now()->timestamp); + } +} diff --git a/routes/console.php b/routes/console.php index e4b01c6..63f8696 100644 --- a/routes/console.php +++ b/routes/console.php @@ -15,5 +15,8 @@ // double-dispatch the same Delivery to the customer's endpoint. Schedule::command('webhooks:process-retries')->everyMinute()->withoutOverlapping(); -// Write a heartbeat so the /health endpoint can verify the scheduler is alive +// Dispatch a heartbeat job onto the webhooks queue every minute. The +// heartbeat key is only written once a live queue worker actually +// processes the job, so /health/detailed can tell a dead worker apart +// from a dead scheduler instead of conflating the two (see #108). Schedule::command('queue:heartbeat')->everyMinute(); diff --git a/tests/Feature/QueueHeartbeatTest.php b/tests/Feature/QueueHeartbeatTest.php new file mode 100644 index 0000000..b1e9f82 --- /dev/null +++ b/tests/Feature/QueueHeartbeatTest.php @@ -0,0 +1,67 @@ + $job->queue === 'webhooks' + ); + } + + public function test_queue_heartbeat_command_does_not_write_the_heartbeat_key_itself(): void + { + // The command only enqueues the job; if it wrote the Redis key + // directly, the heartbeat would stay "fresh" even while the actual + // queue worker process is dead, reproducing the original bug. + Queue::fake(); + Redis::shouldReceive('set')->never(); + + Artisan::call('queue:heartbeat'); + } + + public function test_write_queue_heartbeat_job_writes_the_current_timestamp_to_redis(): void + { + $now = now(); + $this->travelTo($now); + + Redis::shouldReceive('set') + ->once() + ->with('queue:heartbeat', $now->timestamp); + + (new WriteQueueHeartbeat())->handle(); + } + + public function test_write_queue_heartbeat_job_runs_on_the_webhooks_queue(): void + { + // SendWebhook also runs on the `webhooks` queue, so a worker that + // has stopped processing deliveries also stops processing this job + // — which is exactly what makes the heartbeat a real liveness + // signal for the worker that matters. + $job = new WriteQueueHeartbeat(); + + $this->assertSame('webhooks', $job->queue); + } +}