Track outstanding attachment blob work in BlobManager. - #28357
Open
Jatin Garg (jatgarg) wants to merge 2 commits into
Open
Jatin Garg (jatgarg) wants to merge 2 commits into
Jatin Garg (jatgarg) wants to merge 2 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 90f3ba4d-f966-4ac8-a85c-168c32014877
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 90f3ba4d-f966-4ac8-a85c-168c32014877
github-actions
Bot
added
area: tools
area: runtime
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The lifecycle tracking is internally consistent and comprehensively covers success, retry, restoration, failure, and abort paths.
Review effort: Balanced
Findings: None
What changed in this PR
Adds BlobManager-owned tracking for attachment blobs whose sharing is incomplete, including restored pending blobs and terminal cleanup paths.
Changes:
- Adds outstanding-blob count, state accessor, and change event.
- Preserves tracking across retries and resubmissions until BlobAttach processing.
- Adds comprehensive lifecycle tests and synchronous storage-failure cleanup.
| File | Description |
|---|---|
blobManager.ts |
Implements outstanding-work tracking and synchronous failure cleanup. |
index.ts |
Exports the internal event interface. |
blobManager.spec.ts |
Tests tracking across creation, failure, abort, retry, and resubmission. |
pendingBlobs.spec.ts |
Tests tracking for restored pending blobs. |
blobTestUtils.ts |
Adds storage overrides and transition-recording helpers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (646 lines, 5 files), I've queued these reviewers:
How this works
|
Contributor
Bundle size comparisonBase commit: Notable changes
Per-bundle deltas
|
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.
Description
Adds a complete BlobManager-owned view of attachment blobs whose sharing has started but has not finished.
This is independently testable preparation for AB#81181. Nothing subscribes to the new event in this PR, so it does not yet change
ContainerRuntime.isDirty,"dirty", or"saved". The later activation PR will consume this state only after the separate loader/runtime op-state foundation has landed.BlobManager now tracks outstanding work in an idempotent
Setkeyed by local blob ID. A blob enters when BlobManager takes responsibility for sharing it and leaves when:BlobAttachoperation is processed;It remains continuously tracked across upload completion, TTL-driven re-upload, disconnect, and
BlobAttachresubmission. Referenced blobs restored from pending local state are tracked immediately, before sharing resumes. Merely creating and dropping a payload-pending handle does not start tracking.The acknowledgement boundary matters because storage upload alone does not make the service retain the blob. Processing
BlobAttachpublishes the local-to-storage ID mapping and establishes temporary retention until a summary sustains the reference.The
Setmakes duplicate start/stop paths safe: restored sharing may start after construction, and successful completion is observed both synchronously duringBlobAttachprocessing and later by the upload promise'sfinally.This PR also fixes a pre-existing cleanup gap when
storage.createBlob()throws synchronously. The synchronous path now performs the same listener, cache, and pending-handle cleanup as an asynchronously rejected upload.The new accessor and event are internal. No customer-facing API report changes are generated, and no changeset is included because container behavior is not activated here.