Skip to content
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.

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

Removal of IContainerRuntimeOptions.enableGroupedBatching in v2.20 #23270

Open
MarioJGMsoft opened this issue Dec 10, 2024 · 1 comment · May be fixed by #23336
Open

Removal of IContainerRuntimeOptions.enableGroupedBatching in v2.20 #23270

MarioJGMsoft opened this issue Dec 10, 2024 · 1 comment · May be fixed by #23336
Assignees
Labels
api deprecation Changes to a deprecated API triage

Comments

@MarioJGMsoft
Copy link
Contributor

Deprecated API

  • IContainerRuntimeOptions.enableGroupedBatching

Context

We can and should get rid of the mode where we send ops with empty content for Compression.

Configuration-wise, at this point both compression & op grouping are ON in main, and OFF in LTS. So, we can group them together into one modality and should stop mentioning (and dealing with) a modality where compression is on, but op grouping is off.

Since we will now always group a batch of 2 or more messages, we must remove the ability to configure this. Batch grouping will now be dependent on batch compression.

Then it’s just a matter of updating OpCompressor to require single-message batches and remove the empty placeholder code.

Approach

  1. Tag the deprecated option with the @deprecated tag.

  2. Create internal version of containerRuntimeOptions that has enableGroupedBatching and remove the existing option from containerRuntime.

  3. Remove the code that handles compression batches with multiple messages.

DocumentSchemaController.opGroupingEnabled will now depend on the value of the internal version of enableGroupedBatching

Update opCompressor.compressBatch() so that it receives a batch with a single message and stops creating the empty placeholder ops.

Update Outbox.flushInternal() so that it always groups batches with 2 or more messages and compresses them. It’s important to note that we also want to stop grouping and compressing blobAttach batches, since we won’t group them anymore.

Dependencies

The deprecation/removal of FlushMode.Immediate from IContainerRuntimeOptions.flushMode will use the same new IContainerRuntimeOptionsInternal, so those PRs will need to be thoughtfully merged.

Compatibility Concerns

To support 1.x clients we are creating an internal version of enableGroupedBatching that will be able to receive requests from said clients and we will have a mechanism to disable grouped batching and compression both.

There shouldn’t be any issues since currently there is no existing use of turning on grouping and turning off compression.

There also shouldn’t be any issues on the functionality of batch grouping depending on batch compression, since if the user has grouping available in their version of FF they should also have an existing configuration of compression. This is because compression was released before grouping within the FF code.

Expected Timeline

We expect the chanes to be made in Client 2.20 release, which is expected to be on 01/06/2025

@MarioJGMsoft MarioJGMsoft added the api deprecation Changes to a deprecated API label Dec 10, 2024
@MarioJGMsoft MarioJGMsoft linked a pull request Dec 12, 2024 that will close this issue
MarioJGMsoft added a commit that referenced this issue Dec 16, 2024
#23325)

## Description

This PR is a preparation for the breaking change that will happen when
we remove IContainerRuntimeOptions.enableGroupedBatching

## Breaking Changes

Link to the breaking issue:
#[23270](#23270)

## Reviewer Guidance

- Let me know if I missed something or if there's a better solution for
what I want to accomplish

Owning Task:
[AB#26793](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/26793)
@MarioJGMsoft MarioJGMsoft reopened this Dec 16, 2024
@MarioJGMsoft
Copy link
Contributor Author

When I merged the preparation for the breaking change it automatically marked it as closed, but the breaking change hasn't been made yet

@jason-ha jason-ha changed the title Remove batch grouping configuration Removal of IContainerRuntimeOptions.enableGroupedBatching in v2.20 Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api deprecation Changes to a deprecated API triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant