Check engine tables on the configured storage connection - #546
Conversation
TaskWatchdog::tablesReady(), WorkerCompatibilityFleet::heartbeatTableExists() and the v1 Watchdog gate themselves on Schema::hasTable(), which inspects the application's default connection. The models they check honour workflows.storage.connection, so with a dedicated storage connection every gate was false: repair passes returned the empty report, expired leases were never repaired, activity timeouts were never enforced and heartbeats were never written. Ask each model's own connection instead. Behaviour on the default connection is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
rmcdaniel
left a comment
There was a problem hiding this comment.
Confirmed the reported root cause: the guards queried the default schema while their models use workflows.storage.connection. The fix uses each model connection consistently, including the custom v1 model path. I ran the new secondary-connection test in a network-isolated PHP 8.4 container with no secrets: 2 tests, 13 assertions passed. Public PR quality, MySQL smoke, contracts and boundary checks passed. Merge this user-reported repair and let the full main-branch matrix qualify the artifact before release.
|
Merged as da7c3fd. The focused secondary-storage test passed in an isolated PHP 8.4 container (2 tests, 13 assertions), and PR checks were green. I am watching the full main-branch database and Laravel upgrade matrix before publishing the patch release. Thanks for the concrete reproduction and narrowly scoped fix. |
|
Release follow-through: this fix is available in the verified Workflow 2.2.7 release and the Server 2.4.6 image, which bundles it. The sample app's embedded lockfile has also been updated in durable-workflow/sample-app#109. Thanks again for the focused report and patch. |
Summary
TaskWatchdog::tablesReady(),WorkerCompatibilityFleet::heartbeatTableExists()and the v1Watchdog::storedWorkflowTableExists()guard their work withSchema::hasTable(...). TheSchemafacade inspects the application's default connection, but every model those guards check resolves its connection throughResolvesStorageConnection, i.e.workflows.storage.connection.With a dedicated storage connection (the documented setup for keeping engine tables out of the application database) the tables never exist on the default connection, so every guard is false and:
TaskWatchdog::runPass()returns the empty report on every Looping wake and everyworkflow:v2:repair-pass; expired leases are never repaired andActivityTimeoutEnforcernever runs,WorkerCompatibilityFleet::heartbeat()never writes a heartbeat and the fleet snapshot is always empty,Nothing is logged: the guards swallow the negative and return quietly. We found it in an embedded Laravel app with
WORKFLOW_STORAGE_CONNECTION=workflows: a worker killed mid-activity left its taskleasedand the runwaitingindefinitely;workflow:v2:repair-passreported zero candidates.This change asks each model's own connection:
Schema::connection($model->getConnectionName())->hasTable($model->getTable()). When no storage connection is configuredgetConnectionName()isnull, whichSchema::connection()resolves to the default connection, so the default-connection behaviour is unchanged.V2UpgradeStatusCommand::hasTable()already used this form.Verification
tests/Unit/V2/TaskWatchdogStorageConnectionTest: engine tables only on a secondary sqlite connection.TaskWatchdog::runPass()selects and re-dispatches an overdue task, sets the loop-throttle key and records a heartbeat on the storage connection;WorkerCompatibilityFleet::heartbeat()writes a row. Without the fix both tests fail (0candidates / heartbeats).vendor/bin/phpunit tests/Unit/V2/TaskWatchdogStorageConnectionTest.php→ OK (2 tests, 13 assertions)Related suites (
V2RepairPassCommandTest,WatchdogTest,HealthCheckTest,WorkflowServiceProviderSchemaTest,StorageConnectionTest,CommandSequenceStorageConnectionTest,V2OnlyStorageConnectionMigrationTest) → OK (82 tests, 487 assertions)vendor/bin/ecs check→ no errors;vendor/bin/phpstan analyse src tests→ no errorsNot run locally: the full
composer unitrun in my container lacksext-pcntlandext-redis, so 18 unrelated tests error identically with and without this change; the MySQL/PostgreSQL feature matrix is left to CI. No replay or payload-codec fixtures are touched, so the regression-corpus contract does not apply.Relevant automated checks pass locally or are covered by CI.
Behavior changes include focused regression coverage when practical.
User-facing or contract changes update the relevant documentation when needed. — none: the fix makes the documented
storage.connectionbehaviour hold; CHANGELOG entry added under Unreleased.Replay or payload-codec changes follow the regression-corpus contract when applicable. — not applicable.
Compatibility
No API or schema change. Deployments with a dedicated storage connection will start repairing expired leases, enforcing activity timeouts and recording worker heartbeats on the next Looping wake after upgrading; those are the engine's existing, documented behaviours, previously skipped silently. Deployments on the default connection see no change.