ContaineRuntime: Refactor batch processing code to support either op-by-op or batch-all-at-once semantics #22501
Conversation
This file contains 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
github-actions
bot
added
base: main
markfields
changed the title
⯅ @fluid-example/bundle-size-tests: +1.66 KB
Baseline commit: de91c3a |
kian-thompson
approved these changes
Sep 13, 2024
markfields
added a commit
that referenced
this pull request
Sep 16, 2024
…g for the whole batch (#22508) We are concerned that holding batch messages in ContainerRuntime even while DeltaManager advances its tracked sequence numbers through the batch could have unintended consequences. So this PR restores the old behavior of processing each message in a batch one-by-one, rather than holding until the whole batch arrives. Note that there's no change in behavior here for Grouped Batches. ### How the change works PR #21785 switched the RemoteMessageProcessor from returning ungrouped batch ops as they arrived, to holding them and finally returning the whole batch once the last arrived. The downstream code was also updated to take whole batches, whereas before it would take individual messages and use the batch metadata to detect batch start/end. Too many other changes were made after that PR to straight revert it. Logic was added throughout CR and PSM that looks at info about that batch which is found on the first message in the batch. So we can reverse the change and process one-at-a-time, but we need a way to carry around that "batch start info" with the first message in the batch. So we are now modeling the result that RMP yields as one of three cases: - A full batch of messages (could be from a single-message batch or a Grouped Batch) - The first message of a multi-message batch - The next message in a multi-message batch The first two cases include the "batch start info" needed for the recent Offline work. The third case just indicates whether it's the last message or not. #22501 added some of the necessary structure, introducing the type for "batch start info" and updating the downstream code to use that instead of reading it off the old "Inbound Batch" type. This PR now adds those other two cases to the RMP return type and handles processing them throughout CR and PSM.
markfields
added a commit
to markfields/FluidFramework
that referenced
this pull request
Sep 26, 2024
ContaineRuntime: Refactor batch processing code to support either op-by-op or batch-all-at-once semantics (microsoft#22501) No behavior change here, just refactoring to be able to express the result of RemoteMessageProcessor in terms of ops or batches. Follow-up change to adjust the logic will be much more scoped then.
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
Step one of AB#15221
No behavior change here, just refactoring to be able to express the result of RemoteMessageProcessor in terms of ops or batches.
Follow-up change to adjust the logic will be much more scoped then.
Reviewer Guidance
No behavior change. Notice that even in test files, the change is strictly refactoring, test results/asserts are unchanged.