Repository navigation
spaced: share payload admission with native execution calls - #702
Merged
Merged
Conversation
ravi-pplx
added this pull request to stack #703
October 20, 2026 07:29
ravi-pplx
force-pushed
the
ravi/guest-execution-admission
branch
from
October 21, 2026 05:41
202841f to
26d8968
Compare
ravi-pplx
force-pushed
the
ravi/guest-execution-admission
branch
from
October 21, 2026 18:19
26d8968 to
46a52dc
Compare
kumarpallav-pplx
requested review from
ac-pplx and
zibi-pplx
and removed request for
ac-pplx
October 21, 2026 18:35
zibi-pplx
approved these changes
Oct 21, 2026
zibi-pplx
left a comment
Collaborator
There was a problem hiding this comment.
Approving. I checked the admission model against the head:
- Transport and handler weights come from the same
PAYLOAD_METHODS/START_PAYLOAD_SLOTS/BATCH_REQUEST_WEIGHTconstants, andQueuedSlotswraps the sameArc<Semaphore>the handlers use, so the daemon path is not double-charged (the daemon'sProcessadapter forwards requests unchanged). RequestPermit::try_acquireonly reuses a permit from the same budget (Arc::ptr_eq) with enough weight, and the semaphores are private to the service, so a native caller can't substitute capacity.- Permits now live until owned work and cleanup settle, which matches the RFC. The red/green tests for mixed pressure, cancellation and exact size limits are good.
Non-blocking follow-ups:
- Permit handoff through request extensions is an implicit contract (inline). It might be worth a guard.
- Behaviour change worth watching: holding permits through cleanup keeps capacity reserved longer under cancellation-heavy load, so
RESOURCE_EXHAUSTEDrates may rise. That's more correct, and probably worth a counter. - The README could spell out the native error contract:
OutOfRangefor oversized messages,ResourceExhaustedfor capacity, andpplx_space_common_grpc::is_refused_before_startfor retry decisions. That way embedders don't parse messages. crates/libs/spaced/**andcrates/libs/common/grpc/**aren't inci/kind-pull-paths.txt, so PR CI never runs the Kind/Firecracker smoke for this series. Since this code runs in every guest, please run it before merging, or consider adding those paths.- By inspection the per-call overhead is negligible: an atomic check,
encoded_len(), and oneArcper admitted call. A before/after run of the guest filesystem/process microbenchmarks across the stack would confirm it.
Review prepared with AI assistance (Claude Code); findings checked against the PR head.
Contributor
Author
|
For the remaining review notes:
|
Use the existing weighted payload budgets and protobuf message limits at shared process and filesystem entrypoints. Reuse transport permits from the same budget and retain them through owned execution and cleanup. Expose an owned filesystem service using the generated trait. Exercise mixed native/gRPC capacity, cancellation, size boundaries and worker idempotency across both entrypoints.
Return plain OutOfRange for oversized native messages, reserving retryable refusal metadata for capacity errors. Extend the size-boundary regression to check that distinction. Require an acquired permit in the write relay, document that process adapters preserve request extensions, and clarify the test name and docs.
ravi-pplx
force-pushed
the
ravi/guest-execution-admission
branch
from
October 22, 2026 16:43
8d2b4fa to
1b328d0
Compare
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.
Summary
Apply shared message limits and weighted payload budgets to native execution calls. Reuse transport permits and retain capacity through owned cleanup. Add a daemon permit-handoff regression and require Kind smoke for spaced/shared-gRPC changes.
Implements admission from the embedding RFC. Wire and identity contracts are unchanged.
Validation
CI on this head, including Kind/Firecracker, is pending. Admission, cancellation, size boundaries and daemon permit handoff are covered by the affected suite validated at #833.
Severity and risk
Moderate: native calls could bypass transport payload limits. Risk is earlier capacity refusal under overload. Revert with dependent changes to roll back.
AI assistance: Codex contributed code, tests and this description.