fix(orch): keep PostInit Timestamp stable across retries to break envd livelock#2687
Closed
ValentaTomas wants to merge 3 commits into
Closed
fix(orch): keep PostInit Timestamp stable across retries to break envd livelock#2687ValentaTomas wants to merge 3 commits into
ValentaTomas wants to merge 3 commits into
Conversation
…d livelock doRequestWithInfiniteRetries previously set jsonBody.Timestamp = time.Now() on every iteration of the retry loop. envd's PostInit guards SetData with lastSetTime.SetToGreater(initRequest.Timestamp.UnixNano()), which is meant to skip work for an out-of-order or replayed request — but with a fresh timestamp per retry the guard always observed a newer value and ran SetData end-to-end every time. When the orchestrator's 50 ms EnvdInitRequestTimeout fires while envd is still serving the cold first request, envd queues the next retry on initLock and the next one and the next one. Each one then runs SetData against a connection the orchestrator already gave up on, writes 204 to a dead socket, and the orchestrator never observes a success — the livelock described in the bitfrost / Mode-B investigation. Pin Timestamp once before the loop. On the cold path envd's idempotency guard now actually fires for replays, the queue drains by skipping the duplicates, and the next live retry's response reaches the orchestrator. Also marshal the body once instead of on every retry (it doesn't change).
PR SummaryMedium Risk Overview Reusing a single marshaled request body across retries means any per-attempt fields (like deadlines) are no longer reflected in the payload; combined with the pinned timestamp this could also trigger unintended system time adjustments if the first attempt is significantly delayed. Reviewed by Cursor Bugbot for commit df5007c. Bugbot is set up for automated code reviews on this repo. Configure here. |
❌ 8 Tests Failed:
View the top 1 failed test(s) by shortest run time
View the full list of 21 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
Pinning the timestamp before the retry loop causes the sandbox system clock to be set to a stale value if initialization takes significant time, leading to clock skew that affects TLS verification and log ordering. If a request fails after updating the idempotency guard but before completing initialization, subsequent retries with the same pinned timestamp will skip initialization logic and return success, leaving the sandbox in an inconsistent state.
With the orchestrator now pinning Timestamp once across retries, a SetData failure would otherwise leave lastSetTime updated and cause subsequent retries with the same timestamp to skip the (still-needed) SetData and return 204 success. Move the SetToGreater after the success check.
Member
Author
|
Superseded by #2700 (move envd into a dedicated network namespace), which addresses the root cause. |
Member
Author
|
Close this to prevent risking outdated time. |
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
2 participants
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.
Pin
Timestamponce before the retry loop so envdlastSetTime.SetToGreatercan skip replays; marshal body once.