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

Deprecate enableGroupedBatching #23260

Merged
merged 10 commits into from
Dec 11, 2024
Merged

Deprecate enableGroupedBatching #23260

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
86a3ece
refactor(Grouped Batching): removed ability to configure group batching.
MarioJGMsoft Dec 5, 2024
8556f61
Revert "refactor(Grouped Batching): removed ability to configure grou…
MarioJGMsoft Dec 6, 2024
ffa9632
docs: tag enableGroupedBatching as deprecated
MarioJGMsoft Dec 6, 2024
31c0337
update API to show deprecated tag for enableGroupedBatching
MarioJGMsoft Dec 6, 2024
b7fd0cc
docs: added changeset
MarioJGMsoft Dec 6, 2024
8a89e37
Merge branch 'main' into removeBatchGroupingConfiguration
MarioJGMsoft Dec 6, 2024
06c8c6e
docs: Applied changes to changeset based on comments
MarioJGMsoft Dec 6, 2024
269041c
Merge remote-tracking branch 'refs/remotes/fork/removeBatchGroupingCo…
MarioJGMsoft Dec 6, 2024
d39744d
docs: Applied changes based on comments to changeset
MarioJGMsoft Dec 11, 2024
8a1bc5d
Merge branch 'main' into removeBatchGroupingConfiguration
MarioJGMsoft Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/sad-dodos-sell.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@fluidframework/container-runtime": minor
"@fluidframework/fluid-static": minor
---
---
"section": deprecation
---

Marked `IContainerRuntimeOptions.enableGroupedBatching` as deprecated
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
Marked `IContainerRuntimeOptions.enableGroupedBatching` as deprecated
IContainerRuntimeOptions.enableGroupedBatching is now deprecated

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious if removing the inline code formatting was intentional. I personally do like to see it even in headers.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it was intentional. Opinions vary, but last time we discussed this the general consensus was that code formatting in headings is more distracting than helpful, especially when so much of every heading is code.

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


- We will remove the ability to disable Grouped Batching in v2.20.0. The only exception (i.e. where Grouped Batching would be disabled) is for compatibility with older (v1) clients, and this will be implemented without needing to expose `IContainerRuntimeOptions.enableGroupedBatching`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- We will remove the ability to disable Grouped Batching in v2.20.0. The only exception (i.e. where Grouped Batching would be disabled) is for compatibility with older (v1) clients, and this will be implemented without needing to expose `IContainerRuntimeOptions.enableGroupedBatching`.
We will remove the ability to disable Grouped Batching in v2.20.0. The only exception (i.e. where Grouped Batching would be disabled) is for compatibility with older (v1) clients, and this will be implemented without needing to expose `IContainerRuntimeOptions.enableGroupedBatching`.

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
Member

Choose a reason for hiding this comment

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

Suggested change
- We will remove the ability to disable Grouped Batching in v2.20.0. The only exception (i.e. where Grouped Batching would be disabled) is for compatibility with older (v1) clients, and this will be implemented without needing to expose `IContainerRuntimeOptions.enableGroupedBatching`.
The `IContainerRuntimeOptions.enableGroupedBatching` property is deprecated and will be removed in version 2.20.0. This will mean that the grouped batching feature can no longer be disabled. In versions 2.20.0 and beyond, grouped batching is required for the proper functioning of the Fluid Framework.
The sole case where grouped batching will be disabled is for compatibility with older v1 clients, and this will be implemented without any need for the configurable `IContainerRuntimeOptions.enableGroupedBatching` option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

1 change: 1 addition & 0 deletions ...ages/runtime/container-runtime/api-report/container-runtime.legacy.alpha.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ export interface IContainerRuntimeMetadata extends ICreateContainerMetadata, IGC
export interface IContainerRuntimeOptions {
readonly chunkSizeInBytes?: number;
readonly compressionOptions?: ICompressionRuntimeOptions;
// @deprecated
readonly enableGroupedBatching?: boolean;
markfields marked this conversation as resolved.
Show resolved Hide resolved
readonly enableRuntimeIdCompressor?: IdCompressorMode;
readonly explicitSchemaControl?: boolean;
Expand Down
1 change: 1 addition & 0 deletions packages/runtime/container-runtime/src/containerRuntime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ export interface IContainerRuntimeOptions {
* The grouping an ungrouping of such messages is handled by the "OpGroupingManager".
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
* The grouping an ungrouping of such messages is handled by the "OpGroupingManager".
* The grouping and ungrouping of such messages is handled by the "OpGroupingManager".

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

*
* By default, the feature is enabled.
* @deprecated The ability to configure Grouped Batching is now removed and it is now disabled if compression is disabled.
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
* @deprecated The ability to configure Grouped Batching is now removed and it is now disabled if compression is disabled.
* @deprecated Grouped Batching can no longer be disabled, it is required for the proper functioning of the Fluid Framework

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't technically true. We can't change existing functionality in a deprecation as that's effectively a breaking change. This deprecation is instead notifying consumers that they soon won't be able to disable the feature, and that there is no alternative as "it is required for the proper functioning of the Fluid Framework".

Suggested change
* @deprecated The ability to configure Grouped Batching is now removed and it is now disabled if compression is disabled.
* @deprecated The ability to disable Grouped Batching is deprecated and will be removed in v2.20.0. This feature is required for the proper functioning of the Fluid Framework.

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

*/
readonly enableGroupedBatching?: boolean;

Expand Down
Loading