Support applyingStashedOps while detached#19802
Merged
anthony-murphy merged 16 commits intoFeb 28, 2024
Merged
Conversation
92c9ca7 to
55c3243
Compare
…to stash-while-detached
80e44ee to
4620137
Compare
4620137 to
ad1fa45
Compare
…to stash-while-detached
…to stash-while-detached
1b582a9 to
57ce319
Compare
anthony-murphy
commented
Feb 27, 2024
anthony-murphy
commented
Feb 27, 2024
anthony-murphy
commented
Feb 27, 2024
| * Creates the load data from the client. The load data include everything needed to load a new client. It includes the summaries and the minimumSequenceNumber. | ||
| * @internal | ||
| */ | ||
| export function createLoadData(client: Client<IChannelFactory>): ClientLoadData { |
Contributor
Author
There was a problem hiding this comment.
this is new, the rest is copy pasted from dds test harness. this function unifies getting a load data out of a client, which was replicated all over the place
anthony-murphy
commented
Feb 27, 2024
| const localOpMetadata = await this.stateHandler.applyStashedOp(nextMessage.content); | ||
| nextMessage.localOpMetadata = localOpMetadata; | ||
| if (!this.stateHandler.isAttached()) { | ||
| if (localOpMetadata !== undefined) { |
Contributor
Author
There was a problem hiding this comment.
if we are not attached, then there shouldn't be any local op metadata, and we don't track pending messages, as there are not messages when not attached.
anthony-murphy
commented
Feb 27, 2024
| public applyStashedOp(content: any): unknown { | ||
| try { | ||
| this.stashedOpMd = createStashedOpMetadata(); | ||
| this.stashedOpMd = this.isAttachedAndVisible() ? createStashedOpMetadata() : undefined; |
Contributor
Author
There was a problem hiding this comment.
only create and return stashed op metadata if we are attached and visible, otherwise we'll just pass undefined up, and it will be dropped by pending state manager
Collaborator
⯅ @fluid-example/bundle-size-tests: +809 Bytes
Baseline commit: dca5a73 |
alex-pardes
approved these changes
Feb 28, 2024
…l.fuzz.spec.ts Co-authored-by: alex-pardes <105307164+alex-pardes@users.noreply.github.com>
connorskees
approved these changes
Feb 28, 2024
Co-authored-by: Connor Skees <39542938+connorskees@users.noreply.github.com>
anthony-murphy
added a commit
that referenced
this pull request
Mar 1, 2024
…19893) ## Background This change is the final change for allowing consumers of fluid to call serialize on a container while a container is in an attaching state, which will include a failed attach where the failure didn't cause the container to close. This will aid in the case of offline, network, or server outage by allowing the application to capture the un-attached state of the container, preserve it locally, and later create a new file from it. [AB#5502](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/5502) ## Overview This change enables calling container.serialize while a container is in the attaching state. It is the culmination of many smaller changes that have gone in to enable this change. - #18829 - #19400 - #19246 - #19517 - #19518 - #19590 - #19634 - #19802 --------- Co-authored-by: Tony Murphy <anthonm@microsoft.com> Co-authored-by: Daniel Madrid <105010181+dannimad@users.noreply.github.com> Co-authored-by: jzaffiro <110866475+jzaffiro@users.noreply.github.com> Co-authored-by: Matt Rakow <ChumpChief@users.noreply.github.com>
This was referenced May 6, 2026
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.
Background
This change is in preparation for allowing consumers of fluid to call serialize on a container while a container is in an attaching state, which will include a failed attach where the failure didn't cause the container to close. This will aid in the case of offline, network, or server outage by allowing the application to capture the un-attached state of the container, preserve it locally, and later create a new file from it.
AB#5502
Overview
The majority of these changes are test changes which simulate having DDS enter the attaching state, capturing their state, and then rehydrating to a new instance of the DDS.
This was made possible with #19518. As that change made DDS apply stashed ops via the DDSes existing functions to apply local changes, and since those existing functions already supported the detached state, there were no additional per DDS modifications necessary to apply stashed ops on a rehydrated detached container.
Beyond the test changes this change makes the attachment state visible to the apply stashed op pipeline, so it can know to not expect new ops to be generated when detached, and not populate the pending state manager.
The next PR (AB#7077) of this work will enable capturing the pending state while detached. That change will bring end to end test coverage that can't be done currently. However, there is substantial coverage for the internals in this PR via the integration with the stress harness.
AB#7078