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

[BREAK IN 2.20.0] Remove support for Batch Grouping configuration #23336

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

MarioJGMsoft
Copy link
Contributor

@MarioJGMsoft MarioJGMsoft commented Dec 16, 2024

Description

After discussing the work in task #8124, we concluded that it was best to make Batch Grouping dependent on Batch Compression.

This means that we need to remove the ability for users to configure Batch Grouping.

Acceptance Criteria:
Fluid users can no longer configure Batch Grouping.

Execution Plan:
Remove Batch Grouping configuration from containerRuntime.ts and update it to the IContainerRuntimeOptionsInternal.enableGroupedBatching version.

Breaking Changes

This is a breaking change but IContainerRuntimeOptionsInternal has been created to still be compatible with different versions of FF.

Reviewer Guidance

Please let me know if you think there's anything missing that I should add to this PR or anything related to it

Fixes AB#26249

@github-actions github-actions bot added area: runtime Runtime related issues public api change Changes to a public API base: main PRs targeted against main branch labels Dec 16, 2024
@MarioJGMsoft MarioJGMsoft marked this pull request as ready for review January 6, 2025 23:29
@Copilot Copilot bot review requested due to automatic review settings January 6, 2025 23:29
@MarioJGMsoft MarioJGMsoft requested review from a team as code owners January 6, 2025 23:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

.changeset/quiet-days-carry.md Outdated Show resolved Hide resolved
@MarioJGMsoft MarioJGMsoft requested review from a team, pragya91, markfields, jatgarg, tyler-cai-microsoft, kian-thompson and rajatch-ff and removed request for a team January 7, 2025 00:08
"section": legacy
---

Grouped batching is enabled by default and IContainerRuntimeOptions.enableGroupedBatching has been removed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Grouped batching is enabled by default and IContainerRuntimeOptions.enableGroupedBatching has been removed
Grouped batching is now enabled by default and cannot be disabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied changes


Grouped batching is enabled by default and IContainerRuntimeOptions.enableGroupedBatching has been removed

This option allowed an application to disable grouped batching. But now we no longer want users to be able to turn it on or off, so this option can be removed and it will always be enabled by default.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This option allowed an application to disable grouped batching. But now we no longer want users to be able to turn it on or off, so this option can be removed and it will always be enabled by default.
The Grouped Batching feature in the container runtime has stabilized and is now enabled by default.
The `IContainerRuntimeOptions.enableGroupedBatching` option, which allowed an application to disable grouped batching, has been removed. Grouped Batching is now considered a core part of the container runtime, and as such can no longer be disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied changes

Copy link
Contributor

github-actions bot commented Jan 7, 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:
  170492 links
    1603 destination URLs
    1838 URLs ignored
       0 warnings
       0 errors


Copy link
Member

@tylerbutler tylerbutler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs look good; thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: runtime Runtime related issues base: main PRs targeted against main branch changeset-present public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removal of IContainerRuntimeOptions.enableGroupedBatching in v2.20
2 participants