WS3: guard the unbounded tree paths (GetTree, AC Tree validation) - #33
Closed
shreyas-blacksmith wants to merge 1 commit into
Closed
WS3: guard the unbounded tree paths (GetTree, AC Tree validation)#33shreyas-blacksmith wants to merge 1 commit into
shreyas-blacksmith wants to merge 1 commit into
Conversation
…ion) GetTree materializes the entire directory tree into one in-memory response with no pagination, and GetValidatedActionResult reads every referenced output-directory Tree blob wholly into memory - the two remaining paths where a single request's memory is unbounded. Neither size is known up front, so they cannot take the batch paths' up-front reservation; instead: - WithGetTreeLimits adds a fail-fast concurrency slot and a running response byte cap checked as the traversal discovers directories, both returning retryable ResourceExhausted and reporting trips to an optional metrics sink. - WithTreeValidationSizeLimit pre-flight checks the declared Tree bytes per ActionResult validation and reports over-cap results as a cache miss (always semantically safe - the client rebuilds), with a trip callback. Both default off; standalone bazel-remote behavior is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
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
Third and final workstream of the bazel-remote memory-envelope program (stacked on #32, which stacks on #27). This PR guards the two request paths whose memory is discovered mid-flight rather than declared up front, so no up-front reservation can ever cover them:
WithGetTreeLimits— GetTree materializes the entire directory tree into one in-memory response (no pagination upstream). The guard is a fail-fast concurrency slot (TryAcquire, no queueing) plus a running response byte cap decremented by each directory's declared size before fetching it. Both trips return retryableResourceExhaustedand report to an optional metrics sink. Worst-case memory = slots × byte cap.WithTreeValidationSizeLimit—GetValidatedActionResultreads every referenced output-directoryTreeblob wholly into memory. Sizes are declared in the ActionResult, so this is a pre-flight sum check; over-cap results degrade to a cache miss (always semantically safe — the client rebuilds), never an error, with a trip callback.Both options default off; standalone bazel-remote behavior is unchanged.
The program (WS1–3)
sync.Pool), 1 MiB windows, waiter ceiling, batch identity-fallbackWithMaxEntriesresident-entry cap (~268 B/entry measured)Test plan
TestGetTreeConcurrencyGuardFailsFast— held slot denies immediately, release restores service, handler releases its slotTestGetTreeResponseByteCap— over-cap aborts mid-traversal, cap smaller than root denies pre-unmarshal, exact cap serves the full treeTestGetTreeGuardsEndToEnd— option plumbing over the wireTestTreeValidationSizeLimit*— hit→miss flip attributable to the guard alone; exact cap admits; trip callback fires once with declared bytes./cache/... ./server/...suite passesMade with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled. (Staging)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.