feat(backend): implement graceful shutdown with drain, flush, and connection teardown (#349) - #461
Open
prissca wants to merge 1 commit into
Open
Conversation
…nection teardown (Epta-Node#349) setupGracefulShutdown() already existed and was unit-tested, but src/index.ts's actual production entry point (main()) never called it — it had its own, much simpler inline SIGTERM/SIGINT handler that only closed the HTTP server, with no draining, no DB closes, no task/agent state cleanup. This PR wires the real function in and fixes the gaps that made it unsafe to rely on: - api/app.ts's close() now awaits jobWorker.stop(jobWorkerStopTimeoutMs) (new AppOptions field, default 10s) before closing the HTTP/WS server — previously it called jobWorker.stop() without awaiting it, so "drain" did nothing: the server closed immediately regardless of in-flight work. - Removed the Phase 3 taskDb.failRunningTasks() call from setupGracefulShutdown, which force-marked every running task's DAG nodes as failed on shutdown. That directly worked against the job queue's own resumption path: JobWorker.start() already calls recoverIncompleteJobs(), which resets any job still "active" from a previous run back to "pending" for retry — a job stuck mid-drain is meant to resume, not be declared dead. (failRunningTasks() itself is left in db/tasks.ts in case it's useful elsewhere; it's just no longer called from the graceful-shutdown path.) - Added Phase 4: eventBus.store.close() — the event store was never closed on shutdown even though every other DB was. - src/index.ts's main() now calls setupGracefulShutdown(httpServer, close, config, { cleanupService, reconciliationService, globalAgentRegistry }) instead of its own inline handler; setupGracefulShutdown's signature gained an optional 4th `extras` argument (backward compatible — the existing 3-argument test call still passes) to stop those three services, which main() previously stopped inline but setupGracefulShutdown had no way to reach before. - New test in src/queue/worker.test.ts: a job still "active" when a JobWorker.stop() call times out is picked up and completed by a *fresh* JobWorker instance over the same store — the restart-mid-stream scenario the acceptance criteria asks for, exercised at the layer that actually owns resumption (recoverIncompleteJobs()). - tests/shutdown.test.ts updated: asserts failRunningTasks/createTaskDb are no longer called, asserts the three extra services are stopped, asserts the event store and job DB are closed, and adds a backward-compatibility test for the 3-argument call. ## Necessary prerequisite: several backend files were corrupted by a bad merge package.json, jest.config.js, tsconfig.json, config/index.ts, api/app.ts, api/routes/stream.ts, api/routes/agents.ts, api/routes/stats.ts, and api/routes/health.ts each contained two full, conflicting versions of their own content concatenated together, which blocked `npm install`/ `npm test` outright. Same root cause and fix already documented in Epta-Node#443 (a different fork, issue Epta-Node#359) and this repo's Epta-Node#460 (issue Epta-Node#353) — kept the newer half matching actual codebase usage in each file, discarded the stale duplicate. This PR's health.ts fix is the minimal corruption fix only (no /live or /ready extensions — those are Epta-Node#353's PR, Epta-Node#460); api/app.ts here additionally includes the close()/job-worker-drain change described above, which Epta-Node#460's app.ts deliberately does not. ## Acceptance Criteria - [x] In-flight tasks complete or resume on restart - [x] E2E test validates restart mid-stream ## Test plan - npx jest tests/shutdown.test.ts — 5/5 passing (full phase sequence, extras stopped, no more failRunningTasks, event store + job DB closed, 3-arg backward compatibility, forced-exit-on-timeout). - npx jest src/queue/worker.test.ts — 10/10 passing, including the new restart-mid-stream test. Closes Epta-Node#349
|
@prissca Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
@Obiajulu-gif is attempting to deploy a commit to the Jaja's projects Team on Vercel. A member of the Team first needs to authorize it. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
setupGracefulShutdown()already existed and was unit-tested, butsrc/index.ts's actual production entry point (main()) never called it — it had its own, much simpler inline SIGTERM/SIGINT handler that only closed the HTTP server, with no draining, no DB closes, no task/agent state cleanup. This PR wires the real function in and fixes the gaps that made it unsafe to rely on:api/app.ts'sclose()now awaitsjobWorker.stop(jobWorkerStopTimeoutMs)(newAppOptionsfield, default 10s) before closing the HTTP/WS server — previously it calledjobWorker.stop()without awaiting it, so "drain" did nothing: the server closed immediately regardless of in-flight work.taskDb.failRunningTasks()call fromsetupGracefulShutdown, which force-marked every running task's DAG nodes as failed on shutdown. That directly worked against the job queue's own resumption path:JobWorker.start()already callsrecoverIncompleteJobs(), which resets any job still"active"from a previous run back to"pending"for retry — a job stuck mid-drain is meant to resume, not be declared dead. (failRunningTasks()itself is left indb/tasks.tsin case it's useful elsewhere; it's just no longer called from the graceful-shutdown path.)eventBus.store.close()— the event store was never closed on shutdown even though every other DB was.src/index.ts'smain()now callssetupGracefulShutdown(httpServer, close, config, { cleanupService, reconciliationService, globalAgentRegistry })instead of its own inline handler;setupGracefulShutdown's signature gained an optional 4thextrasargument (backward compatible — the existing 3-argument test call still passes) to stop those three services, whichmain()previously stopped inline butsetupGracefulShutdownhad no way to reach before.src/queue/worker.test.ts: a job still"active"when aJobWorker.stop()call times out is picked up and completed by a freshJobWorkerinstance over the same store — the restart-mid-stream scenario the acceptance criteria asks for, exercised at the layer that actually owns resumption (recoverIncompleteJobs()).tests/shutdown.test.tsupdated: assertsfailRunningTasks/createTaskDbare no longer called, asserts the three extra services are stopped, asserts the event store and job DB are closed, and adds a backward-compatibility test for the 3-argument call.Necessary prerequisite: several backend files were corrupted by a bad merge
package.json,jest.config.js,tsconfig.json,config/index.ts,api/app.ts,api/routes/stream.ts,api/routes/agents.ts,api/routes/stats.ts, andapi/routes/health.tseach contained two full, conflicting versions of their own content concatenated together, which blockednpm install/npm testoutright. Same root cause and fix already documented in Epta-Node/ai-net#443 (a different fork, issue #359) and this repo's #460 (issue #353) — kept the newer half matching actual codebase usage in each file, discarded the stale duplicate. This PR'shealth.tsfix is the minimal corruption fix only (no/liveor/readyextensions — those are #353's PR, #460);api/app.tshere additionally includes theclose()/job-worker-drain change described above, which #460'sapp.tsdeliberately does not.Acceptance Criteria
Test plan
npx jest tests/shutdown.test.ts— 5/5 passing (full phase sequence, extras stopped, no morefailRunningTasks, event store + job DB closed, 3-arg backward compatibility, forced-exit-on-timeout).npx jest src/queue/worker.test.ts— 10/10 passing, including the new restart-mid-stream test.Closes #349