New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update Documentation in OpLifecycle #23636

Merged

MarioJGMsoft merged 3 commits into microsoft:main from MarioJGMsoft:updateOpLifecycleDocs

Jan 29, 2025

Contributor

MarioJGMsoft commented Jan 22, 2025 •

edited by azure-boards bot

Loading

Description

When the changes in the feature are merged, the current README.md of OpLifecycle will be outdated, therefore we need to update it to match the current flow of ops.

Execution Plan

Update the README.md of the OpLifecycle folder.

Acceptance Criteria

The new README.md reflects the new flow of OpLifecycle.

Reviewer Guidance

Let me know if there's a better way to write the changes in the docs

Fixes: AB#28650


          docs: updated opLifecycle docs

b65ea02

MarioJGMsoft requested a review from a team

January 22, 2025 23:52

github-actions bot added the area: runtime label

MarioJGMsoft requested review from pragya91, markfields, jason-ha, jatgarg, tyler-cai-microsoft, kian-thompson and WillieHabi and removed request for a team

January 22, 2025 23:52

github-actions bot added the base: main label

MarioJGMsoft changed the title ~~docs: updated opLifecycle docs~~ Update Documentation in OpLifecycle

Member

markfields commented Jan 22, 2025

Thanks for doing this! Overall suggestion - Can you move the "Grouped batching" section above the "Compression" section? Then your new part about grouping being required will have enough context to make more sense.

markfields reviewed

View reviewed changes

packages/runtime/container-runtime/src/opLifecycle/README.md Outdated

Comment on lines 72 to 75

               Compressing a batch yields a batch with the same number of messages. It compresses all the content, shifting the compressed payload into the first op,
               leaving the rest of the batch's messages as empty placeholders to reserve sequence numbers for the compressed messages.
+              ### Single message batch update

Member

markfields Jan 23, 2025

I'd update this to emphasize the current reality more. So before this paragraph, start ### Only single-message batches are compressed. Then under that heading you can explain how compression can only be enabled if Grouped Batching is enabled, and that it wasn't always this way, and explain about how compressing a multi-message batch would yield empty placeholder ops (btw we don't say "operations"), but we don't do that anymore (but we do support reading those ops still)

Contributor Author

MarioJGMsoft Jan 23, 2025

Applied changes

markfields reviewed

View reviewed changes

packages/runtime/container-runtime/src/opLifecycle/README.md Outdated

Comment on lines 209 to 211

		## How it works (Grouped Batching disabled) -Outdated-

		### Outdated - Main code no longer creates empty ops

Member

markfields Jan 23, 2025

Suggested change

      
            ## How it works (Grouped Batching disabled) -Outdated-
          
            ### Outdated - Main code no longer creates empty ops
          
            ## Legacy behavior - Compression without Grouped Batching
          
            ### IMPORTANT - As of 2.20.0, we no longer compress ungrouped batches, but we do need to read such ops - read on to learn what these legacy ops look like

Or something like that

Contributor Author

MarioJGMsoft Jan 23, 2025

Applied changes


          docs: applied changes based on comments and discussion

517bfd8

markfields reviewed

View reviewed changes

packages/runtime/container-runtime/src/opLifecycle/README.md Outdated

@@ @@ -200,7 +209,9 @@ Ungrouped batch: @@
               +-----------------+-----------------+-----------------+-----------------+-----------------+
               ```
-              ## How it works (Grouped Batching disabled)
+              ## Legacy begavior - How it works (Grouped Batching disabled)

Member

markfields Jan 23, 2025

Suggested change

      
            ## Legacy begavior - How it works (Grouped Batching disabled)
          
            ## Legacy behavior - How it used to work (Compression+Chunking without Grouped Batching)

Contributor Author

MarioJGMsoft Jan 23, 2025

Applied changes

markfields reviewed

View reviewed changes

packages/runtime/container-runtime/src/opLifecycle/README.md

               Grouped Batching can be disabled by setting `IContainerRuntimeOptions.enableGroupedBatching` to `false`.
-              See [below](#how-grouped-batching-works) for an example.
+              See [below](#how-it-works) for an example.

Member

markfields Jan 23, 2025

Does this link work?

Contributor Author

MarioJGMsoft Jan 23, 2025

It didn't before so I fixed it.

markfields reviewed

View reviewed changes

packages/runtime/container-runtime/src/opLifecycle/README.md Outdated

		@@ -80,7 +68,7 @@ Grouped batching is only relevant for `FlushMode.TurnBased`, since `OpGroupingMa

		Grouped Batching can be disabled by setting `IContainerRuntimeOptions.enableGroupedBatching` to `false`.

Member

markfields Jan 23, 2025

Let's remove this line too

Contributor Author

MarioJGMsoft Jan 23, 2025

Applied changes

markfields reviewed

View reviewed changes

packages/runtime/container-runtime/src/opLifecycle/README.md Outdated


		### Only single-message batches are compressed

		The batch to compress has to have only one message and it yields a batch with a single message. It compresses all the content, storing the compressed payload into the first op.

Member

markfields Jan 23, 2025

Suggested change

      
            The batch to compress has to have only one message and it yields a batch with a single message. It compresses all the content, storing the compressed payload into the first op.
          
            The batch to compress has to have only one message and it yields a batch with a single message. It compresses all the content, replacing the op's original contents with the compressed payload and the appropriate metadata to indicate it's compressed.

Contributor Author

MarioJGMsoft Jan 23, 2025

Applied changes

markfields approved these changes

View reviewed changes

Member

markfields left a comment

Left a few final suggested edits


          docs: applied changes based on comments

dbab9d5

MarioJGMsoft marked this pull request as ready for review

January 23, 2025 18:58

Contributor

github-actions bot commented Jan 23, 2025

🔗 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:
  170508 links
    1603 destination URLs
    1842 URLs ignored
       0 warnings
       0 errors

MarioJGMsoft merged commit 71e5977 into microsoft:main

37 checks passed

MarioJGMsoft deleted the updateOpLifecycleDocs branch

January 29, 2025 23:07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

markfields markfields approved these changes

pragya91 Awaiting requested review from pragya91 pragya91 was automatically assigned from microsoft/team-rajat

jason-ha Awaiting requested review from jason-ha jason-ha was automatically assigned from microsoft/team-rajat

jatgarg Awaiting requested review from jatgarg jatgarg was automatically assigned from microsoft/team-rajat

tyler-cai-microsoft Awaiting requested review from tyler-cai-microsoft tyler-cai-microsoft was automatically assigned from microsoft/team-rajat

kian-thompson Awaiting requested review from kian-thompson kian-thompson was automatically assigned from microsoft/team-rajat

WillieHabi Awaiting requested review from WillieHabi WillieHabi was automatically assigned from microsoft/team-rajat

Labels

area: runtime base: main