diff --git a/BREAKING.md b/BREAKING.md index 8ebb901e736a..5fde38da2699 100644 --- a/BREAKING.md +++ b/BREAKING.md @@ -136,6 +136,9 @@ As a short term fix the return type of this method can be safely casted to an IF - [IDirectory extends IDisposable](#IDirectory-extends-IDisposable) - [raiseContainerWarning removed from IContainerContext](#raiseContainerWarning-removed-from-IContainerContext) - [`IContainerRuntimeBase.setFlushMode` is deprecated](#icontainerruntimebasesetflushmode-is-deprecated) +- [connected deprecated from IContainer, IFluidContainer, and FluidContainer](#connected-deprecated-from-IContainer-IFluidContainer-and-FluidContainer) +- [setAutoReconnect and resume deprecated from IContainer and Container](#setAutoReconnect-and-resume-deprecated-from-IContainer-and-Container) +- [IContainer.connect() and IContainer.disconnect() will be made mandatory in future major release](#icontainer-connect-and-icontainer-disconnect-will-be-made-mandatory-in-future-major-release) ### Doing operations not allowed on deleted sub directory Users will not be allowed to do operations on a deleted directory. Users can subscribe to `disposed` event to know if a sub directory is deleted. Accessing deleted sub directory will throw `UsageError` exception now. @@ -149,6 +152,19 @@ IDirectory has started extending IDisposable. This means that users implementing ### `IContainerRuntimeBase.setFlushMode` is deprecated `IContainerRuntimeBase.setFlushMode` is deprecated and will be removed in a future release. FlushMode will become an immutable property for the container runtime, optionally provided at creation time via the `IContainerRuntimeOptions` interface. See [#9480](https://github.com/microsoft/FluidFramework/issues/9480#issuecomment-1084790977) +### connected deprecated from IContainer, IFluidContainer, and FluidContainer +`connected` has been deprecated from `IContainer`, `IFluidContainer`, and `FluidContainer`. It will be removed in a future major release. Use `connectionState` property on the respective interfaces/classes instead. Please switch to the new APIs as soon as possible, and provide any feedback to the FluidFramework team if necessary. +``` diff +- if (fluidContainer.connected) ++ if (fluidContainer.connectionState === ConnectionState.Connected) +``` + +### setAutoReconnect and resume deprecated from IContainer and Container +`setAutoReconnect()` and `resume()` have been deprecated from `IContainer` and `Container`. They will be removed in a future major release. Use `connect()` instead of `setAutoReconnect(true)` and `resume()`, and use `disconnect()` instead of `setAutoReconnect(false)`. Note, when using these new functions you will need to ensure that the container is both attached and not closed to prevent an error being thrown. Please switch to the new APIs as soon as possible, and provide any feedback to the FluidFramework team if necessary. + +### IContainer.connect() and IContainer.disconnect() will be made mandatory in future major release +In major release 1.0, the optional functions `IContainer.connect()` `IContainer.disconnect()` will be made mandatory functions. + ## 0.58 Breaking changes - [Move IntervalType from merge-tree to sequence package](#Move-IntervalType-from-merge-tree-to-sequence-package) - [Remove logger property from IContainerContext](#Remove-logger-property-from-IContainerContext) diff --git a/api-report/container-loader.api.md b/api-report/container-loader.api.md index 90207f022f25..87edecccc624 100644 --- a/api-report/container-loader.api.md +++ b/api-report/container-loader.api.md @@ -67,12 +67,16 @@ export class Container extends EventEmitterWithErrorHandling i // (undocumented) get closeSignal(): AbortSignal; // (undocumented) + connect(): void; + // (undocumented) get connected(): boolean; // (undocumented) get connectionState(): ConnectionState; static createDetached(loader: Loader, codeDetails: IFluidCodeDetails): Promise; // (undocumented) get deltaManager(): IDeltaManager; + // (undocumented) + disconnect(): void; forceReadonly(readonly: boolean): void; // (undocumented) getAbsoluteUrl(relativeUrl: string): Promise; @@ -96,13 +100,13 @@ export class Container extends EventEmitterWithErrorHandling i request(path: IRequest): Promise; // (undocumented) get resolvedUrl(): IResolvedUrl | undefined; - // (undocumented) + // @deprecated resume(): void; get scopes(): string[] | undefined; // (undocumented) serialize(): string; get serviceConfiguration(): IClientConfiguration | undefined; - // (undocumented) + // @deprecated setAutoReconnect(reconnect: boolean): void; // (undocumented) get storage(): IDocumentStorageService; diff --git a/api-report/fluid-static.api.md b/api-report/fluid-static.api.md index a1dcbbdafe5e..24effa80ca90 100644 --- a/api-report/fluid-static.api.md +++ b/api-report/fluid-static.api.md @@ -6,6 +6,7 @@ import { AttachState } from '@fluidframework/container-definitions'; import { BaseContainerRuntimeFactory } from '@fluidframework/aqueduct'; +import { ConnectionState } from '@fluidframework/container-definitions'; import { DataObject } from '@fluidframework/aqueduct'; import { IAudience } from '@fluidframework/container-definitions'; import { IChannelFactory } from '@fluidframework/datastore-definitions'; @@ -42,6 +43,7 @@ export class FluidContainer extends TypedEventEmitter imp attach(): Promise; get attachState(): AttachState; get connected(): boolean; + get connectionState(): ConnectionState; create(objectClass: LoadableObjectClass): Promise; dispose(): void; get disposed(): boolean; @@ -59,7 +61,9 @@ export interface IConnection { export interface IFluidContainer extends IEventProvider { attach(): Promise; readonly attachState: AttachState; + // @deprecated readonly connected: boolean; + readonly connectionState: ConnectionState; create(objectClass: LoadableObjectClass): Promise; dispose(): void; readonly disposed: boolean; diff --git a/common/lib/container-definitions/api-report/container-definitions.api.md b/common/lib/container-definitions/api-report/container-definitions.api.md index 839ad9d8b55b..9f7ffbad646f 100644 --- a/common/lib/container-definitions/api-report/container-definitions.api.md +++ b/common/lib/container-definitions/api-report/container-definitions.api.md @@ -129,9 +129,12 @@ export interface IContainer extends IEventProvider, IFluidRout close(error?: ICriticalContainerError): void; closeAndGetPendingLocalState(): string; readonly closed: boolean; + connect?(): void; + // @deprecated readonly connected: boolean; readonly connectionState: ConnectionState; deltaManager: IDeltaManager; + disconnect?(): void; // @alpha forceReadonly?(readonly: boolean): any; getAbsoluteUrl(relativeUrl: string): Promise; @@ -143,10 +146,10 @@ export interface IContainer extends IEventProvider, IFluidRout readonly readOnlyInfo: ReadOnlyInfo; request(request: IRequest): Promise; resolvedUrl: IResolvedUrl | undefined; - // @alpha + // @deprecated resume?(): void; serialize(): string; - // @alpha + // @deprecated setAutoReconnect?(reconnect: boolean): void; } diff --git a/common/lib/container-definitions/src/loader.ts b/common/lib/container-definitions/src/loader.ts index 1de9174eb272..d07cb06c631e 100644 --- a/common/lib/container-definitions/src/loader.ts +++ b/common/lib/container-definitions/src/loader.ts @@ -254,20 +254,37 @@ export interface IContainer extends IEventProvider, IFluidRout /** * Boolean indicating whether the container is currently connected or not + * @deprecated - 0.58, This API will be removed in 1.0 + * Check `connectionState === ConnectionState.Connected` instead + * See https://github.com/microsoft/FluidFramework/issues/9167 for context */ readonly connected: boolean; + /** + * Attempts to connect the container to the delta stream and process ops + */ + connect?(): void; + + /** + * Disconnects the container from the delta stream and stops processing ops + */ + disconnect?(): void; + /** * Dictates whether or not the current container will automatically attempt to reconnect to the delta stream * after receiving a disconnect event * @param reconnect - Boolean indicating if reconnect should automatically occur - * @alpha + * @deprecated - 0.58, This API will be removed in 1.0 + * Use `connect()` and `disconnect()` instead of `setAutoReconnect(true)` and `setAutoReconnect(false)` respectively + * See https://github.com/microsoft/FluidFramework/issues/9167 for context */ setAutoReconnect?(reconnect: boolean): void; /** * Have the container attempt to resume processing ops - * @alpha + * @deprecated - 0.58, This API will be removed in 1.0 + * Use `connect()` instead + * See https://github.com/microsoft/FluidFramework/issues/9167 for context */ resume?(): void; diff --git a/examples/data-objects/webflow/src/document/index.ts b/examples/data-objects/webflow/src/document/index.ts index ae210a4038a2..24389bfe3c48 100644 --- a/examples/data-objects/webflow/src/document/index.ts +++ b/examples/data-objects/webflow/src/document/index.ts @@ -163,7 +163,7 @@ export class FlowDocument extends LazyLoadedDataObject>(textId); if (handle === undefined) { diff --git a/packages/dds/merge-tree/src/mergeTree.ts b/packages/dds/merge-tree/src/mergeTree.ts index b523f38750ae..526e07c0837a 100644 --- a/packages/dds/merge-tree/src/mergeTree.ts +++ b/packages/dds/merge-tree/src/mergeTree.ts @@ -1036,10 +1036,6 @@ export class MergeTree { private static readonly initBlockUpdateActions: BlockUpdateActions; private static readonly theUnfinishedNode = { childCount: -1 }; - // WARNING: - // Setting blockUpdateMarkers to false will result in eventual consistency issues - // for property updates on markers when loading from snapshots - private static readonly blockUpdateMarkers = true; root: IMergeBlock; private readonly blockUpdateActions: BlockUpdateActions = MergeTree.initBlockUpdateActions; @@ -1060,12 +1056,7 @@ export class MergeTree { } private makeBlock(childCount: number) { - let block: MergeBlock; - if (MergeTree.blockUpdateMarkers) { - block = new HierMergeBlock(childCount); - } else { - block = new MergeBlock(childCount); - } + const block: MergeBlock = new HierMergeBlock(childCount); block.ordinal = ""; return block; } diff --git a/packages/dds/merge-tree/src/test/beastTest.ts b/packages/dds/merge-tree/src/test/beastTest.ts index 7c7f40985786..4db35ab47f25 100644 --- a/packages/dds/merge-tree/src/test/beastTest.ts +++ b/packages/dds/merge-tree/src/test/beastTest.ts @@ -1665,7 +1665,7 @@ export class DocumentTree { } private generateClient() { - const client = new TestClient({ blockUpdateMarkers: true }); + const client = new TestClient(); client.startOrUpdateCollaboration("Fred"); for (const child of this.children) { this.addToMergeTree(client, child); @@ -1730,7 +1730,7 @@ export class DocumentTree { } function findReplacePerf(filename: string) { - const client = new TestClient({ blockUpdateMarkers: true }); + const client = new TestClient(); loadTextFromFile(filename, client.mergeTree); const clockStart = clock(); diff --git a/packages/dds/merge-tree/src/test/client.conflictFarm.spec.ts b/packages/dds/merge-tree/src/test/client.conflictFarm.spec.ts index cc4207aabcad..b722310e964f 100644 --- a/packages/dds/merge-tree/src/test/client.conflictFarm.spec.ts +++ b/packages/dds/merge-tree/src/test/client.conflictFarm.spec.ts @@ -71,7 +71,7 @@ describe("MergeTree.Client", () => { const mt = random.engines.mt19937(); mt.seedWithArray([0xDEADBEEF, 0xFEEDBED, minLength]); - const clients: TestClient[] = [new TestClient({ blockUpdateMarkers: true })]; + const clients: TestClient[] = [new TestClient()]; clients.forEach( (c, i) => c.startOrUpdateCollaboration(clientNames[i])); diff --git a/packages/dds/merge-tree/src/test/client.reconnectFarm.spec.ts b/packages/dds/merge-tree/src/test/client.reconnectFarm.spec.ts index 449ef2afb659..a8443c18507d 100644 --- a/packages/dds/merge-tree/src/test/client.reconnectFarm.spec.ts +++ b/packages/dds/merge-tree/src/test/client.reconnectFarm.spec.ts @@ -76,7 +76,7 @@ describe("MergeTree.Client", () => { const mt = random.engines.mt19937(); mt.seedWithArray([0xDEADBEEF, 0xFEEDBED, clientCount]); - const clients: TestClient[] = [new TestClient({ blockUpdateMarkers: true })]; + const clients: TestClient[] = [new TestClient()]; clients.forEach( (c, i) => c.startOrUpdateCollaboration(clientNames[i])); diff --git a/packages/dds/merge-tree/src/test/wordUnitTests.ts b/packages/dds/merge-tree/src/test/wordUnitTests.ts index ef38a1d62d74..91ccffb2ba97 100644 --- a/packages/dds/merge-tree/src/test/wordUnitTests.ts +++ b/packages/dds/merge-tree/src/test/wordUnitTests.ts @@ -133,7 +133,7 @@ function makeBookmarks(client: TestClient, bookmarkCount: number) { function measureFetch(startFile: string, withBookmarks = false) { const bookmarkCount = 20000; - const client = new TestClient({ blockUpdateMarkers: true }); + const client = new TestClient(); loadTextFromFileWithMarkers(startFile, client.mergeTree); if (withBookmarks) { makeBookmarks(client, bookmarkCount); diff --git a/packages/dds/sequence/src/mapKernel.ts b/packages/dds/sequence/src/mapKernel.ts index dec4e2d928b3..20b9b98a579c 100644 --- a/packages/dds/sequence/src/mapKernel.ts +++ b/packages/dds/sequence/src/mapKernel.ts @@ -40,7 +40,7 @@ interface IMapMessageHandler { op: IMapOperation, local: boolean, message: ISequencedDocumentMessage | undefined, - localOpMetadata: unknown, + localOpMetadata: IMapMessageLocalMetadata, ): void; /** @@ -48,11 +48,17 @@ interface IMapMessageHandler { * @param op - The map operation to submit * @param localOpMetadata - The metadata to be submitted with the message. */ - submit(op: IMapOperation, localOpMetadata: unknown): void; + submit(op: IMapOperation): void; getStashedOpLocalMetadata(op: IMapOperation): unknown; } +interface IMapMessageLocalMetadata{ + pendingClearMessageId?: number, + pendingMessageId?: number, + lastProcessedSeq: number +} + /** * Describes an operation specific to a value type. */ @@ -183,6 +189,8 @@ export class MapKernel implements IValueTypeCreator { */ private readonly localValueMaker: LocalValueMaker; + private lastProcessedSeq: number = -1; + /** * Create a new shared map kernel. * @param serializer - The serializer to serialize / parse handles @@ -195,7 +203,7 @@ export class MapKernel implements IValueTypeCreator { constructor( private readonly serializer: IFluidSerializer, private readonly handle: IFluidHandle, - private readonly submitMessage: (op: any, localOpMetadata: unknown) => void, + private readonly submitMessage: (op: any, localOpMetadata: IMapMessageLocalMetadata) => void, private readonly isAttached: () => boolean, valueTypes: Readonly[]>, public readonly eventEmitter = new TypedEventEmitter(), @@ -499,8 +507,14 @@ export class MapKernel implements IValueTypeCreator { public trySubmitMessage(op: any, localOpMetadata: unknown): boolean { const type: string = op.type; if (this.messageHandlers.has(type)) { + const mapLocalMetadata: Partial = localOpMetadata; + // we don't know how to rebase these operations, so if any other op has come in + // we will fail. + if(this.lastProcessedSeq !== mapLocalMetadata?.lastProcessedSeq) { + throw new Error("SharedInterval does not support reconnect in presence of external changes"); + } // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.messageHandlers.get(type)!.submit(op as IMapOperation, localOpMetadata); + this.messageHandlers.get(type)!.submit(op as IMapOperation); return true; } return false; @@ -529,11 +543,14 @@ export class MapKernel implements IValueTypeCreator { message: ISequencedDocumentMessage | undefined, localOpMetadata: unknown, ): boolean { + // track the seq of every incoming message, so we can detect if any + // changes happened during a resubmit + this.lastProcessedSeq = message.sequenceNumber; if (this.messageHandlers.has(op.type)) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion this.messageHandlers .get(op.type)! - .process(op, local, message, localOpMetadata); + .process(op, local, message, localOpMetadata as IMapMessageLocalMetadata); return true; } return false; @@ -631,11 +648,12 @@ export class MapKernel implements IValueTypeCreator { private needProcessKeyOperation( op: IMapKeyOperation, local: boolean, - localOpMetadata: unknown, + localOpMetadata: IMapMessageLocalMetadata, ): boolean { if (this.pendingClearMessageId !== -1) { if (local) { - assert(localOpMetadata !== undefined && localOpMetadata as number < this.pendingClearMessageId, + assert(localOpMetadata?.pendingClearMessageId !== undefined + && localOpMetadata.pendingClearMessageId < this.pendingClearMessageId, 0x1f1 /* "Received out of order op when there is an unacked clear message" */); } // If we have an unacked clear, we can ignore all ops. @@ -648,7 +666,7 @@ export class MapKernel implements IValueTypeCreator { if (local) { assert(localOpMetadata !== undefined, 0x1f2 /* `pendingMessageId is missing from the local client's ${op.type} operation` */); - const pendingMessageId = localOpMetadata as number; + const pendingMessageId = localOpMetadata.pendingMessageId; const pendingKeyMessageId = this.pendingKeys.get(op.key); if (pendingKeyMessageId === pendingMessageId) { this.pendingKeys.delete(op.key); @@ -674,7 +692,7 @@ export class MapKernel implements IValueTypeCreator { if (local) { assert(localOpMetadata !== undefined, 0x1f3 /* "pendingMessageId is missing from the local client's clear operation" */); - const pendingMessageId = localOpMetadata as number; + const pendingMessageId = localOpMetadata?.pendingMessageId; if (this.pendingClearMessageId === pendingMessageId) { this.pendingClearMessageId = -1; } @@ -686,7 +704,7 @@ export class MapKernel implements IValueTypeCreator { } this.clearCore(local, message); }, - submit: (op: IMapClearOperation, localOpMetadata: unknown) => { + submit: (op: IMapClearOperation) => { // We don't reuse the metadata but send a new one on each submit. this.submitMapClearMessage(op); }, @@ -704,7 +722,7 @@ export class MapKernel implements IValueTypeCreator { } this.deleteCore(op.key, local, message); }, - submit: (op: IMapDeleteOperation, localOpMetadata: unknown) => { + submit: (op: IMapDeleteOperation) => { // We don't reuse the metadata but send a new one on each submit. this.submitMapKeyMessage(op); }, @@ -725,7 +743,7 @@ export class MapKernel implements IValueTypeCreator { const context = this.makeLocal(op.key, op.value); this.setCore(op.key, context, local, message); }, - submit: (op: IMapSetOperation, localOpMetadata: unknown) => { + submit: (op: IMapSetOperation) => { // We don't reuse the metadata but send a new one on each submit. this.submitMapKeyMessage(op); }, @@ -758,8 +776,8 @@ export class MapKernel implements IValueTypeCreator { const event: IValueChanged = { key: op.key, previousValue }; this.eventEmitter.emit("valueChanged", event, local, message, this.eventEmitter); }, - submit: (op: IMapValueTypeOperation, localOpMetadata: unknown) => { - this.submitMessage(op, localOpMetadata); + submit: (op: IMapValueTypeOperation) => { + this.submitMessage(op, {lastProcessedSeq: this.lastProcessedSeq}); }, getStashedOpLocalMetadata: (op: IMapValueTypeOperation) => { assert(false, 0x016 /* "apply stashed op not implemented for custom value type ops" */); @@ -780,8 +798,8 @@ export class MapKernel implements IValueTypeCreator { * @param op - The clear message */ private submitMapClearMessage(op: IMapClearOperation): void { - const pendingMessageId = this.getMapClearMessageLocalMetadata(op); - this.submitMessage(op, pendingMessageId); + const pendingClearMessageId = this.getMapClearMessageLocalMetadata(op); + this.submitMessage(op, {pendingClearMessageId, lastProcessedSeq: this.lastProcessedSeq}); } private getMapKeyMessageLocalMetadata(op: IMapKeyOperation): number { @@ -796,7 +814,7 @@ export class MapKernel implements IValueTypeCreator { */ private submitMapKeyMessage(op: IMapKeyOperation): void { const pendingMessageId = this.getMapKeyMessageLocalMetadata(op); - this.submitMessage(op, pendingMessageId); + this.submitMessage(op, {pendingMessageId, lastProcessedSeq: this.lastProcessedSeq}); } /** @@ -821,7 +839,7 @@ export class MapKernel implements IValueTypeCreator { }, }; // Send the localOpMetadata as undefined because we don't care about the ack. - this.submitMessage(op, undefined /* localOpMetadata */); + this.submitMessage(op, {lastProcessedSeq: this.lastProcessedSeq}); const event: IValueChanged = { key, previousValue }; this.eventEmitter.emit("valueChanged", event, true, null, this.eventEmitter); diff --git a/packages/dds/sequence/src/test/testFarm.ts b/packages/dds/sequence/src/test/testFarm.ts index bf28925eb736..6f192c4b8db3 100644 --- a/packages/dds/sequence/src/test/testFarm.ts +++ b/packages/dds/sequence/src/test/testFarm.ts @@ -244,11 +244,7 @@ export function TestPack(verbose = true) { let annotateProps: PropertySet; const insertAsRefPos = false; - let options = {}; - if (measureBookmarks) { - options = { blockUpdateMarkers: true }; - } - const testServer = new TestServer(options); + const testServer = new TestServer({}); testServer.measureOps = true; if (startFile) { loadTextFromFile(startFile, testServer.mergeTree, fileSegCount); @@ -1463,7 +1459,7 @@ export class DocumentTree { } private generateClient() { - const client = new TestClient({ blockUpdateMarkers: true }); + const client = new TestClient(); client.startOrUpdateCollaboration("Fred"); for (const child of this.children) { this.addToMergeTree(client, child); diff --git a/packages/framework/fluid-static/package.json b/packages/framework/fluid-static/package.json index d66a1e106156..639a25f5cdbc 100644 --- a/packages/framework/fluid-static/package.json +++ b/packages/framework/fluid-static/package.json @@ -73,6 +73,11 @@ }, "typeValidation": { "version": "0.59.1000", - "broken": {} + "broken": { + "0.58.2002": { + "ClassDeclaration_FluidContainer": {"forwardCompat": false}, + "InterfaceDeclaration_IFluidContainer": {"forwardCompat": false} + } + } } -} \ No newline at end of file +} diff --git a/packages/framework/fluid-static/src/fluidContainer.ts b/packages/framework/fluid-static/src/fluidContainer.ts index 2d115acb7816..71d83f77f5d9 100644 --- a/packages/framework/fluid-static/src/fluidContainer.ts +++ b/packages/framework/fluid-static/src/fluidContainer.ts @@ -5,7 +5,7 @@ import { TypedEventEmitter } from "@fluidframework/common-utils"; import { IFluidLoadable } from "@fluidframework/core-interfaces"; import { IEvent, IEventProvider } from "@fluidframework/common-definitions"; -import { AttachState, IContainer } from "@fluidframework/container-definitions"; +import { AttachState, IContainer, ConnectionState } from "@fluidframework/container-definitions"; import { LoadableObjectClass, LoadableObjectRecord } from "./types"; import { RootDataObject } from "./rootDataObject"; @@ -74,9 +74,17 @@ export interface IFluidContainerEvents extends IEvent { export interface IFluidContainer extends IEventProvider { /** * Whether the container is connected to the collaboration session. + * @deprecated - 0.58, This API will be removed in 1.0 + * Check `connectionState === ConnectionState.Connected` instead + * See https://github.com/microsoft/FluidFramework/issues/9167 for context */ readonly connected: boolean; + /** + * Provides the current connected state of the container + */ + readonly connectionState: ConnectionState; + /** * A container is considered **dirty** if it has local changes that have not yet been acknowledged by the service. * You should always check the `isDirty` flag before closing the container or navigating away from the page. @@ -184,6 +192,13 @@ export class FluidContainer extends TypedEventEmitter imp return this.container.connected; } + /** + * {@inheritDoc IFluidContainer.connectionState} + */ + public get connectionState(): ConnectionState { + return this.container.connectionState; + } + /** * {@inheritDoc IFluidContainer.initialObjects} */ diff --git a/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.ts b/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.ts index e972c2c29b15..f4e72516c033 100644 --- a/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.ts +++ b/packages/framework/fluid-static/src/test/types/validateFluidStaticPrevious.ts @@ -96,6 +96,7 @@ declare function get_old_ClassDeclaration_FluidContainer(): declare function use_current_ClassDeclaration_FluidContainer( use: TypeOnly); use_current_ClassDeclaration_FluidContainer( + // @ts-expect-error compatibility expected to be broken get_old_ClassDeclaration_FluidContainer()); /* @@ -144,6 +145,7 @@ declare function get_old_InterfaceDeclaration_IFluidContainer(): declare function use_current_InterfaceDeclaration_IFluidContainer( use: TypeOnly); use_current_InterfaceDeclaration_IFluidContainer( + // @ts-expect-error compatibility expected to be broken get_old_InterfaceDeclaration_IFluidContainer()); /* diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 38042569cc47..aa86ee8f32fd 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -397,7 +397,6 @@ export class Container extends EventEmitterWithErrorHandling i private resumedOpProcessingAfterLoad = false; private firstConnection = true; - private manualReconnectInProgress = false; private readonly connectionTransitionTimes: number[] = []; private messageCountAfterDisconnection: number = 0; private _loadedFromVersion: IVersion | undefined; @@ -909,11 +908,31 @@ export class Container extends EventEmitterWithErrorHandling i ); } + /** + * Dictates whether or not the current container will automatically attempt to reconnect to the delta stream + * after receiving a disconnect event + * @param reconnect - Boolean indicating if reconnect should automatically occur + * @deprecated - 0.58, This API will be removed in 1.0 + * Use `connect()` and `disconnect()` instead of `setAutoReconnect(true)` and `setAutoReconnect(false)` respectively + * See https://github.com/microsoft/FluidFramework/issues/9167 for context + */ public setAutoReconnect(reconnect: boolean) { if (this.closed) { throw new Error("Attempting to setAutoReconnect() a closed Container"); } + const mode = reconnect ? ReconnectMode.Enabled : ReconnectMode.Disabled; + this.setAutoReconnectInternal(mode); + + // If container state is not attached and resumed, then don't connect to delta stream. Also don't set the + // manual reconnection flag to true as we haven't made the initial connection yet. + if (reconnect && this._attachState === AttachState.Attached && this.resumedOpProcessingAfterLoad) { + // Ensure connection to web socket + this.connectToDeltaStream({ reason: "autoReconnect" }); + } + } + + private setAutoReconnectInternal(mode: ReconnectMode) { const currentMode = this._deltaManager.connectionManager.reconnectMode; if (currentMode === mode) { @@ -925,27 +944,65 @@ export class Container extends EventEmitterWithErrorHandling i this.setAutoReconnectTime = now; this.mc.logger.sendTelemetryEvent({ - eventName: reconnect ? "AutoReconnectEnabled" : "AutoReconnectDisabled", + eventName: mode === ReconnectMode.Enabled ? "AutoReconnectEnabled" : "AutoReconnectDisabled", connectionMode: this.connectionMode, connectionState: ConnectionState[this.connectionState], duration, }); this._deltaManager.connectionManager.setAutoReconnect(mode); + } - // If container state is not attached and resumed, then don't connect to delta stream. Also don't set the - // manual reconnection flag to true as we haven't made the initial connection yet. - if (reconnect && this._attachState === AttachState.Attached && this.resumedOpProcessingAfterLoad) { - if (this.connectionState === ConnectionState.Disconnected) { - // Only track this as a manual reconnection if we are truly the ones kicking it off. - this.manualReconnectInProgress = true; - } + public connect() { + if (this.closed) { + throw new UsageError(`The Container is closed and cannot be connected`); + } + else if (this._attachState !== AttachState.Attached) { + throw new UsageError(`The Container is not attached and cannot be connected`); + } + else if (!this.connected) { + // Note: no need to fetch ops as we do it preemptively as part of DeltaManager.attachOpHandler(). + // If there is gap, we will learn about it once connected, but the gap should be small (if any), + // assuming that connect() is called quickly after initial container boot. + this.connectInternal({ reason: "DocumentConnect", fetchOpsFromStorage: false }); + } + } - // Ensure connection to web socket - this.connectToDeltaStream({ reason: "autoReconnect" }); + private connectInternal(args: IConnectionArgs) { + assert(!this.closed, "Attempting to connect() a closed Container"); + assert(this._attachState === AttachState.Attached, "Attempting to connect() a container that is not attached"); + + // Resume processing ops and connect to delta stream + this.resumeInternal(args); + + // Set Auto Reconnect Mode + const mode = ReconnectMode.Enabled; + this.setAutoReconnectInternal(mode); + } + + public disconnect() { + if (this.closed) { + throw new UsageError(`The Container is closed and cannot be disconnected`); + } + else { + this.disconnectInternal(); } } + private disconnectInternal() { + assert(!this.closed, "Attempting to disconnect() a closed Container"); + + // Set Auto Reconnect Mode + const mode = ReconnectMode.Disabled; + this.setAutoReconnectInternal(mode); + } + + /** + * Have the container attempt to resume processing ops + * @deprecated - 0.58, This API will be removed in 1.0 + * Use `connect()` instead + * See https://github.com/microsoft/FluidFramework/issues/9167 for context + */ public resume() { if (!this.closed) { // Note: no need to fetch ops as we do it preemptively as part of DeltaManager.attachOpHandler(). @@ -1476,7 +1533,6 @@ export class Container extends EventEmitterWithErrorHandling i }); deltaManager.on("disconnect", (reason: string) => { - this.manualReconnectInProgress = false; this.collabWindowTracker.stopSequenceNumberUpdate(); this.connectionStateHandler.receivedDisconnectEvent(reason); }); @@ -1549,8 +1605,6 @@ export class Container extends EventEmitterWithErrorHandling i } if (this.firstConnection) { connectionInitiationReason = "InitialConnect"; - } else if (this.manualReconnectInProgress) { - connectionInitiationReason = "ManualReconnect"; } else { connectionInitiationReason = "AutoReconnect"; } @@ -1575,7 +1629,6 @@ export class Container extends EventEmitterWithErrorHandling i if (value === ConnectionState.Connected) { this.firstConnection = false; - this.manualReconnectInProgress = false; } } diff --git a/packages/loader/driver-utils/package.json b/packages/loader/driver-utils/package.json index 6ab6f7cec3f9..c9613322a8d9 100644 --- a/packages/loader/driver-utils/package.json +++ b/packages/loader/driver-utils/package.json @@ -64,7 +64,6 @@ "@fluidframework/core-interfaces": "^0.43.1000-0", "@fluidframework/driver-definitions": "^0.46.1000-0", "@fluidframework/gitresources": "^0.1036.1000-0", - "@fluidframework/odsp-driver-definitions": "^0.59.1000", "@fluidframework/protocol-base": "^0.1036.1000-0", "@fluidframework/protocol-definitions": "^0.1028.1000-0", "@fluidframework/telemetry-utils": "^0.59.1000", @@ -103,4 +102,4 @@ "version": "0.59.1000", "broken": {} } -} \ No newline at end of file +} diff --git a/packages/loader/driver-utils/src/runWithRetry.ts b/packages/loader/driver-utils/src/runWithRetry.ts index 983a25d7792a..f4cf2ffdd2c7 100644 --- a/packages/loader/driver-utils/src/runWithRetry.ts +++ b/packages/loader/driver-utils/src/runWithRetry.ts @@ -5,7 +5,7 @@ import { ITelemetryLogger } from "@fluidframework/common-definitions"; import { delay, performance } from "@fluidframework/common-utils"; -import { OdspErrorType } from "@fluidframework/odsp-driver-definitions"; +import { DriverErrorType } from "@fluidframework/driver-definitions"; import { canRetryOnError, getRetryDelayFromError } from "./network"; import { pkgVersion } from "./packageVersion"; import { NonRetryableError } from "."; @@ -61,15 +61,22 @@ export async function runWithRetry( eventName: `${fetchCallName}_cancel`, retry: numRetries, duration: performance.now() - startTime, + fetchCallName, }, err); throw err; } if (progress.cancel?.aborted === true) { + logger.sendTelemetryEvent({ + eventName: `${fetchCallName}_runWithRetryAborted`, + retry: numRetries, + duration: performance.now() - startTime, + fetchCallName, + }, err); throw new NonRetryableError( - "runWithRetryAborted", - OdspErrorType.fetchTimeout, - { eventName: `runWithRetryAborted_${fetchCallName}`, driverVersion: pkgVersion }, + "runWithRetry was Aborted", + DriverErrorType.genericError, + { driverVersion: pkgVersion, fetchCallName }, ); } @@ -89,6 +96,7 @@ export async function runWithRetry( eventName: `${fetchCallName}_lastError`, retry: numRetries, duration: performance.now() - startTime, + fetchCallName, }, lastError); } diff --git a/packages/runtime/container-runtime/src/test/summaryManager.spec.ts b/packages/runtime/container-runtime/src/test/summaryManager.spec.ts index a54b8b9f9bd3..b2f484fe5fa8 100644 --- a/packages/runtime/container-runtime/src/test/summaryManager.spec.ts +++ b/packages/runtime/container-runtime/src/test/summaryManager.spec.ts @@ -7,7 +7,9 @@ import { strict as assert } from "assert"; import sinon from "sinon"; import { Deferred, TypedEventEmitter } from "@fluidframework/common-utils"; import { IFluidHandle, IFluidLoadable } from "@fluidframework/core-interfaces"; +import { MessageType } from "@fluidframework/protocol-definitions"; import { MockLogger } from "@fluidframework/telemetry-utils"; +import { MockDeltaManager } from "@fluidframework/test-runtime-utils"; import { IConnectedEvents, IConnectedState, @@ -22,6 +24,10 @@ import { SummarizerStopReason, } from "../summarizerTypes"; import { ISummarizerClientElection, ISummarizerClientElectionEvents } from "../summarizerClientElection"; +import { RunningSummarizer } from "../runningSummarizer"; +import { SummarizeHeuristicData } from "../summarizerHeuristics"; +import { SummaryCollection, ISummaryOpMessage } from "../summaryCollection"; +import { neverCancelledSummaryToken } from "../runWhileConnectedCoordinator"; describe("Summary Manager", () => { let clock: sinon.SinonFakeTimers; @@ -30,18 +36,14 @@ describe("Summary Manager", () => { const flushPromises = async () => new Promise((resolve) => process.nextTick(resolve)); const thisClientId = "this"; const mockLogger = new MockLogger(); + const mockDeltaManager = new MockDeltaManager(); let summaryManager: SummaryManager; + let runningSummarizer: RunningSummarizer; + // let runCount: number; + const summarizerClientId = "test"; // Fake objects - let fakeOpListener; - const summaryCollection = { - opsSinceLastAck: 0, - addOpListener: (listener) => { fakeOpListener = listener; }, - removeOpListener: (listener) => { - assert.strictEqual(fakeOpListener, listener, "Re-init of fakeOpListener?"); - fakeOpListener = undefined; - }, - }; + const summaryCollection = new SummaryCollection(mockDeltaManager, mockLogger); const throttler = { delayMs: 0, numAttempts: 0, @@ -51,6 +53,23 @@ describe("Summary Manager", () => { delayFn: () => 0, }; + const summaryOp: ISummaryOpMessage = { + clientId: "clientId", + clientSequenceNumber: 5, + minimumSequenceNumber: 5, + referenceSequenceNumber: 5, + sequenceNumber: 6, + term: 0, + timestamp: 6, + type: MessageType.Summarize, + contents: { + handle: "OpHandle", + head: "head", + message: "message", + parents: ["parents"], + }, + }; + class TestConnectedState extends TypedEventEmitter implements IConnectedState { public connected = false; public clientId: string | undefined; @@ -90,10 +109,36 @@ describe("Summary Manager", () => { public async run(onBehalfOf: string): Promise { this.onBehalfOf = onBehalfOf; this.state = "running"; + runningSummarizer = await RunningSummarizer.start( + mockLogger, + summaryCollection.createWatcher(summarizerClientId), + { + idleTime: 5000, // 5 sec (idle) + maxTime: 5000 * 12, // 1 min (active) + maxOps: 1000, // 1k ops (active) + maxAckWaitTime: 120000, // 2 min + }, + // submitSummaryCallback + async (options) => { + return { + stage: "base", + minimumSequenceNumber: 0, + referenceSequenceNumber: 0, + error: undefined, + } as const; + }, + new SummarizeHeuristicData(0, { refSequenceNumber: 0, summaryTime: Date.now() }), + () => { }, + summaryCollection, + neverCancelledSummaryToken, + // stopSummarizerCallback + (reason) => { }, + ); await Promise.all([ this.stopDeferred.promise, this.runDeferred.promise, ]); + await runningSummarizer.waitStop(true); this.state = "stopped"; return "summarizerClientDisconnected"; } @@ -175,7 +220,7 @@ describe("Summary Manager", () => { summarizer.removeAllListeners(); connectedState.removeAllListeners(); throttler.delayMs = 0; - summaryCollection.opsSinceLastAck = 0; + mockDeltaManager.lastSequenceNumber = 0; requestCalls = 0; clock.reset(); }); @@ -252,7 +297,7 @@ describe("Summary Manager", () => { describe("Start Summarizer Delay", () => { it("Should wait for initial delay before first start", async () => { - summaryCollection.opsSinceLastAck = 999; // 999 < 1000, so do not bypass + mockDeltaManager.lastSequenceNumber = 999; // 999 < 1000, so do not bypass createSummaryManager({ initialDelayMs: 2000, opsToBypassInitialDelay: 1000, @@ -273,7 +318,7 @@ describe("Summary Manager", () => { }); it("Should bypass initial delay if enough ops have already passed", async () => { - summaryCollection.opsSinceLastAck = 1000; // 1000 >= 1000, so bypass + mockDeltaManager.lastSequenceNumber = 1000; // seq >= opsToBypass, so bypass createSummaryManager({ initialDelayMs: 2000, opsToBypassInitialDelay: 1000, @@ -298,7 +343,7 @@ describe("Summary Manager", () => { // make it work in main scenario, not some corner case that does not matter. // Issue #7273 tracks making appropriate product and test change and re-enable the test. it("Should bypass initial delay if enough ops pass later", async () => { - summaryCollection.opsSinceLastAck = 500; // 500 < 1000, so do not bypass yet + mockDeltaManager.lastSequenceNumber = 500; createSummaryManager({ initialDelayMs: 2000, opsToBypassInitialDelay: 1000, @@ -310,13 +355,13 @@ describe("Summary Manager", () => { clock.tick(1999); await flushPromises(); assertRequests(0, "should not have requested summarizer yet"); - summaryCollection.opsSinceLastAck = 999; // 999 < 1000, still do not bypass - fakeOpListener(); // Fire a fake "op" event + mockDeltaManager.lastSequenceNumber = 999; // seq < opsToBypass. No bypass. + mockDeltaManager.emit("op", summaryOp); clientElection.electClient(thisClientId); // force trigger refresh await flushPromises(); assertRequests(0, "still should not have requested summarizer yet"); - summaryCollection.opsSinceLastAck = 1000; // 1000 >= 1000, so should bypass now - fakeOpListener(); // Fire a fake "op" event + mockDeltaManager.lastSequenceNumber = 1000; // Bypass now + mockDeltaManager.emit("op", summaryOp); clientElection.electClient(thisClientId); // force trigger refresh await flushPromises(); assertRequests(1, "should request summarizer, bypassing initial delay"); @@ -325,6 +370,28 @@ describe("Summary Manager", () => { assertState(SummaryManagerState.Running, "summarizer should be running"); }); + it("Should create last summary when summarizer created without delay, then disconnected", async () => { + throttler.delayMs = 0; + createSummaryManager({ + opsToBypassInitialDelay: 0, + connected: false, + }); + clientElection.electClient(thisClientId); + await flushPromises(); + assertState(SummaryManagerState.Off, "not connected"); + mockDeltaManager.lastSequenceNumber = 10001; + connectedState.connect(); + await flushPromises(); + assertState(SummaryManagerState.Running, "Summarizer should be starting"); + assertRequests(1, "Should begin without delay"); + completeSummarizerRequest(); + await flushPromises(); + assertState(SummaryManagerState.Running, "Should be running"); + connectedState.disconnect(); + await flushPromises(); + assertState(SummaryManagerState.Stopping, "Should be stopping"); + }); + it("Should wait for throttler delay before starting summarizer", async () => { throttler.delayMs = 100; createSummaryManager({ diff --git a/server/gitrest/Dockerfile b/server/gitrest/Dockerfile index 68af061a1a9b..ac37794c5e7c 100644 --- a/server/gitrest/Dockerfile +++ b/server/gitrest/Dockerfile @@ -1,7 +1,7 @@ # Copyright (c) Microsoft Corporation and contributors. All rights reserved. # Licensed under the MIT License. -FROM node:14.18.3-stretch AS base +FROM node:14.19.1-stretch AS base # Add Tini ENV TINI_VERSION v0.18.0 diff --git a/server/historian/Dockerfile b/server/historian/Dockerfile index 247e9ba48185..df87264cb130 100644 --- a/server/historian/Dockerfile +++ b/server/historian/Dockerfile @@ -1,7 +1,7 @@ # Copyright (c) Microsoft Corporation and contributors. All rights reserved. # Licensed under the MIT License. -FROM node:14.18.3-stretch-slim AS base +FROM node:14.19.1-stretch-slim AS base # node-gyp dependencies RUN apt-get update && apt-get install -y \ diff --git a/server/routerlicious/Dockerfile b/server/routerlicious/Dockerfile index 0de0caa16f18..f3732616f331 100644 --- a/server/routerlicious/Dockerfile +++ b/server/routerlicious/Dockerfile @@ -1,7 +1,7 @@ # Copyright (c) Microsoft Corporation and contributors. All rights reserved. # Licensed under the MIT License. -FROM node:14.18.3-stretch-slim AS base +FROM node:14.19.1-stretch-slim AS base # node-gyp dependencies RUN apt-get update && apt-get install -y \ diff --git a/server/tinylicious/.devcontainer/Dockerfile b/server/tinylicious/.devcontainer/Dockerfile index b60ddbd78044..2d004f0394ed 100644 --- a/server/tinylicious/.devcontainer/Dockerfile +++ b/server/tinylicious/.devcontainer/Dockerfile @@ -1,7 +1,7 @@ # Copyright (c) Microsoft Corporation and contributors. All rights reserved. # Licensed under the MIT License. -FROM node:14.18.3 +FROM node:14.19.1 # Avoid warnings by switching to noninteractive ENV DEBIAN_FRONTEND=noninteractive