Repository navigation
fix(runtime): admit resource reservations all-or-nothing - #2084
Merged
Merged
Conversation
A rejected reservation charged the process ledger, then undid that charge when the VM ledger was full. The undo stored a notify_one permit, so reserve_when_available woke itself and retried without ever returning Pending. With one runtime worker the task that frees VM capacity never ran, and a large guest HTTPS download made the sidecar unresponsive. ResourceLedger::reserve_all checks every request in every scope under ordered locks and charges only when all fit, so a rejected admission has no side effects. reserve, CapabilityRegistry::reserve, and HTTP/2 stream admission use it. Fixes #2083.
|
This PR was not deployed automatically as @eersnington does not have access to the Railway project. In order to get automatic PR deploys, please add @eersnington to your workspace on Railway. |
Contributor
📟 Pip-Boy ·
|
| check | new in this PR | fixed | existing on base |
|---|---|---|---|
| advisories | ✅ 0 | 0 | 28 |
| licenses | ✅ 0 | 0 | 0 |
| sources | ✅ 0 | 0 | 0 |
| bans | ✅ 0 | 0 | 0 |
| unused | ✅ 0 | 0 | 16 |
🦀 Binaries · publish Dockerfiles, --release
Base: main@3c60c9a, the merge base.
| artifact | npm @latest |
base | this PR | Δ vs base |
|---|---|---|---|---|
agentos-native-sidecar linux-x64-gnu |
143.8 MB | 143.8 MB | 143.8 MB | −1.6 KB (−0.0%) |
⏱ Timing
| job | wall time | slowest step |
|---|---|---|
zizmor |
10s | checkout · 4s |
changes |
37s | checkout · 32s |
native · linux-x64-gnu |
14m52s | Build with the publish Dockerfile · 13m47s |
hygiene |
1m01s | checkout · 25s |
⏭ footprint |
— | skipped |
⏭ self-test |
— | skipped |
report |
29s | Write the comment and run summary · 10s |
🔭 Full CI trace for this commit
One OpenTelemetry trace of CI and Pip-Boy for 339e226: every job and step, with the sizes above attached. Run after CI finishes.
pnpm install --frozen-lockfile --filter 'pipboy...'
pnpm --filter pipboy pipboy trace --sha 339e2269e330d1acebb1dcea00e37184dec570fbAdd --otlp <url> to send it to any OTLP/HTTP collector.
Agent instruction: From a rivet-dev/agentos checkout, run the commands above, then read the trace file the last command prints. List the slowest jobs and steps and explain where CI time went.
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
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.
Fixes #2083. Reported and diagnosed in #2083.
On a 1-CPU host, a guest HTTPS download larger than the VM's buffer limits made the sidecar unresponsive, and the client killed it after 30 s. Every VM on the sidecar was lost.
ResourceLedger::reservecharged the process ledger first and undid the charge when the VM ledger was full. The undo callednotify_one(), which stores a permit when nobody is waiting yet.reserve_when_availablethen consumed its own permit, retried, failed, and stored a new one. It never returnedPending, so on one runtime worker the task that frees VM capacity never ran. The waiter here isAsyncCompletionSender::send(crates/native-sidecar/src/state.rs).CapabilityRegistry::reservehad the same loop on a single ledger: it took the capability and ready-handle counts, failed onSockets, and released them.flowchart LR R["reserve / CapabilityRegistry::reserve / HTTP/2 admission"] --> A["ResourceLedger::reserve_all"] A --> L["lock process ledger, then VM ledger"] L --> C{"all requests fit<br/>in every scope?"} C -- no --> E["LimitError: no charge, no notify"] C -- yes --> OK["charge all, return Reservations"]ResourceLedger::reserve_all(&[(ResourceClass, usize)])checks every request in every scope under ordered locks, then charges. A rejected admission changes nothing, so a waiter can no longer wake itself.reservecallsreserve_allwith one request.CapabilityRegistry::reserveandreserve_http2_resourcesadmit their counts in one call.maybe_warn,observe_usage) take no ledger lock.fetch, Linuxmaintaskset -c 0)sidecar unresponsive