Repository navigation
feat(sandbox): add retry logic for transient container failures - #1232
Conversation
SandboxManager::execute_with_policy() had no retry logic. Transient Docker errors (daemon temporarily unavailable, container creation race conditions, container start failures) caused immediate job failure. Adds up to 2 retries (3 total attempts) with exponential backoff (2s, 4s) for transient error types only: - DockerNotAvailable - ContainerCreationFailed - ContainerStartFailed Non-transient errors (Timeout, ExecutionFailed, NetworkBlocked, Config) are returned immediately without retry. Container cleanup on retry is safe: ContainerRunner::execute() always force-removes the container before returning. Closes #1224 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness of sandbox execution by introducing a retry mechanism for transient Docker container failures. By identifying and retrying temporary issues like Docker daemon glitches or container creation races, it prevents jobs from failing prematurely due to intermittent infrastructure problems, thereby improving overall system reliability. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces retry logic for transient Docker container failures, which is a great improvement for robustness. The implementation is solid, extracting the container execution into a separate function and wrapping it in a retry loop with exponential backoff. The classification of transient errors and the accompanying tests are well-done. I have one suggestion to simplify the error handling after all retries are exhausted to make it more direct and improve error reporting.
| Err(last_err.unwrap_or_else(|| SandboxError::ExecutionFailed { | ||
| reason: "all retry attempts exhausted".to_string(), | ||
| })) |
There was a problem hiding this comment.
The logic of the retry loop ensures that last_err will always be Some if the loop completes, as it would have been set in the final iteration. The unwrap_or_else is therefore defensive against an unreachable state. Using unwrap() is more idiomatic here to assert this invariant. Furthermore, the fallback error SandboxError::ExecutionFailed is generic and loses the valuable context of the last actual transient error that occurred. Returning the last error is more informative for debugging, aligning with the principle of creating specific error variants for different failure modes to provide semantically correct and clear error messages.
Err(last_err.unwrap())References
- Create specific error variants for different failure modes (e.g., DownloadFailed with a URL string vs. ManifestRead with a file path) to provide semantically correct and clear error messages.
|
Re: Gemini's suggestion to use |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ai#1232) * feat(sandbox): add retry logic for transient container failures (nearai#1224) SandboxManager::execute_with_policy() had no retry logic. Transient Docker errors (daemon temporarily unavailable, container creation race conditions, container start failures) caused immediate job failure. Adds up to 2 retries (3 total attempts) with exponential backoff (2s, 4s) for transient error types only: - DockerNotAvailable - ContainerCreationFailed - ContainerStartFailed Non-transient errors (Timeout, ExecutionFailed, NetworkBlocked, Config) are returned immediately without retry. Container cleanup on retry is safe: ContainerRunner::execute() always force-removes the container before returning. Closes nearai#1224 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ai#1232) * feat(sandbox): add retry logic for transient container failures (nearai#1224) SandboxManager::execute_with_policy() had no retry logic. Transient Docker errors (daemon temporarily unavailable, container creation race conditions, container start failures) caused immediate job failure. Adds up to 2 retries (3 total attempts) with exponential backoff (2s, 4s) for transient error types only: - DockerNotAvailable - ContainerCreationFailed - ContainerStartFailed Non-transient errors (Timeout, ExecutionFailed, NetworkBlocked, Config) are returned immediately without retry. Container cleanup on retry is safe: ContainerRunner::execute() always force-removes the container before returning. Closes nearai#1224 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: fix formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Adds retry logic for transient Docker container failures (#1224).
SandboxManager::execute_with_policy()previously had no retry logic -- transient Docker errors (daemon restart, container creation race, cgroup conflicts) caused immediate job failure.Changes:
try_execute_in_container()helperis_transient_sandbox_error()classifier for retry-eligible errorsDockerNotAvailable,ContainerCreationFailed,ContainerStartFailedTimeout,ExecutionFailed,NetworkBlocked,Config, etc.ContainerRunner::execute()always force-removes before returningTest plan
transient_errors_are_retryable-- verifies all 3 transient variantsnon_transient_errors_are_not_retryable-- verifies Timeout, ExecutionFailed, NetworkBlocked, Configcargo clippy --all --all-features-- zero warningsCloses #1224