Write new pending message format in PendingStateManager #16136
Conversation
github-actions
bot
added
area: runtime
| // IdAllocations need their localOpMetadata stashed in the contents | ||
| // of the op to correctly resume the session when processing stashed ops | ||
| if (content.type === ContainerMessageType.IdAllocation) { | ||
| content.contents.stashedState = message.localOpMetadata; |
There was a problem hiding this comment.
@justus-camp Is it possible to have this stashedState be part of the content when submitting the message (i.e. in maybeSubmitIdAllocationOp)?
I want to avoid parse then serialize as much as possible just to check the type add another prop to the content string
There was a problem hiding this comment.
I used localOpMetadata because we don't want to submit the stashedState over the wire but need it to rehydrate the IdCompressor for stashed ops. My initial prototype actually did what you're suggesting and deleted stashedState from the op before submission, but there's a check in PendingStateManager::processPendingLocalMessage that makes doing it this way difficult and causes a DataProcessingError. I suppose we could ignore IdAllocation ops in that check but that's pretty unfortunate.
It's worth noting that overall this stashed op implementation isn't great and we kind of "retro-fitted" the original compressor to do it this way.
There was a problem hiding this comment.
Is it possible to move the serialized state to a summary blob? I think it should, but I may be missing something. Regardless, I'll re-add the logic in the PendingStateManager for back/forward compat, but ideally the PSM wouldn't know about specific op types (can be fixed later on).
|
Please review this PR: @sonalideshpandemsft @tylerbutler @scottn12 |
|
I'll merge this PR after main/next integrate PR is merged: #16428 |
## Description The `PendingStateManager` will now only read the new pending message format. This is the final stage in the multi-step process of changing the format of pending local messages read and written by the `PendingStateManager`. See #16136 [AB#4763](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/4763)
In PendingStateManager.getLocalState(), write the new message format instead of the old format
See changes in #14462
AB#3826
Follow-up item for 2.0.0-internal.7.0.0: AB#4763