Skip to content

fix(supervisor): give each boundary outage its own wait recovery window - #5526

Open
mr-miles wants to merge 2 commits into
NVIDIA:mainfrom
mr-miles:fix/wait-recovery-window-reset
Open

mr-miles wants to merge 2 commits into
NVIDIA:mainfrom
mr-miles:fix/wait-recovery-window-reset

Conversation

@mr-miles

Copy link
Copy Markdown

Summary

A supervisor wait() survived only the first boundary transport loss in a sandbox's lifetime. call_wait_stream opened its 30s recovery window at the first failure and never reset it. After one successful reattach, any later loss found the window already expired and returned without trying to reattach. The supervisor then exited with boundary lost during wait. This PR gives each outage its own window.

Related Issue

Closes #5525

Changes

  • Replace begin_recovery_window with WaitRecoveryWindow, which tracks the recovery deadline per connection generation. A failure on a newer generation (a reattach replaced the connection) starts a fresh CONNECT_RETRY_TIMEOUT window.
  • Repeated failures on the same generation still share one bounded window. This matters because recover_after_unavailable returns Ok without reconnecting when the probe finds the connection live, and resetting on every Ok would let a live connection whose streams keep failing retry forever.
  • accept_tcp uses the same call_wait_stream path, so a second transport loss no longer permanently stops the network proxy either.
  • Port the existing wait_recovery_window_begins_at_transport_failure test. Add tests for:
    • failures on one connection sharing a window that still expires
    • a failure 43 minutes after a reattach getting a fresh window

Release Notes

Long-running sandboxes no longer exit when the supervisor loses its connection to the sandbox a second time. Each connection loss now gets a full reconnect window.

Testing

  • cargo fmt --check
  • cargo test -p openshell-sandbox-backend --lib
  • cargo clippy -p openshell-sandbox-backend --all-targets
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-bot Bot commented Nov 12, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

mr-miles and others added 2 commits November 12, 2026 20:26
A supervisor wait recovers from one boundary transport loss, but after
that recovery a later loss on the replacement connection is reported as
"boundary lost during wait" without any reattach attempt, once the first
outage's 30s recovery window has passed.

Add a regression test that holds a wait open across two transport
aborts more than CONNECT_RETRY_TIMEOUT apart and expects the wait to
reattach both times and return the exit status. It fails on this
commit; the following commit fixes it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
call_wait_stream opened its 30s recovery window at the first transport
failure and never reset it. A wait lasts as long as the workload, so
after one successful recovery any later transport loss found the window
long expired and returned Unavailable without attempting a reattach.
BoundaryProcess::wait() then reported "boundary lost during wait" and
the supervisor exited, even though the boundary was still reachable.
accept_tcp shares the same path and stopped the proxy the same way.

Key the window by connection generation. A failure on a newer
generation means a recovery replaced the connection, so it starts a
fresh window. Repeated failures on one generation still share a single
bounded window, so a live connection whose streams keep failing does
not retry forever.

This makes wait_survives_a_second_transport_loss_after_recovery pass,
and adds unit tests for the per-generation window itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mr-miles
mr-miles force-pushed the fix/wait-recovery-window-reset branch from ad53ac8 to d7b9635 Compare November 12, 2026 20:27

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(supervisor): wait() survives only one boundary transport loss; the second is fatal without a reattach attempt

1 participant