Coordinate abandoned effect recovery - #51
Conversation
Return stable emit handles and retire abandoned effects atomically with durable recovery callbacks. Preserve owner heartbeat grace across cleanup and fence late completion using instance-before-effect locks.
|
@greptileai Please review the current head for recovery/claim races, transaction boundaries, callback idempotency, adapter parity, and public type safety. |
Greptile SummaryThis PR adds coordinated recovery for abandoned SQL effects and prepares release 0.15.0.
Confidence Score: 5/5The PR appears safe to merge; the latest changes only relax two integration-test time budgets without changing production behavior or assertions. No new actionable defect or repository-rule violation was found. The earlier wake-up, scan-bounding, conditional-structure, and test-annotation findings are resolved at the current head; the Important Files Changed
Reviews (7): Last reviewed commit: "test: Allow time for cold MySQL schema s..." | Re-trigger Greptile |
|
@greptileai Please review current head 8326f27. Addressed all four findings: bounded database-time freshness filtering, post-commit actor wakeups, flat timeout guards, and concrete race-test error types. Added deterministic PostgreSQL claim/check races, rollback and lookup-failure tests, and a crash-after-retirement proof. PostgreSQL: 44 passing; default suite: 399 passing; type and lint checks clean. |
|
@greptileai Please review current head 38f0216. The remaining database-wrapper annotation now derives from Parameters<DatabaseConnection["get"]>[1]. Type/lint checks and all 29 PostgreSQL recovery tests pass. All prior review findings are addressed. |
|
@greptileai Please review current head 65e21c0, which prepares 0.15.0. The last finding concerns an unchanged EffectOptions.arguments type from base 317e636; the inline reply links the original source and explains why narrowing this validated input would be an unapproved additional API break. Please assess that compatibility explanation. All introduced annotation findings are fixed. Package build and installed-package smoke checks pass. |
|
@greptileai Please review current head c677b4b. The remaining nested adapter selection now uses early returns in createTestDatabase. All introduced review findings are addressed; type/lint checks and all 44 PostgreSQL tests pass. The unchanged EffectOptions input contract remains preserved as discussed in the prior thread. |
|
@greptileai Please review current head 81693fd. Consumer integration found that one transient database error permanently stopped heartbeat maintenance. The loop now reports the failure and retries at the next interval without consuming effect attempts. Red/green/reversal regression, 400 unit tests, 45 PostgreSQL tests, strict packed consumer checks, and a real PostgreSQL stop/start while HTTP effects remain active all pass. |
|
@greptileai Please review current head 9de7c2c. The only changes since the 5/5 review are 30-second test budgets for the heartbeat regression with cold schema setup and the existing MySQL transmit integration. Both MySQL versions repeatedly exceeded the prior five-second default in the transmit case; recovery tests passed on rerun. The deterministic concurrency barrier deadlines and all production code are unchanged. Type checks and 45 PostgreSQL tests pass. |
An actor can now retain the exact effect returned by
emitand opt into library-coordinated recovery. A stale processing owner is rechecked under the claim-locking protocol; retirement and the originating actor's recovery callback commit together. Fresh owners, pending retries, completed results, and dead effects retain distinct outcomes.Closes #50. Ruby counterpart: cardmagic/solid-objects-ruby#68. Prepares version 0.15.0 with matching package/runtime metadata and a dated changelog.
Before, a watchdog could create overlapping external work:
After,
onRecoveryenables automatic recovery through the existing effect workers. No actor watchdog or explicit recovery request is required:The library checks owner heartbeats during effect polling and stale-process cleanup. The timeout measures heartbeat freshness, not elapsed effect duration. Only
onRecoveryowns replacement.An application that also wants explicit status observations can add
onStatus: "inspectExport"to the sameemitcall, then optionally stage a check from an actor operation:onStatusreports that requested check; it does not create a status subscription. Completed-status repair shares the guarded helper used by success, so duplicate notifications cannot apply the result twice. Full optional watchdog code and the transaction proposal are indocs/api.mdanddocs/effect-recovery.md.The lock order is origin instance → effects → recovery bindings → current owner processes, with deterministic ordering within each group and database time sampled after locks. Actor-staged checks reuse the fenced transaction. Automatic recovery uses one transaction per candidate. A durable retirement binding plus the existing terminal completed storage state prevents reclamation and fences late completion/failure; callback insertion failure rolls back retirement. Bindings survive effect/message pruning until their originating instance is removed.
Compatibility:
emitintentionally changes fromvoidtoEffectHandle; overrides and wrappers must return the handle. Install additive schema migration 9 and upgrade all effect/cleanup workers before opting in. Cloudflare preserves ordinary handle identity and rejects process-heartbeat recovery before commit. Retirement is not cancellation or exactly-once external execution; external actions still need idempotency.Validation:
pnpm run check,pnpm test: 400 tests passed; adapter-specific skips are exercised separately.Real PostgreSQL 17:
pnpm run test:postgresql, 45 passed, including both claim/check race orderings, changed claimants, refreshed heartbeats, simultaneous recovery, callback rollback, and lookup uncertainty.pnpm run test:cloudflare: 48 passed;pnpm run buildandpnpm run test:packagepassed.Test-first handle regression observed
nullinstead of the persisted ID; retirement regression observed a second claim instead of a callback. Removing the instance-first completion fence reproduceddeadlock detected; restored code passes.CI also runs the recovery suite on PostgreSQL 14/18 and MySQL.
pnpm run test:recoverykills a worker after retirement commits and verifies a restarted worker delivers one durable recovery callback.Automatic scans prefilter freshness using database time and respect
claimScanLimit; a barrier regression verifies fresh candidates do not block unrelated pending claims. Committed retirement wakes actor workers.A fresh strict TypeScript consumer installed the packed npm artifact and passed 14 end-to-end HTTP scenarios with separate worker processes. Coverage includes automatic recovery without an actor watchdog, SIGKILL, SIGSTOP/resume with late success/failure, explicit checks, retry/failure compatibility, pruning, foreign handles, and timeout rollback. The equivalent Ruby integration exposed and fixed missing in-handler heartbeats; JS already maintains them.
Transient heartbeat query failures now emit
solid_objects.process.heartbeat_failedand retry on the next configured interval without consuming effect attempts. A failing-query regression passes after the fix and fails when it is removed. Both installed consumers also pass a real PostgreSQL stop/start while their HTTP effects remain held: the original owners resume heartbeats, explicit checks defer, and no duplicate execution occurs.Cold-schema heartbeat and MySQL transmit integration cases use a 30-second test budget after repeated CI failures at the five-second default. Deterministic race barriers retain their shorter deadlines.