diff --git a/experimental/dds/tree2/src/feature-libraries/sequence-field/compose.ts b/experimental/dds/tree2/src/feature-libraries/sequence-field/compose.ts index 47ef12631260..b94d0cbd0343 100644 --- a/experimental/dds/tree2/src/feature-libraries/sequence-field/compose.ts +++ b/experimental/dds/tree2/src/feature-libraries/sequence-field/compose.ts @@ -26,6 +26,7 @@ import { MoveEffect, isMoveIn, isMoveOut, + getMoveIn, } from "./moveEffectTable"; import { getInputLength, @@ -170,11 +171,24 @@ function composeMarks( moveEffects: MoveEffectTable, revisionMetadata: RevisionMetadataSource, ): Mark { - const nodeChange = composeChildChanges( + let nodeChange = composeChildChanges( baseMark.changes, newMark.changes === undefined ? undefined : tagChange(newMark.changes, newRev), composeChild, ); + if (nodeChange !== undefined) { + const baseSource = getMoveIn(baseMark); + if (baseSource !== undefined) { + setModifyAfter( + moveEffects, + getEndpoint(baseSource, undefined), + nodeChange, + newRev, + composeChild, + ); + nodeChange = undefined; + } + } if (isImpactfulCellRename(newMark, newRev, revisionMetadata)) { const newAttachAndDetach = asAttachAndDetach(newMark); const newDetachRevision = newAttachAndDetach.detach.revision ?? newRev; @@ -295,37 +309,14 @@ function composeMarks( } else if (!markHasCellEffect(baseMark)) { return withRevision(withNodeChange(newMark, nodeChange), newRev); } else if (!markHasCellEffect(newMark)) { - if (isMoveIn(baseMark) && nodeChange !== undefined) { - setModifyAfter( - moveEffects, - getEndpoint(baseMark, undefined), - nodeChange, - newRev, - composeChild, - ); - return baseMark; - } return withNodeChange(baseMark, nodeChange); } else if (areInputCellsEmpty(baseMark)) { assert(isDetach(newMark), 0x71c /* Unexpected mark type */); assert(isAttach(baseMark), 0x71d /* Expected generative mark */); - let localNodeChange = nodeChange; const attach = extractMarkEffect(baseMark); const detach = extractMarkEffect(withRevision(newMark, newRev)); - if (isMoveIn(attach) && nodeChange !== undefined) { - setModifyAfter( - moveEffects, - getEndpoint(attach, undefined), - nodeChange, - newRev, - composeChild, - ); - - localNodeChange = undefined; - } - if (isMoveIn(attach) && isMoveOut(detach)) { const finalSource = getEndpoint(attach, undefined); const finalDest = getEndpoint(detach, newRev); @@ -354,10 +345,7 @@ function composeMarks( if (areEqualCellIds(getOutputCellId(newMark, newRev, revisionMetadata), baseMark.cellId)) { // The output and input cell IDs are the same, so this mark has no effect. - return withNodeChange( - { count: baseMark.count, cellId: baseMark.cellId }, - localNodeChange, - ); + return withNodeChange({ count: baseMark.count, cellId: baseMark.cellId }, nodeChange); } return normalizeCellRename( { @@ -367,7 +355,7 @@ function composeMarks( attach, detach, }, - localNodeChange, + nodeChange, ); } else { if (isMoveMark(baseMark) && isMoveMark(newMark)) { diff --git a/experimental/dds/tree2/src/feature-libraries/sequence-field/moveEffectTable.ts b/experimental/dds/tree2/src/feature-libraries/sequence-field/moveEffectTable.ts index 60126b444373..e833f114801a 100644 --- a/experimental/dds/tree2/src/feature-libraries/sequence-field/moveEffectTable.ts +++ b/experimental/dds/tree2/src/feature-libraries/sequence-field/moveEffectTable.ts @@ -97,6 +97,17 @@ export function isMoveIn(effect: MarkEffect): effect is MoveIn { return effect.type === "MoveIn"; } +export function getMoveIn(effect: MarkEffect): MoveIn | undefined { + switch (effect.type) { + case "MoveIn": + return effect; + case "AttachAndDetach": + return getMoveIn(effect.attach); + default: + return undefined; + } +} + function adjustMoveEffectBasis(effect: MoveEffectWithBasis, newBasis: MoveId): MoveEffect { if (effect.basis === newBasis) { return effect; diff --git a/experimental/dds/tree2/src/test/feature-libraries/sequence-field/compose.spec.ts b/experimental/dds/tree2/src/test/feature-libraries/sequence-field/compose.spec.ts index 8367df666f34..02cf5509c605 100644 --- a/experimental/dds/tree2/src/test/feature-libraries/sequence-field/compose.spec.ts +++ b/experimental/dds/tree2/src/test/feature-libraries/sequence-field/compose.spec.ts @@ -1104,6 +1104,23 @@ describe("SequenceField - Compose", () => { assert.deepEqual(composed, expected); }); + it("move-in+delete ○ modify", () => { + const changes = TestChange.mint([], 42); + const [mo, mi] = Mark.move(1, { revision: tag1, localId: brand(1) }); + const attachDetach = Mark.attachAndDetach( + mi, + Mark.delete(1, { revision: tag2, localId: brand(2) }), + ); + const base = makeAnonChange([mo, attachDetach]); + const modify = tagChange( + [Mark.modify(changes, { revision: tag2, localId: brand(2) })], + tag3, + ); + const actual = shallowCompose([base, modify]); + const expected = [{ ...mo, changes }, attachDetach]; + assert.deepEqual(actual, expected); + }); + it("effect management for [move, modify, move]", () => { const changes = TestChange.mint([], 42); const [mo, mi] = Mark.move(1, brand(0)); diff --git a/experimental/dds/tree2/src/test/shared-tree/fuzz/topLevel.fuzz.spec.ts b/experimental/dds/tree2/src/test/shared-tree/fuzz/topLevel.fuzz.spec.ts index a371e034fa9d..c0ab1d6979d8 100644 --- a/experimental/dds/tree2/src/test/shared-tree/fuzz/topLevel.fuzz.spec.ts +++ b/experimental/dds/tree2/src/test/shared-tree/fuzz/topLevel.fuzz.spec.ts @@ -106,7 +106,6 @@ describe("Fuzz - Top-Level", () => { saveFailures: { directory: failureDirectory, }, - skip: [42], }; createDDSFuzzSuite(model, options); }); diff --git a/packages/loader/container-loader/src/connectionStateHandler.ts b/packages/loader/container-loader/src/connectionStateHandler.ts index fafc123e1b7d..575263ad4aaf 100644 --- a/packages/loader/container-loader/src/connectionStateHandler.ts +++ b/packages/loader/container-loader/src/connectionStateHandler.ts @@ -312,12 +312,25 @@ class ConnectionStateCatchup extends ConnectionStateHandlerPassThrough { class ConnectionStateHandler implements IConnectionStateHandler { private _connectionState = ConnectionState.Disconnected; private _pendingClientId: string | undefined; + + /** + * Tracks that we observe the "leave" op within the timeout for our previous clientId (see comment on ConnectionStateHandler class) + * ! This ensures we do not switch to a new clientId until we process all potential messages from old clientId + * ! i.e. We will always see the "leave" op for a client after we have seen all the ops it has sent + * ! This check helps prevent the same op from being resubmitted by the PendingStateManager upon reconnecting + */ private readonly prevClientLeftTimer: Timer; + + /** + * Tracks that we observe our own "join" op within the timeout after receiving a "connected" event from the DeltaManager + */ private readonly joinOpTimer: Timer; + private protocol?: IProtocolHandler; private connection?: IConnectionDetailsInternal; private _clientId?: string; + /** Track how long we waited to see "leave" op for previous clientId */ private waitEvent: PerformanceEvent | undefined; public get connectionState(): ConnectionState { @@ -453,9 +466,9 @@ class ConnectionStateHandler implements IConnectionStateHandler { 0x2e2 /* "Must only wait for leave message when clientId in quorum" */, ); - // Move to connected state only if we are in Connecting state, we have seen our join op - // and there is no timer running which means we are not waiting for previous client to leave - // or timeout has occurred while doing so. + // Move to connected state only if: + // 1. We have seen our own "join" op (i.e. for this.pendingClientId) + // 2. There is no "leave" timer running, meaning this is our first connection or the previous client has left (via this.prevClientLeftTimer) if ( this.pendingClientId !== this.clientId && this.hasMember(this.pendingClientId) && diff --git a/packages/runtime/container-runtime/src/containerRuntime.ts b/packages/runtime/container-runtime/src/containerRuntime.ts index 2e16a8b18d1a..0f9cb81fb74f 100644 --- a/packages/runtime/container-runtime/src/containerRuntime.ts +++ b/packages/runtime/container-runtime/src/containerRuntime.ts @@ -3667,6 +3667,7 @@ export class ContainerRuntime /** * Finds the right store and asks it to resubmit the message. This typically happens when we * reconnect and there are pending messages. + * ! Note: successfully resubmitting an op that has been successfully sequenced is not possible due to checks in the ConnectionStateHandler (Loader layer) * @param message - The original LocalContainerRuntimeMessage. * @param localOpMetadata - The local metadata associated with the original message. */ diff --git a/packages/runtime/container-runtime/src/dataStores.ts b/packages/runtime/container-runtime/src/dataStores.ts index 500137eb42fd..f6249a820db4 100644 --- a/packages/runtime/container-runtime/src/dataStores.ts +++ b/packages/runtime/container-runtime/src/dataStores.ts @@ -61,13 +61,8 @@ import { } from "./dataStoreContext"; import { StorageServiceWithAttachBlobs } from "./storageServiceWithAttachBlobs"; import { IDataStoreAliasMessage, isDataStoreAliasMessage } from "./dataStore"; -import { GCNodeType, disableDatastoreSweepKey, sendGCUnexpectedUsageEvent } from "./gc"; -import { - summarizerClientType, - IContainerRuntimeMetadata, - nonDataStorePaths, - rootHasIsolatedChannels, -} from "./summary"; +import { GCNodeType, disableDatastoreSweepKey } from "./gc"; +import { IContainerRuntimeMetadata, nonDataStorePaths, rootHasIsolatedChannels } from "./summary"; type PendingAliasResolve = (success: boolean) => void; @@ -281,6 +276,19 @@ export class DataStores implements IDisposable { } const context = this.contexts.get(aliasMessage.internalId); + // If the data store has been deleted, log an error and ignore this message. This helps prevent document + // corruption in case a deleted data store accidentally submitted a signal. + if ( + this.checkAndLogIfDeleted( + aliasMessage.internalId, + context, + "Changed", + "processAliasMessageCore", + ) + ) { + return false; + } + if (context === undefined) { this.mc.logger.sendErrorEvent({ eventName: "AliasFluidDataStoreNotFound", @@ -382,18 +390,43 @@ export class DataStores implements IDisposable { public resubmitDataStoreOp(envelope: IEnvelope, localOpMetadata: unknown) { const context = this.contexts.get(envelope.address); + // If the data store has been deleted, log an error and throw an error. If there are local changes for a + // deleted data store, it can otherwise lead to inconsistent state when compared to other clients. + if ( + this.checkAndLogIfDeleted(envelope.address, context, "Changed", "resubmitDataStoreOp") + ) { + throw new DataCorruptionError("Context is deleted!", { + callSite: "resubmitDataStoreOp", + ...tagCodeArtifacts({ id: envelope.address }), + }); + } assert(!!context, 0x160 /* "There should be a store context for the op" */); context.reSubmit(envelope.contents, localOpMetadata); } public rollbackDataStoreOp(envelope: IEnvelope, localOpMetadata: unknown) { const context = this.contexts.get(envelope.address); + // If the data store has been deleted, log an error and throw an error. If there are local changes for a + // deleted data store, it can otherwise lead to inconsistent state when compared to other clients. + if ( + this.checkAndLogIfDeleted(envelope.address, context, "Changed", "rollbackDataStoreOp") + ) { + throw new DataCorruptionError("Context is deleted!", { + callSite: "rollbackDataStoreOp", + ...tagCodeArtifacts({ id: envelope.address }), + }); + } assert(!!context, 0x2e8 /* "There should be a store context for the op" */); context.rollback(envelope.contents, localOpMetadata); } public async applyStashedOp(envelope: IEnvelope): Promise { const context = this.contexts.get(envelope.address); + // If the data store has been deleted, log an error and ignore this message. This helps prevent document + // corruption in case the data store that stashed the op is deleted. + if (this.checkAndLogIfDeleted(envelope.address, context, "Changed", "applyStashedOp")) { + return undefined; + } assert(!!context, 0x161 /* "There should be a store context for the op" */); return context.applyStashedOp(envelope.contents); } @@ -411,8 +444,21 @@ export class DataStores implements IDisposable { ) { const envelope = message.contents as IEnvelope; const transformed = { ...message, contents: envelope.contents }; - this.validateNotDeleted(envelope.address); const context = this.contexts.get(envelope.address); + + // If the data store has been deleted, log an error and ignore this message. This helps prevent document + // corruption in case a deleted data store accidentally submitted an op. + if ( + this.checkAndLogIfDeleted( + envelope.address, + context, + "Changed", + "processFluidDataStoreOp", + ) + ) { + return; + } + assert(!!context, 0x162 /* "There should be a store context for the op" */); context.process(transformed, local, localMessageMetadata); @@ -430,7 +476,22 @@ export class DataStores implements IDisposable { requestHeaderData: RuntimeHeaderData, ): Promise { const headerData = { ...defaultRuntimeHeaderData, ...requestHeaderData }; - this.validateNotDeleted(id, headerData); + if ( + this.checkAndLogIfDeleted( + id, + this.contexts.get(id), + "Requested", + "getDataStore", + requestHeaderData, + ) + ) { + // The requested data store has been deleted by gc. Create a 404 response exception. + const request: IRequest = { url: id }; + throw responseToException( + createResponseError(404, "DataStore was deleted", request), + request, + ); + } const context = await this.contexts.getBoundOrRemoted(id, headerData.wait); if (context === undefined) { @@ -448,8 +509,16 @@ export class DataStores implements IDisposable { id: string, requestHeaderData: RuntimeHeaderData, ): Promise { - // If the data store has been deleted, return undefined. - if (this.checkIfDeleted(id, requestHeaderData)) { + // If the data store has been deleted, log an error and return undefined. + if ( + this.checkAndLogIfDeleted( + id, + this.contexts.get(id), + "Requested", + "getDataStoreIfAvailable", + requestHeaderData, + ) + ) { return undefined; } const headerData = { ...defaultRuntimeHeaderData, ...requestHeaderData }; @@ -461,55 +530,43 @@ export class DataStores implements IDisposable { } /** - * Checks if the data store has been deleted by GC. - * @param id - data store id - * @param request - the request information to log if the validation detects the data store has been deleted - * @param requestHeaderData - the request header information to log if the validation detects the data store has been deleted + * Checks if the data store has been deleted by GC. If so, log an error. + * @param id - The data store's id. + * @param context - The data store context. + * @param callSite - The function name this is called from. + * @param requestHeaderData - The request header information to log if the data store is deleted. * @returns true if the data store is deleted. Otherwise, returns false. */ - private checkIfDeleted(id: string, requestHeaderData?: RuntimeHeaderData) { + private checkAndLogIfDeleted( + id: string, + context: FluidDataStoreContext | undefined, + deletedLogSuffix: string, + callSite: string, + requestHeaderData?: RuntimeHeaderData, + ) { const dataStoreNodePath = `/${id}`; if (!this.isDataStoreDeleted(dataStoreNodePath)) { return false; } - assert( - !this.contexts.has(id), - 0x570 /* Inconsistent state! GC says the data store is deleted, but the data store is not deleted from the runtime. */, - ); - sendGCUnexpectedUsageEvent( - this.mc, - { - eventName: "GC_Deleted_DataStore_Requested", - category: "error", - isSummarizerClient: this.runtime.clientDetails.type === summarizerClientType, - id, - headers: JSON.stringify(requestHeaderData), - gcTombstoneEnforcementAllowed: this.runtime.gcTombstoneEnforcementAllowed, - }, - undefined /* packagePath */, - ); - return true; - } - /** - * Validate that the data store had not been deleted by GC. - * @param id - data store id - * @param requestHeaderData - the request header information to log if the validation detects the data store has been deleted - */ - private validateNotDeleted(id: string, requestHeaderData?: RuntimeHeaderData) { - if (this.checkIfDeleted(id, requestHeaderData)) { - // The requested data store is removed by gc. Create a 404 gc response exception. - const request: IRequest = { url: id }; - throw responseToException( - createResponseError(404, "DataStore was deleted", request), - request, - ); - } + this.mc.logger.sendErrorEvent({ + eventName: `GC_Deleted_DataStore_${deletedLogSuffix}`, + ...tagCodeArtifacts({ id }), + callSite, + headers: JSON.stringify(requestHeaderData), + exists: context !== undefined, + }); + return true; } public processSignal(fluidDataStoreId: string, message: IInboundSignalMessage, local: boolean) { - this.validateNotDeleted(fluidDataStoreId); const context = this.contexts.get(fluidDataStoreId); + // If the data store has been deleted, log an error and ignore this message. This helps prevent document + // corruption in case a deleted data store accidentally submitted a signal. + if (this.checkAndLogIfDeleted(fluidDataStoreId, context, "Changed", "processSignal")) { + return; + } + if (!context) { // Attach message may not have been processed yet assert(!local, 0x163 /* "Missing datastore for local signal" */); diff --git a/packages/runtime/container-runtime/src/pendingStateManager.ts b/packages/runtime/container-runtime/src/pendingStateManager.ts index d4d3adcbff52..b92650497a80 100644 --- a/packages/runtime/container-runtime/src/pendingStateManager.ts +++ b/packages/runtime/container-runtime/src/pendingStateManager.ts @@ -331,6 +331,7 @@ export class PendingStateManager implements IDisposable { /** * Called when the Container's connection state changes. If the Container gets connected, it replays all the pending * states in its queue. This includes triggering resubmission of unacked ops. + * ! Note: successfully resubmitting an op that has been successfully sequenced is not possible due to checks in the ConnectionStateHandler (Loader layer) */ public replayPendingStates() { assert( diff --git a/packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts b/packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts index 7c6fcd7931ef..554763b7bac3 100644 --- a/packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts +++ b/packages/test/test-end-to-end-tests/src/test/gc/gcSweepDataStores.spec.ts @@ -20,7 +20,6 @@ import { waitForContainerConnection, mockConfigProvider, ITestContainerConfig, - timeoutPromise, } from "@fluidframework/test-utils"; import { describeCompat, @@ -169,28 +168,6 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) }; }; - /** - * Returns a promise that will resolve when all the passed in containers have closed dur - * to "DataStore was deleted" error. - */ - const getContainersClosePromise = async (containers: IContainer[]) => { - const closePromises: Promise[] = []; - for (const container of containers) { - const promise = timeoutPromise((resolve, reject) => { - const listener = (error: IErrorBase | undefined) => { - if (!error?.message.startsWith("DataStore was deleted:")) { - reject(error); - } - container.off("closed", listener); - resolve(); - }; - container.on("closed", listener); - }); - closePromises.push(promise); - } - return Promise.all(closePromises); - }; - describe("Using swept data stores not allowed", () => { // If this test starts failing due to runtime is closed errors try first adjusting `sweepTimeoutMs` above itExpects( @@ -203,6 +180,7 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) { eventName: "fluid:telemetry:FluidDataStoreContext:GC_Deleted_DataStore_Changed", clientType: "noninteractive/summarizer", + callSite: "submitMessage", }, ], async () => { @@ -238,6 +216,7 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) { eventName: "fluid:telemetry:FluidDataStoreContext:GC_Deleted_DataStore_Changed", clientType: "noninteractive/summarizer", + callSite: "submitSignal", }, ], async () => { @@ -262,10 +241,9 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) ); }); - describe("Loading swept data stores not allowed", () => { - // TODO: Receive ops scenarios - loaded before and loaded after (are these just context loading errors?) + describe("Using deleted data stores", () => { itExpects( - "Requesting swept datastores fails in client loaded after sweep timeout and summarizing container", + "Requesting swept datastores not allowed", [ { eventName: "fluid:telemetry:Summarizer:Running:SweepReadyObject_Loaded", @@ -274,11 +252,13 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) { eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", clientType: "interactive", + callSite: "getDataStore", }, - // Summarizer client's request + // Summarizer client's request logs an error { eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", clientType: "noninteractive/summarizer", + callSite: "getDataStore", }, ], async () => { @@ -336,35 +316,26 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) ); itExpects( - "Receiving ops for swept datastores fails in client after sweep timeout and summarizing container", + "Ops for swept data stores is ignored but logs an error", [ { eventName: "fluid:telemetry:Summarizer:Running:SweepReadyObject_Loaded", clientType: "noninteractive/summarizer", }, { - eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", - clientType: "noninteractive/summarizer", - }, - { - eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", - clientType: "interactive", - }, - { - eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", - clientType: "interactive", - }, - { - eventName: "fluid:telemetry:Container:ContainerClose", + eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Changed", clientType: "noninteractive/summarizer", + callSite: "processFluidDataStoreOp", }, { - eventName: "fluid:telemetry:Container:ContainerClose", + eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Changed", clientType: "interactive", + callSite: "processFluidDataStoreOp", }, { - eventName: "fluid:telemetry:Container:ContainerClose", + eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Changed", clientType: "interactive", + callSite: "processFluidDataStoreOp", }, ], async () => { @@ -391,12 +362,6 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) const { summaryVersion } = await summarize(summarizer); const receivingContainer = await loadContainer(summaryVersion); - const closePromise = getContainersClosePromise([ - sendingContainer, - summarizingContainer, - receivingContainer, - ]); - // Send an op to the swept data store dataObject._root.set("send", "op"); @@ -405,54 +370,44 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) // Wait for the GC and the above op to be processed which will close all the containers. await provider.ensureSynchronized(); - await closePromise; - // The containers should close + // The containers should not close assert( - sendingContainer.closed, - "Sending container with deleted datastore should close on receiving an op for it", + !sendingContainer.closed, + "Sending container should not close on receiving an op for deleted data store", ); assert( - summarizingContainer.closed, - "Summarizing container with deleted datastore should close on receiving an op for it", + !summarizingContainer.closed, + "Summarizing container should not close on receiving an op for deleted data store", ); assert( - receivingContainer.closed, - "Container with deleted datastore should close on receiving an op for it", + !receivingContainer.closed, + "Receiving container should close on receiving an op for deleted data store", ); }, ); itExpects( - "Receiving signals for swept datastores fails in client after sweep timeout and summarizing container", + "Signals for swept datastores are ignored but logs an error", [ { eventName: "fluid:telemetry:Summarizer:Running:SweepReadyObject_Loaded", clientType: "noninteractive/summarizer", }, { - eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", - clientType: "noninteractive/summarizer", - }, - { - eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", - clientType: "interactive", - }, - { - eventName: "fluid:telemetry:Container:ContainerClose", + eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Changed", clientType: "noninteractive/summarizer", + callSite: "processSignal", }, { - eventName: "fluid:telemetry:Container:ContainerClose", - clientType: "interactive", - }, - { - eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Requested", + eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Changed", clientType: "interactive", + callSite: "processSignal", }, { - eventName: "fluid:telemetry:Container:ContainerClose", + eventName: "fluid:telemetry:ContainerRuntime:GC_Deleted_DataStore_Changed", clientType: "interactive", + callSite: "processSignal", }, ], async () => { @@ -483,12 +438,6 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) const { summaryVersion } = await summarize(summarizer); const receivingContainer = await loadContainer(summaryVersion); - const closePromise = getContainersClosePromise([ - sendingContainer, - summarizingContainer, - receivingContainer, - ]); - // Send a signal to the swept data store dataObject._runtime.submitSignal("a", "signal"); @@ -499,21 +448,18 @@ describeCompat("GC data store sweep tests", "NoCompat", (getTestObjectProvider) // Once the GC op has been processed, resume the inbound signal queue so that the signal is processed. sendingContainer.deltaManager.inboundSignal.resume(); - // All the containers should have closed now. - await closePromise; - - // The containers should close + // The containers should not close assert( - sendingContainer.closed, - "Sending container with deleted datastore should close on receiving an op for it", + !sendingContainer.closed, + "Sending container should not close on receiving a signal for deleted data store", ); assert( - summarizingContainer.closed, - "Summarizing container with deleted datastore should close on receiving a signal for it", + !summarizingContainer.closed, + "Summarizing container should not close on receiving a signal for deleted data store", ); assert( - receivingContainer.closed, - "Container with deleted datastore should close on receiving a signal for it", + !receivingContainer.closed, + "Receiving container should not close on receiving a signal for deleted data store", ); }, ); diff --git a/server/routerlicious/packages/protocol-base/api-report/protocol-base.api.md b/server/routerlicious/packages/protocol-base/api-report/protocol-base.api.md index d14ae165b9b4..be8d8d0f4ca2 100644 --- a/server/routerlicious/packages/protocol-base/api-report/protocol-base.api.md +++ b/server/routerlicious/packages/protocol-base/api-report/protocol-base.api.md @@ -27,7 +27,7 @@ export function getGitMode(value: SummaryObject): string; // @internal export function getGitType(value: SummaryObject): "blob" | "tree"; -// @internal (undocumented) +// @alpha (undocumented) export interface IProtocolHandler { // (undocumented) readonly attributes: IDocumentAttributes; @@ -55,7 +55,7 @@ export interface IQuorumSnapshot { values: QuorumProposalsSnapshot["values"]; } -// @internal (undocumented) +// @alpha (undocumented) export interface IScribeProtocolState { // (undocumented) members: [string, ISequencedClient][]; diff --git a/server/routerlicious/packages/protocol-base/src/protocol.ts b/server/routerlicious/packages/protocol-base/src/protocol.ts index ef2d3b222a13..ff561977be88 100644 --- a/server/routerlicious/packages/protocol-base/src/protocol.ts +++ b/server/routerlicious/packages/protocol-base/src/protocol.ts @@ -19,7 +19,7 @@ import { import { IQuorumSnapshot, Quorum } from "./quorum"; /** - * @internal + * @alpha */ export interface IScribeProtocolState { sequenceNumber: number; @@ -30,7 +30,7 @@ export interface IScribeProtocolState { } /** - * @internal + * @alpha */ export interface IProtocolHandler { readonly quorum: IQuorum;