Write support for blob handles with pending payload #24458

ChumpChief · 2025-04-25T00:58:29Z

Ports the remainder of the prototype in #24158, which is write support for blob handles with pending payload.

Mostly unchanged as compared to the prototype, other than updating for the renames taken in #24320.

Copilot

Pull Request Overview

This PR ports write support for blob handles with pending payload by adding a new flag (createBlobPayloadPending) and updating tests, API signatures, and internal handling of blob payload states. Key changes include:

Updating test files to run with and without pending payload support.
Adding new type guards and API exports (e.g. isFluidHandlePayloadPending) to support pending payload states.
Updating blob manager logic to handle payload state transitions (local, shared, failed).

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/test/test-end-to-end-tests/src/test/blobsisAttached.spec.ts	Refactored imports and wrapped tests in a loop for createBlobPayloadPending variations.
packages/test/test-end-to-end-tests/src/test/blobs.spec.ts	Updated test container config and test descriptions to pass the new flag.
packages/runtime/runtime-utils/*	Export and add new type guard for handling pending blob payloads.
packages/runtime/container-runtime/src/*	Updated blob manager methods and tests to support blob handles with pending payloads.
packages/common/core-interfaces/*	Added new interfaces and types for the pending payload feature.

packages/runtime/container-runtime/src/test/blobManager.spec.ts

Co-authored-by: Copilot <[email protected]>

github-actions · 2025-04-25T01:07:01Z

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> [email protected] serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  163679 links
    1315 destination URLs
    1547 URLs ignored
       0 warnings
       0 errors

anthony-murphy · 2025-04-25T16:51:38Z

packages/common/core-interfaces/src/handles.ts

+ * @legacy
+ * @alpha
+ */
+export type PayloadState = "local" | "shared" | "pending" | "failed";


i'm trying to think through the usage here. what are the supported state transitions? how do users reason about them? What is the experience for local client vs remote clients? are they symmetrical? should they be, or do client need custom code for local and remote.

i think this would be easier to review if the implementation were split from api, as the api will likely have significantly more comments than the internals. getting the internals in would also unblock re-enabling testing with staging mode.

i think we should remove 'local' at this layer, and only have "persisted" | "pending" | "failed", anything related to local should be moved to a blob handle type such that local information is only available on the strongly typed local object returned from the call to upload.

anthony-murphy · 2025-04-25T20:56:17Z

packages/common/core-interfaces/src/handles.ts

+	(event: "shared", listener: () => void);
+	/**
+	 * Emitted for locally created handles when the payload fails sharing to remote collaborators.
+	 */
+	(event: "failed", listener: (error: unknown) => void);


i think we should remove both of these, and just have a single event for progress. Maybe something like:
(event: "progress", listener: ({type:"update"} | {type:"persisted"} | {type:"failed" error?:Error})=>void)

this ensure the event is applicable to all clients, and it lets the handle implementation short-circuit the event if someone registers while the state is already terminal (persisted or failed). we do this for a number of events like connected, as we saw users frequently building dangling event handles that never resolved as the connection state was already connected.

something like progress is probably optional for now, but someday i could see us using signals in the blob manager so broadcast progress updates.

anthony-murphy · 2025-04-25T21:02:57Z

packages/runtime/container-runtime/src/blobManager/blobManager.ts

 {
 	private attached: boolean = false;

 	public get isAttached(): boolean {
 		return this.routeContext.isAttached && this.attached;
 	}

+	private readonly _events = new TypedEventEmitter<IFluidHandlePayloadPendingEvents>();


i have some concerns about memory usage. a simple callback would be much cheaper in terms of memory, especially as we don't think will be used very often, as after a handle is in a terminal state there will never be a need to listen to events again

another middle ground option would be to lazily initialize the TypedEventEmitter, so it only exists when it is accessed. emission would need to change to not create it if it doesn't already exist

Will look at options here. I was also thinking I should probably look at switching to the new Tree emitter too.

ChumpChief added 5 commits April 24, 2025 14:20

Port core-interfaces portion

34eac3e

Port runtime-utils portion

f715d3a

Port BlobManager changes

6f7beca

Fixup tests

6510b24

Add end-to-end-tests

5e8872d

Copilot AI review requested due to automatic review settings April 25, 2025 00:58

ChumpChief requested a review from a team as a code owner April 25, 2025 00:58

github-actions bot added base: main PRs targeted against main branch area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc public api change Changes to a public API labels Apr 25, 2025

Merge branch 'main' into PendingPayloadWrite

854d288

Copilot AI reviewed Apr 25, 2025

View reviewed changes

packages/runtime/container-runtime/src/test/blobManager.spec.ts Outdated Show resolved Hide resolved

Update packages/runtime/container-runtime/src/test/blobManager.spec.ts

aff9689

Co-authored-by: Copilot <[email protected]>

ChumpChief requested review from anthony-murphy, markfields, znewton, yann-achard-MS and dannimad April 25, 2025 01:44

anthony-murphy reviewed Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write support for blob handles with pending payload #24458

Write support for blob handles with pending payload #24458

ChumpChief commented Apr 25, 2025 •

edited by azure-boards bot

Loading

Copilot AI left a comment

github-actions bot commented Apr 25, 2025

anthony-murphy Apr 25, 2025 •

edited

Loading

anthony-murphy Apr 25, 2025 •

edited

Loading

anthony-murphy Apr 25, 2025 •

edited

Loading

anthony-murphy Apr 25, 2025 •

edited

Loading

anthony-murphy Apr 25, 2025

anthony-murphy Apr 25, 2025

ChumpChief Apr 25, 2025

Write support for blob handles with pending payload #24458

Are you sure you want to change the base?

Write support for blob handles with pending payload #24458

Conversation

ChumpChief commented Apr 25, 2025 • edited by azure-boards bot Loading

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

github-actions bot commented Apr 25, 2025

linkcheck output

anthony-murphy Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

anthony-murphy Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

anthony-murphy Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

anthony-murphy Apr 25, 2025 • edited Loading

Choose a reason for hiding this comment

anthony-murphy Apr 25, 2025

Choose a reason for hiding this comment

anthony-murphy Apr 25, 2025

Choose a reason for hiding this comment

ChumpChief Apr 25, 2025

Choose a reason for hiding this comment

ChumpChief commented Apr 25, 2025 •

edited by azure-boards bot

Loading

anthony-murphy Apr 25, 2025 •

edited

Loading

anthony-murphy Apr 25, 2025 •

edited

Loading

anthony-murphy Apr 25, 2025 •

edited

Loading

anthony-murphy Apr 25, 2025 •

edited

Loading