Retry heartbeat timeouts by putting back in the queue #1689
Conversation
|
WalkthroughThis change introduces a new Changes
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/webapp/app/v3/taskRunHeartbeatFailed.server.tsOops! Something went wrong! :( ESLint: 8.45.0 ESLint couldn't find the config "custom" to extend from. Please check that the name of the config is correct. The config "custom" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/webapp/app/v3/taskRunHeartbeatFailed.server.ts (3)
52-77: Consider enhancing error messages with attempt information.The logic for handling PENDING tasks based on attempt count is well-implemented. However, the error message could be more informative by including the attempt count in the failure case.
- message: "Did not receive a heartbeat from the worker in time", + message: `Did not receive a heartbeat from the worker in time (after ${taskRun._count.attempts} attempt${taskRun._count.attempts === 1 ? '' : 's'})`,
54-54: Fix inconsistent service name in logs.The debug logs refer to "RequeueTaskRunService" but the class is named "TaskRunHeartbeatFailedService". This inconsistency should be fixed.
- logger.debug("[RequeueTaskRunService] Requeueing task run, there were no attempts.", { + logger.debug("[TaskRunHeartbeatFailedService] Requeueing task run, there were no attempts.", { - "[RequeueTaskRunService] Failing task run because the heartbeat failed, it's PENDING, locked, and has attempts", + "[TaskRunHeartbeatFailedService] Failing task run because the heartbeat failed, it's PENDING, locked, and has attempts",Also applies to: 61-61
71-75: Consider adding heartbeat timeout duration to error context.To aid in debugging, consider including the heartbeat timeout duration in the error context. This would help diagnose why runs are timing out.
error: { type: "INTERNAL_ERROR", code: TaskRunErrorCodes.TASK_RUN_HEARTBEAT_TIMEOUT, - message: "Did not receive a heartbeat from the worker in time", + message: "Did not receive a heartbeat from the worker in time", + context: { + heartbeatTimeoutMinutes: 15, // from PR description + lastHeartbeatAt: taskRun.lockedAt, + }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/v3/taskRunHeartbeatFailed.server.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/webapp/app/v3/taskRunHeartbeatFailed.server.ts (1)
33-37: LGTM! Well-structured query modification.The addition of the
_countfield to count attempts is clean and follows Prisma conventions.
matt-aitken
changed the title
* If there’s a heartbeat error and no attempts we put it back in the queue to try again * When nacking, return whether it was put back in the queue or not * Try and nack, if it fails then fail the run * Consolidated switch statement * Fail executing/retrying runs
* Uses image cards for the frameworks * Removes old snippets * New AI agents side menu section * WIP adding new ai agent pages * Better overview page * More copy added to the agent example pages * Copy improvements * Removes “Creating a project” page and side menu section * Fixes broken links * Updates to the latest Mintlify version, fixes issues, changes theme * Adds descriptions to the main dropdown menu items * Reformatted Introduction docs ‘landing page’ * Retry heartbeat timeouts by putting back in the queue (#1689) * If there’s a heartbeat error and no attempts we put it back in the queue to try again * When nacking, return whether it was put back in the queue or not * Try and nack, if it fails then fail the run * Consolidated switch statement * Fail executing/retrying runs * OOM retrying on larger machines (#1691) * OOM retrying on larger machines * Create forty-windows-shop.md * Update forty-windows-shop.md * Only retry again if the machine is different from the original * Kubernetes OOMs appear as non-zero sigkills, adding support for treating these as OOMs * Complete the original attempt span if retrying due to an OOM * Revert "Complete the original attempt span if retrying due to an OOM" This reverts commit 5f652c6. * chore: Update version for release (#1666) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Release 3.3.14 * Set machine when triggering docs * Batch queue runs that are waiting for deploy (#1693) * Detect ffmpeg OOM errors, added manual OutOfMemoryError (#1694) * Detect ffmpeg OOM errors, added manual OutOfMemoryError * Create eighty-spies-knock.md * Improved the machines docs, including the new OutOfMemoryError * chore: Update version for release (#1695) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Release 3.3.15 * Create new partitioned TaskEvent table, and switch to it gradually as new runs are created (#1696) * Create new partitioned TaskEvent table, and switch to it gradually as new runs are created * Add env var for partition window in seconds * Make startCreatedAt required in task event store * Don't create an attempt if the run is final, batchTriggerAndWait bad continue fix (#1698) * WIP fix for ResumeAttemptService selecting the wrong attempt (which has no error or output) * Don’t create an attempt if the run is already in a final status * Don’t get all the columns for the query. Improved the logging. * Added a log to the batch example * Filter out the undefined values * Fix missing logs on child runs by using the root task run createdAt if it exists (#1697) * Provider changes to support image cache (#1700) * add env var for additional pull secrets * make static images configurable * optional image prefixes * optional labels with sample rates * add missing core paths * remove excessive logs * Fix run container exits after OOM retries (#1701) * remove unused imports * tell run to exit before force requeue * handle exit for case where we already retried after oom * improve retry span and add machine props * don't try to exit run in dev * Upgrade local dev to use electric beta.15 (#1699) * Text fixes * Removed pnpm files --------- Co-authored-by: Matt Aitken <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Eric Allam <[email protected]> Co-authored-by: nicktrn <[email protected]>
We use heartbeats to prevent stalled runs from getting stuck forever. If 15 mins passes without a successful heartbeat from the worker for your run, we need to do something about it.
Ordinarily this is a rare issue, but when there is a lot of contention it is more prevalent.
There a few situations where this can happen:
In almost all situations we now put the run back in the queue to retry (it has a maximum number of retries it will do). If it can't be put back in the queue, because it's exceeded the maximum, then we'll fail the run as we do now.
This still isn't ideal because 15 mins have passed from when it should have run, but it does drastically increase the likelihood it will get successfully executed.
In Run Engine 2.0 the heartbeat/stall system has been reworked and we retry start failures much faster.
Summary by CodeRabbit
Bug Fixes
Documentation