STOR-5615: Count full replay memory for JSRPC calls - #7558
Merged
Merged
Conversation
Contributor
|
Since last review: 0 resolved, 0 still open, 0 new. Carried forward from the last review: kj-style (no author changes in their files since then; earlier findings stand) Not re-run: tests, api-compat, docs, compat-flags, design-simplicity, jsg-gc, memory-safety (no author changes in their files since the last review) Reviewed commit: 3236e38b · github run |
apeacock1991
force-pushed
the
apeacock/STOR-5615-replay-memory-budget
branch
from
September 28, 2026 09:24
b5a8214 to
12e2696
Compare
jqmmes
approved these changes
Sep 28, 2026
apeacock1991
force-pushed
the
apeacock/STOR-5615-replay-memory-budget
branch
from
September 28, 2026 09:42
12e2696 to
aa2413b
Compare
jasnell
approved these changes
Sep 28, 2026
apeacock1991
force-pushed
the
apeacock/STOR-5615-replay-memory-budget
branch
3 times, most recently
from
September 28, 2026 13:00
2f66804 to
b2b5f30
Compare
The replay memory budget is a resource limit, but its hook was on RequestObserver, so an embedder had to enforce the limit in its metrics code. Move tryReserveActorCallReplayMemory() to LimitEnforcer. The default still refuses every reservation, so workerd's own server sends JSRPC calls without retries, as before. RequestObserver keeps the measurements. It now tracks every replayable call, including those with a reservation, so demand doesn't depend on the budget. The new recordActorCallReplayMemoryRejected() records each call sent without retries because the reservation was refused.
The replay memory reservation counted the used length of the serialized arguments, but the call plan keeps the whole serializer buffer. Tracked demand counted the buffer capacity, but not the call metadata or retry state. Count the buffer capacity in the reservation, and track the same bytes. The budget and the demand metric now use the same estimate for each call. Calls whose retry policy allows no retries used to take a reservation too, and held it until they settled even though they retain nothing to replay. Enough of them could fill the budget and stop other calls from retrying. They now neither reserve nor track replay memory.
apeacock1991
force-pushed
the
apeacock/STOR-5615-replay-memory-budget
branch
from
September 28, 2026 14:02
b2b5f30 to
3236e38
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.
Each JSRPC call now reports one replay memory estimate, whether or not it gets a reservation.
The observer's budget and demand metric now measure calls the same way. Demand no longer depends on whether a reservation succeeds.