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

refactor(container-runtime): Add IProtocolOptions and CompatibilityMode to container-runtime #23951

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

scottn12
Copy link
Contributor

@scottn12 scottn12 commented Feb 28, 2025

TODO

  1. Add tests to validate properties are applied correctly
  2. Fix circular import issue with CompressionAlgorithms

Description

This PR adds IProtocolOptions and CompatibilityMode (moved from fluid-static) to the container-runtime package.

IProtocolOptions

The options currently in IContainerRuntimeOptions can be categorized into two groups. The first group are options that we provide to customers to customize their FluidFramework experience depending on their needs (i.e. summaryOptions). The second group are options where their main function is to to maintain compliance with our cross-client compatibility policy. For example, enableGroupedBatching, since batching is a feature we would ideally recommend to all customers but is not compatible with 1.x clients.
IProtocolOptions contains the options in the second group. This was done to add clarity for both customers and FF devs on which options are compat-related.

In this PR's implementation, the container runtime accepts inputs from both IContainerRuntimeOptions and IProtocolOptions. If a property is provided in IProtocolOptions, it will overwrite the value in IContainerRuntimeOptions. Eventually, we will remove any properties from IContainerRuntimeOptions that overlap with IProtocolOptions.

CompatibilityMode

CompatibilityMode was added in this PR to lay the groundwork for easy upgrade paths for customers. Additional changes are required, but it will ultimately allow customers to specify the "minimum required version" to collaborate. It will ultimately accomplish the following:

  1. Bundle the appropriate configuraitons for collaboration with the "minimum required version".
  2. Disallow versions under the "minimum required version" from joining a session.
    By giving customers a "switch" they can flip once they reach saturation on a new version of FF, they will have an easy and clear upgrade path.

Note: Since container-runtime is a depedency of fluid-static, we needed to move CompatibilityMode to the container-runtime package to make it accessible within container-runtime.

Reviewer Guidance

  • Some features are currently enabled by default, which based on recent compat policy discussions should likely be disabled by default. See comment below on batching for more details.
  • Do we need to stage the removal of CompatibilityMode and compatibilityModeRuntimeOptions from fluid-static?

Next Steps

  1. Depreacate the properties on IContainerRuntimeOptions that overlap with IProtocolOptions (and eventually remove).
  2. Implement "disallow versions under the `minimum required version` from joining a session".

AB#31227

@github-actions github-actions bot added area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc public api change Changes to a public API base: main PRs targeted against main branch labels Feb 28, 2025
enableRuntimeIdCompressor,
chunkSizeInBytes = defaultChunkSizeInBytes,
enableGroupedBatching = true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something I noticed - we have enableGroupedBatching (and compression) on by default. Based on recent compat policy discussions, I believe this should be disabled by default, since it breaks compataiblity with an adjacent major public release (1.x).

@@ -33,3 +32,8 @@ export type {
MemberChangedListener,
Myself,
} from "./types.js";

// Re-export so other packages don't need to pull in container-runtime
// TODO: Should we re-export?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see a great reason to re-export the API in fluid-static, but I believe this would be a breaking change to a public API. Do we need to stage removing the API completely from fluid-static?

// @alpha
export interface IProtocolOptions {
readonly compressionOptions?: ICompressionRuntimeOptions;
readonly enableGCSweep?: true | undefined;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

gcOptions is a tricky case, since it contains a compatability/protocol related option. Ultimately, I decided to just include that specific option (enableGCSweep) in IProtocolOptions and leave the rest of in IContainerRuntimeOptions.

Copy link
Contributor

github-actions bot commented Mar 5, 2025

🔗 Found some broken links! 💔

Run a link check locally to find them. See
https://github.com/microsoft/FluidFramework/wiki/Checking-for-broken-links-in-the-documentation for more information.

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...

http://localhost:3000/docs/data-structures/tree
- (36:97) 'the Shar..' => http://localhost:3000/docs/api/tree (HTTP 404)

http://localhost:3000/docs/data-structures/tree/
- (36:97) 'the Shar..' => http://localhost:3000/docs/api/tree (HTTP 404)

http://localhost:3000/docs/data-structures/tree/schema-definition
- (30:128) 'SharedTr..' => http://localhost:3000/docs/api/tree (HTTP 404)

http://localhost:3000/docs/start/tree-start
- (44:4) 'the API ..' => http://localhost:3000/docs/api/tree/schemafactory-class (HTTP 404)
- (61:7) 'the API' => http://localhost:3000/docs/api/tree/treechangeevents-interface (HTTP 404)


Stats:
  162477 links
    1326 destination URLs
    1558 URLs ignored
       0 warnings
       3 errors

 ELIFECYCLE  Command failed with exit code 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: runtime Runtime related issues area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant