Skip to content

fix(api): make WatchSandbox loss-aware and resumable - #3209

Draft
letv1nnn wants to merge 9 commits into
NVIDIA:mainfrom
letv1nnn:3055-watch-resumable-cursor/letv1nnn
Draft

fix(api): make WatchSandbox loss-aware and resumable#3209
letv1nnn wants to merge 9 commits into
NVIDIA:mainfrom
letv1nnn:3055-watch-resumable-cursor/letv1nnn

Conversation

@letv1nnn

@letv1nnn letv1nnn commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Make WatchSandbox loss-aware and resumable. Recoverable broadcast lag now emits a SandboxStreamWarning and keeps streaming instead of terminating with RESOURCE_EXHAUSTED. Every resumable event (log line, platform event) carries a monotonic per-sandbox cursor, and a reconnecting client can pass resume_after_cursor to replay only what it missed. Unrecoverable loss (a trimmed cursor) terminates the stream with OUT_OF_RANGE so gaps are never silent.

Draft / work in progress — server, proto, and docs are complete; SDK reconnect helpers and e2e coverage are still pending (see TODO).

Related Issue

Refs #3055 (partial; issue stays open until SDK helpers land).

Changes

  • Lag is recoverable. Status, log, and platform receivers emit SandboxStreamWarning and continue on RecvError::Lagged; Closed stays terminal. Adds lag_warning / lag_warning_event helpers.
  • Shared cursor. TracingLogBus and PlatformEventBus draw sequence numbers from one per-sandbox SeqAllocator, so the merged stream is ordered in a single cursor space. Each buffered event is stamped at publish time.
  • Proto. Adds SandboxStreamEvent.cursor and WatchSandboxRequest.resume_after_cursor. cursor = 0 marks non-resumable events (status snapshots, warnings); resume_after_cursor = 0 means no cursor resume (tail-limited replay via log_tail_lines / event_tail).
  • Resume + gap detection. tail_after(cursor) returns events after a cursor or a ResumeGap when the requested cursor was trimmed. The producer replays both resumable sources after the cursor, merged in cursor order, then enters live delivery. A gap terminates with OUT_OF_RANGE carrying the requested and earliest-available cursors.
  • Exactly-once at the replay/live boundary. The producer tracks the highest replayed cursor and suppresses live events at or below it, so an event buffered during watch initialization is delivered once.
  • Teardown. TracingLogBus::remove clears both resumable per-sandbox maps before resetting the shared allocator, closing a publish-during-teardown window.
  • Docs. Documents the cursor contract, recoverable vs unrecoverable loss, and cross-source ordering in proto, architecture/gateway.md, and docs/observability/accessing-logs.mdx.

TODO (follow-up)

  • SDK reconnect/resume helpers (Go first: track cursor, resend resume_after_cursor, surface warnings, signal gap).
  • E2E: reconnect-no-loss, cursor expiry, stop_on_terminal ERROR regression.

Testing

  • cargo test -p openshell-server — all pass, including resume replay, cross-source merge order, duplicate suppression, gap → OUT_OF_RANGE, init-race single-delivery, and lag-warning regression.
  • mise run pre-commit
  • E2E — pending SDK helpers.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated
  • SDK helpers (tracked above)

…nating

Broadcast lag on the status, log, and platform receivers was converted to a RESOURCE_EXHAUSTED status that terminated the whole watch stream. Lag is recoverable: the receiver resumes at the oldest surviving message. Emit a SandboxStreamWarning and continue streaming instead; keep terminating on Closed. Add helpers and unit tests covering the warning payload and receiver recovery after lag.

Partially addresses NVIDIA#3055 (cursor/resume follow up separately).

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
…numbers

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Allocate cursors from a single SeqAllocator shared by the log and
platform event buses, so a sandbox's merged watch stream carries
unique, strictly increasing cursors. A single resume_after_cursor can
then unambiguously locate a client's position across both sources.

Rewrite both publish paths to allocate the sequence, stamp
event.cursor, send, and append to the tail under one lock. This
removes the previous get_mut().expect() TOCTOU race where a concurrent
remove() between the two lock sections could panic.

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Add tail_after() to the log and platform event buses, returning every
buffered event newer than a client's resume cursor. Each PerSandbox now
tracks last_trimmed_seq (the highest seq it has evicted) so a resume is
reported as an unrecoverable ResumeGap only when this bus dropped an
event the client still needs.

Judging gaps by evictions, not by the tail's oldest seq, is required
under the shared cursor space: each bus's tail is non-contiguous in the
global sequence because the other bus owns the missing seqs, so
comparing against tail.front() would flag false gaps.

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
… buses

Wire resume_after_cursor into the watch producer. On a non-zero cursor,
replay events strictly after it from both the log and platform buses,
merge by shared cursor, and emit in order before entering the live loop.
A trimmed range on either bus is an unrecoverable gap and terminates the
stream with OUT_OF_RANGE carrying the requested and earliest-available
cursors, distinct from recoverable lag which warns and continues.

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Add handler-level tests for the resumable watch stream: replay strictly
after the client cursor, merge log and platform events in shared-cursor
order, suppress duplicates when resuming at the latest cursor, and
terminate with OUT_OF_RANGE when the requested cursor has been trimmed.

Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 6, 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.

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.

1 participant