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.

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

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

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

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

wants to merge 8 commits into from
+10 −12

Conversation

MarioJGMsoft
Copy link
Contributor

@MarioJGMsoft MarioJGMsoft commented Dec 16, 2024 •
edited by azure-boards bot
Loading

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 linked an issue Dec 16, 2024 that may be closed by this pull request
Removal of IContainerRuntimeOptions.enableGroupedBatching in v2.20 #23270
Open
@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
Copilot
Copilot AI reviewed Jan 6, 2025
View reviewed changes
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
@github-actions GitHub Actions
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

tylerbutler
tylerbutler reviewed Jan 7, 2025
View reviewed changes
.changeset/quiet-days-carry.md
"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

.changeset/quiet-days-carry.md

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.

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

Copilot Copilot left review comments

@tylerbutler tylerbutler tylerbutler left review comments

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

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

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

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

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

@rajatch-ff rajatch-ff Awaiting requested review from rajatch-ff rajatch-ff was automatically assigned from microsoft/team-rajat

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Removal of IContainerRuntimeOptions.enableGroupedBatching in v2.20
2 participants
@MarioJGMsoft @tylerbutler