fix: let queued tasks start while a long task is running - #74
Open
LUCKYREDDY31 wants to merge 1 commit into
Open
LUCKYREDDY31 wants to merge 1 commit into
LUCKYREDDY31 wants to merge 1 commit into
Conversation
TaskWorker.tick() awaited every run it claimed before releasing its lock, so one slow task held up all queued work, scheduled watch checks and approved tasks until it finished. Start the claimed runs and release the tick right away, tracking started tasks in an inFlight set so a worker still runs at most three at once and never starts the same task twice.
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.
Problem
TaskWorker.tick()claims up to three due tasks and then awaits all of their runs before it releasesticking. Every later tick returns early, so no new task starts until the slowest run in the batch finishes.A model task can run for up to five minutes, and each browser request can take up to 45 seconds. While one runs, everything else waits: a quick job such as a spending summary stays
queued, a watch's scheduled check runs late, and a task whose review was just approved does not resume. The worker keeps writing its heartbeat during this time, so the app reports it as running and the person sees no reason for the delay. Nothing is lost or run twice; the work is only late.What changed
apps/server/src/engine/worker.ts:tick()now starts the runs it claims and releasestickingwithout waiting for them, so the next tick can start queued work. A newinFlightset tracks tasks this worker has started and not yet settled. It replaces theactivecheck when picking tasks, and the limit of three now counts tasks already in flight, so a worker still runs at most three at once.tick()still awaits the runs it started, so direct callers behave as before.stop()waits forinFlightinstead ofactive, which also covers a task that is still being claimed.tests/engine.test.ts: a regression test next to "pending reviews do not starve queued work". It holds one task open, queues a second, ticks again, and asserts the second task finished while the first is still running.Verification
main(the second task staysqueued) and passes with the fix. With only theworker.tschange reverted, it is the one failing test out of 207.pnpm lint,pnpm typecheck,pnpm --dir apps/worker typecheck,pnpm test(207/207) andpnpm build:serverpass on Node 24 and Node 22. On Node 24, the web, iOS and Android exports,pnpm test:browser, and both Docker container suites also pass.mainboth waited until 17.1s, when the slow task finished.stop()still requeued an interrupted task, and cancelling a running task still stopped it.Security review