Add timeouts to summarizer stopping (microsoft#26271)

There are some peculiar cases of documents hitting 10k ops where a
summarizer doesn't shut down properly. The parent container continually
recognizes it is the elected client, but it still has a summarizer
hanging somewhere. Some timeouts were added in various places to ensure
the summarizer closes properly for the parent to spawn a new one.

Looking at telemetry, the P99.9 of a summarizer's first event to proper
connection was 35 seconds. Thus, a timeout of 2 minutes should be more
than enough.

One interesting potential case is the following:
1. Parent container spawns summarizer
2. Summarizer starts load flow
3. Parent container disconnects (`summarizer?.stop(...)` is not called
since load flow has not finished and returned the `summarizer` instance)
4. Summarizer finishes load flow, and we attempt to call
`summarizer.run(...)`
5. At the start of `summarizer.run(...)`, it waits to see a
`"connected"` event from the ContainerRuntime
i. This may not always happen if the summarizer gets into some weird
state and isn't able to get a connection from service, causing the whole
summarizer flow to hang


[AB#54693](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/54693)

Author: kian-thompson

packages/runtime/container-runtime/src/summary/summaryDelayLoadedModule/summarizer.ts

@@ -9,7 +9,7 @@ import type {
 	SummarizerStopReason,
 } from "@fluidframework/container-runtime-definitions/internal";
 import type { IFluidHandleContext } from "@fluidframework/core-interfaces/internal";
-import { Deferred } from "@fluidframework/core-utils/internal";
+import { assert, Deferred } from "@fluidframework/core-utils/internal";
 import {
 	type IFluidErrorBase,
 	type ITelemetryLoggerExt,
@@ -165,9 +165,37 @@ export class Summarizer extends TypedEventEmitter<ISummarizerEvents> implements
 	}
 
 	private async runCore(onBehalfOf: string): Promise<SummarizerStopReason> {
-		const runCoordinator: ICancellableSummarizerController = await this.runCoordinatorCreateFn(
-			this.runtime,
-		);
+		// Race coordinator creation against a timeout to ensure we don't hang indefinitely
+		// if the summarizer container fails to connect.
+		let coordinatorTimedOut = false;
+		const coordinatorTimeoutMs = 2 * 60 * 1000; // 2 minutes
+		const coordinatorResult = await Promise.race([
+			this.runCoordinatorCreateFn(this.runtime).then((coordinator) => {
+				if (coordinatorTimedOut) {
+					coordinator.stop("summarizerClientDisconnected");
+				}
+				return coordinator;
+			}),
+			new Promise<undefined>((resolve) =>
+				setTimeout(() => {
+					coordinatorTimedOut = true;
+					resolve(undefined);
+				}, coordinatorTimeoutMs),
+			),
+		]);
+
+		// If we timed out before coordinator was created, exit early
+		if (coordinatorTimedOut) {
+			this.logger.sendTelemetryEvent({
+				eventName: "CreateRunCoordinatorTimeout",
+				onBehalfOf,
+				timeoutMs: coordinatorTimeoutMs,
+			});
+			return "summarizerClientDisconnected";
+		}
+
+		assert(coordinatorResult !== undefined, "Expect coordinatorResult to be defined");
+		const runCoordinator = coordinatorResult;
 
 		// Wait for either external signal to cancel, or loss of connectivity.
 		const stopP = Promise.race([runCoordinator.waitCancelled, this.stopDeferred.promise]);
@@ -216,10 +244,22 @@ export class Summarizer extends TypedEventEmitter<ISummarizerEvents> implements
 		// summarizer client to not be created until current summarizer fully moves to exit, and that would reduce
 		// cons of #2 substantially.
 
-		// Cleanup after running
-		await runningSummarizer.waitStop(
+		// Cleanup after running with a timeout to prevent hanging
+		const waitStopPromise = runningSummarizer.waitStop(
 			!runCoordinator.cancelled && Summarizer.stopReasonCanRunLastSummary(stopReason),
 		);
+		const summarizerStopTimeoutMs = 2 * 60 * 1000; // 2 minutes
+		const timeoutPromise = new Promise<"timeout">((resolve) =>
+			setTimeout(() => resolve("timeout"), summarizerStopTimeoutMs),
+		);
+		const waitStopResult = await Promise.race([waitStopPromise, timeoutPromise]);
+		if (waitStopResult === "timeout") {
+			this.logger.sendTelemetryEvent({
+				eventName: "SummarizerStopTimeout",
+				onBehalfOf,
+				timeoutMs: summarizerStopTimeoutMs,
+			});
+		}
 
 		// Propagate reason and ensure that if someone is waiting for cancellation token, they are moving to exit
 		runCoordinator.stop(stopReason);

packages/runtime/container-runtime/src/summary/summaryManager.ts

@@ -103,6 +103,7 @@ export class SummaryManager
 	private state = SummaryManagerState.Off;
 	private summarizer?: ISummarizer;
 	private _disposed = false;
+	private summarizerStopTimeout?: ReturnType<typeof setTimeout>;
 
 	public get disposed(): boolean {
 		return this._disposed;
@@ -347,16 +348,26 @@ export class SummaryManager
 			})
 			.finally(() => {
 				assert(this.state !== SummaryManagerState.Off, 0x264 /* "Expected: Not Off" */);
-				this.state = SummaryManagerState.Off;
+				this.cleanupAfterSummarizerStop();
+			});
+	}
 
-				this.cleanupForwardedEvents();
-				this.summarizer?.close();
-				this.summarizer = undefined;
+	private cleanupAfterSummarizerStop(): void {
+		this.state = SummaryManagerState.Off;
 
-				if (this.getShouldSummarizeState().shouldSummarize) {
-					this.startSummarization();
-				}
-			});
+		// Clear any pending stop timeout to avoid it firing for a different summarizer
+		if (this.summarizerStopTimeout !== undefined) {
+			clearTimeout(this.summarizerStopTimeout);
+			this.summarizerStopTimeout = undefined;
+		}
+
+		this.cleanupForwardedEvents();
+		this.summarizer?.close();
+		this.summarizer = undefined;
+
+		if (this.getShouldSummarizeState().shouldSummarize) {
+			this.startSummarization();
+		}
 	}
 
 	private stop(reason: SummarizerStopReason): void {
@@ -368,6 +379,23 @@ export class SummaryManager
 		// Stopping the running summarizer client should trigger a change
 		// in states when the running summarizer closes
 		this.summarizer?.stop(reason);
+
+		const summarizerCloseTimeoutMs = 2 * 60 * 1000; // 2 minutes
+		// Clear any existing timeout before setting a new one
+		if (this.summarizerStopTimeout !== undefined) {
+			clearTimeout(this.summarizerStopTimeout);
+		}
+		// Set a timeout to force cleanup if the summarizer doesn't close in time
+		this.summarizerStopTimeout = setTimeout(() => {
+			if (this.state === SummaryManagerState.Stopping && this.summarizer !== undefined) {
+				this.logger.sendTelemetryEvent({
+					eventName: "SummarizerStopTimeout",
+					timeoutMs: summarizerCloseTimeoutMs,
+					stopReason: reason,
+				});
+				this.cleanupAfterSummarizerStop();
+			}
+		}, summarizerCloseTimeoutMs);
 	}
 
 	/**
@@ -453,6 +481,9 @@ export class SummaryManager
 		this.connectedState.off("connected", this.handleConnected);
 		this.connectedState.off("disconnected", this.handleDisconnected);
 		this.cleanupForwardedEvents();
+		if (this.summarizerStopTimeout !== undefined) {
+			clearTimeout(this.summarizerStopTimeout);
+		}
 		this._disposed = true;
 	}