-
Notifications
You must be signed in to change notification settings - Fork 539
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,9 @@ | |
// @alpha | ||
export const AllowTombstoneRequestHeaderKey = "allowTombstone"; | ||
|
||
// @public | ||
export type CompatibilityMode = "1" | "2"; | ||
|
||
// @alpha | ||
export enum CompressionAlgorithms { | ||
// (undocumented) | ||
|
@@ -192,6 +195,7 @@ export interface IContainerRuntimeMetadata extends ICreateContainerMetadata, IGC | |
// @alpha | ||
export interface IContainerRuntimeOptions { | ||
readonly chunkSizeInBytes?: number; | ||
readonly compatibilityMode?: CompatibilityMode; | ||
readonly compressionOptions?: ICompressionRuntimeOptions; | ||
// @deprecated | ||
readonly enableGroupedBatching?: boolean; | ||
|
@@ -201,6 +205,7 @@ export interface IContainerRuntimeOptions { | |
readonly gcOptions?: IGCRuntimeOptions; | ||
readonly loadSequenceNumberVerification?: "close" | "log" | "bypass"; | ||
readonly maxBatchSizeInBytes?: number; | ||
readonly protocolOptions?: IProtocolOptions; | ||
// (undocumented) | ||
readonly summaryOptions?: ISummaryRuntimeOptions; | ||
} | ||
|
@@ -354,6 +359,16 @@ export interface IOnDemandSummarizeOptions extends ISummarizeOptions { | |
readonly retryOnFailure?: boolean; | ||
} | ||
|
||
// @alpha | ||
export interface IProtocolOptions { | ||
readonly compressionOptions?: ICompressionRuntimeOptions; | ||
readonly enableGCSweep?: true | undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// @deprecated | ||
readonly enableGroupedBatching?: boolean; | ||
readonly enableRuntimeIdCompressor?: IdCompressorMode; | ||
readonly explicitSchemaControl?: boolean; | ||
} | ||
|
||
// @alpha @deprecated | ||
export interface IRefreshSummaryAckOptions { | ||
readonly ackHandle: string; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -155,6 +155,10 @@ import { | |
getSummaryForDatastores, | ||
wrapContext, | ||
} from "./channelCollection.js"; | ||
import { | ||
defaultCompatibilityMode, | ||
type CompatibilityMode, | ||
} from "./compatibilityConfiguration.js"; | ||
import { IPerfSignalReport, ReportOpPerfTelemetry } from "./connectionTelemetry.js"; | ||
import { ContainerFluidHandleContext } from "./containerHandleContext.js"; | ||
import { channelToDataStore } from "./dataStore.js"; | ||
|
@@ -486,12 +490,71 @@ export interface ICompressionRuntimeOptions { | |
readonly compressionAlgorithm: CompressionAlgorithms; | ||
} | ||
|
||
/** | ||
* TODO: TSDoc update | ||
* Options for container runtime related to protocol/data format. | ||
* All clients connected to the same container must be able to understand the same protocol. | ||
* @legacy | ||
* @alpha | ||
*/ | ||
export interface IProtocolOptions { | ||
/** | ||
* Enable the IdCompressor in the runtime. | ||
* @experimental Not ready for use. | ||
*/ | ||
readonly enableRuntimeIdCompressor?: IdCompressorMode; | ||
|
||
/** | ||
* If enabled, the runtime will group messages within a batch into a single | ||
* message to be sent to the service. | ||
* The grouping and ungrouping of such messages is handled by the "OpGroupingManager". | ||
* | ||
* By default, the feature is enabled. This feature can only be disabled when compression is also disabled. | ||
* @deprecated The ability to disable Grouped Batching is deprecated and will be removed in a future release. This feature is required for the proper functioning of the Fluid Framework. | ||
*/ | ||
readonly enableGroupedBatching?: boolean; | ||
|
||
/** | ||
* When this property is set to true, it requires runtime to control is document schema properly through ops | ||
* The benefit of this mode is that clients who do not understand schema will fail in predictable way, with predictable message, | ||
* and will not attempt to limp along, which could cause data corruptions and crashes in random places. | ||
* When this property is not set (or set to false), runtime operates in legacy mode, where new features (modifying document schema) | ||
* are engaged as they become available, without giving legacy clients any chance to fail predictably. | ||
*/ | ||
readonly explicitSchemaControl?: boolean; | ||
|
||
/** | ||
* Flag that if true, will enable the full Sweep Phase of garbage collection for this session, | ||
* where Tombstoned objects are permanently deleted from the container. | ||
* | ||
* IMPORTANT: This only applies if this document is allowed to run Sweep Phase. | ||
* | ||
* Current default behavior is for Sweep Phase not to delete Tombstoned objects, | ||
* but merely to prevent them from being loaded. | ||
*/ | ||
readonly enableGCSweep?: true | undefined; | ||
|
||
/** | ||
* Enables the runtime to compress ops. See {@link ICompressionRuntimeOptions}. | ||
*/ | ||
readonly compressionOptions?: ICompressionRuntimeOptions; | ||
} | ||
|
||
/** | ||
* Options for container runtime. | ||
* @legacy | ||
* @alpha | ||
*/ | ||
export interface IContainerRuntimeOptions { | ||
/** | ||
* TODO: TSDoc | ||
*/ | ||
readonly protocolOptions?: IProtocolOptions; | ||
/** | ||
* TODO: TSDoc | ||
*/ | ||
readonly compatibilityMode?: CompatibilityMode; | ||
|
||
readonly summaryOptions?: ISummaryRuntimeOptions; | ||
readonly gcOptions?: IGCRuntimeOptions; | ||
/** | ||
|
@@ -990,21 +1053,39 @@ export class ContainerRuntime | |
|
||
const mc = loggerToMonitoringContext(logger); | ||
|
||
// Options without overlap with IProtocolOptions | ||
const { | ||
summaryOptions = {}, | ||
gcOptions = {}, | ||
loadSequenceNumberVerification = "close", | ||
flushMode = defaultFlushMode, | ||
maxBatchSizeInBytes = defaultMaxBatchSizeInBytes, | ||
chunkSizeInBytes = defaultChunkSizeInBytes, | ||
protocolOptions = {}, | ||
compatibilityMode = defaultCompatibilityMode, | ||
}: IContainerRuntimeOptionsInternal = runtimeOptions; | ||
// Options with overlap with IProtocolOptions | ||
let { | ||
compressionOptions = runtimeOptions.enableGroupedBatching === false | ||
? disabledCompressionConfig // Compression must be disabled if Grouping is disabled | ||
: defaultCompressionConfig, | ||
maxBatchSizeInBytes = defaultMaxBatchSizeInBytes, | ||
enableRuntimeIdCompressor, | ||
chunkSizeInBytes = defaultChunkSizeInBytes, | ||
enableGroupedBatching = true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). |
||
explicitSchemaControl = false, | ||
}: IContainerRuntimeOptionsInternal = runtimeOptions; | ||
|
||
// While we transition to using IProtocolOptions, we will need to handle duplicate properties. | ||
// For now, any properties defined in protocolOptions will overwrite duplicate properties in runtimeOptions. | ||
scottn12 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
enableGroupedBatching = protocolOptions.enableGroupedBatching ?? enableGroupedBatching; | ||
explicitSchemaControl = protocolOptions.explicitSchemaControl ?? explicitSchemaControl; | ||
compressionOptions = | ||
enableGroupedBatching === false | ||
? disabledCompressionConfig // Compression must be disabled if Grouping is disabled | ||
: defaultCompressionConfig; | ||
gcOptions.enableGCSweep = protocolOptions.enableGCSweep ?? gcOptions.enableGCSweep; | ||
enableRuntimeIdCompressor = | ||
protocolOptions.enableRuntimeIdCompressor ?? enableRuntimeIdCompressor; | ||
|
||
const registry = new FluidDataStoreRegistry(registryEntries); | ||
|
||
const tryFetchBlob = async <T>(blobName: string): Promise<T | undefined> => { | ||
|
@@ -1206,6 +1287,8 @@ export class ContainerRuntime | |
enableRuntimeIdCompressor: enableRuntimeIdCompressor as "on" | "delayed", | ||
enableGroupedBatching, | ||
explicitSchemaControl, | ||
protocolOptions, | ||
compatibilityMode, | ||
}; | ||
|
||
const runtime = new containerRuntimeCtor( | ||
|
There was a problem hiding this comment.
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?