From a222e8d93c266b12aacd076f6a8ab9165b7db857 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Wed, 18 Nov 2020 18:29:58 -0800 Subject: [PATCH 01/18] Retry blob/tree reads as we move to skeleton snapshots --- .../src/prefetchDocumentStorageService.ts | 24 ++++---- packages/loader/driver-utils/package.json | 6 ++ .../loader/driver-utils/src/readAndParse.ts | 26 +++++++- .../src/test/readAndParseTests.spec.ts | 60 +++++++++++++++++++ .../container-runtime/src/blobManager.ts | 3 +- 5 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 packages/loader/driver-utils/src/test/readAndParseTests.spec.ts diff --git a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts index 4c57d7464c6c..c4bb4d7bc859 100644 --- a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts +++ b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts @@ -2,11 +2,14 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ + +/* eslint-disable @typescript-eslint/no-floating-promises */ + import { ISnapshotTree, IVersion, } from "@fluidframework/protocol-definitions"; -import { DocumentStorageServiceProxy } from "@fluidframework/driver-utils"; +import { DocumentStorageServiceProxy, readWithRetry } from "@fluidframework/driver-utils"; import { debug } from "./debug"; export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy { @@ -15,10 +18,9 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy private prefetchEnabled = true; public async getSnapshotTree(version?: IVersion): Promise { - const p = this.internalStorageService.getSnapshotTree(version); + const p = readWithRetry(async () => this.internalStorageService.getSnapshotTree(version)); if (this.prefetchEnabled) { // We don't care if the prefetch succeed - // eslint-disable-next-line @typescript-eslint/no-floating-promises p.then((tree: ISnapshotTree | null | undefined) => { if (tree === null || tree === undefined) { return; } this.prefetchTree(tree); @@ -28,7 +30,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy } public async read(blobId: string): Promise { - return this.cachedRead(blobId); + return readWithRetry(async () => this.cachedRead(blobId)); } public stopPrefetch() { @@ -36,8 +38,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy this.prefetchCache.clear(); } - // eslint-disable-next-line @typescript-eslint/promise-function-async - private cachedRead(blobId: string): Promise { + private async cachedRead(blobId: string): Promise { if (this.prefetchEnabled) { const prefetchedBlobP: Promise | undefined = this.prefetchCache.get(blobId); if (prefetchedBlobP !== undefined) { @@ -56,7 +57,6 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy for (const blob of secondary) { // We don't care if the prefetch succeed - // eslint-disable-next-line @typescript-eslint/no-floating-promises this.cachedRead(blob); } } @@ -67,7 +67,6 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy if (blobKey.startsWith(".") || blobKey === "header" || blobKey.startsWith("quorum")) { if (blob !== null) { // We don't care if the prefetch succeed - // eslint-disable-next-line @typescript-eslint/no-floating-promises this.cachedRead(blob); } } else if (!blobKey.startsWith("deltas")) { @@ -79,8 +78,13 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy for (const commit of Object.keys(tree.commits)) { this.getVersions(tree.commits[commit], 1) - // eslint-disable-next-line @typescript-eslint/promise-function-async - .then((moduleCommit) => this.getSnapshotTree(moduleCommit[0])) + .then((moduleCommit) => { + this.internalStorageService.getSnapshotTree(moduleCommit[0]) + .then((snapshotTree: ISnapshotTree | null | undefined) => { + if (snapshotTree === null || snapshotTree === undefined) { return; } + this.prefetchTree(snapshotTree); + }); + }) .catch((error) => debug("Ignored cached read error", error)); } diff --git a/packages/loader/driver-utils/package.json b/packages/loader/driver-utils/package.json index 8d6af79ed2eb..a7c2a624dd07 100644 --- a/packages/loader/driver-utils/package.json +++ b/packages/loader/driver-utils/package.json @@ -23,6 +23,10 @@ "eslint:fix": "eslint --ext=ts,tsx --format stylish src --fix", "lint": "npm run eslint", "lint:fix": "npm run eslint:fix", + "test": "npm run test:mocha", + "test:coverage": "nyc npm test -- --reporter mocha-junit-reporter --reporter-options mochaFile=nyc/junit-report.xml", + "test:mocha": "mocha --recursive dist/test -r node_modules/@fluidframework/mocha-test-setup --unhandled-rejections=strict", + "test:mocha:verbose": "cross-env FLUID_TEST_VERBOSE=1 npm run test:mocha", "tsc": "tsc", "tsfmt": "tsfmt --verify", "tsfmt:fix": "tsfmt --replace" @@ -60,6 +64,7 @@ "devDependencies": { "@fluidframework/build-common": "^0.19.2", "@fluidframework/eslint-config-fluid": "^0.21.0-0", + "@fluidframework/mocha-test-setup": "^0.30.0", "@microsoft/api-extractor": "^7.7.2", "@types/assert": "^1.5.1", "@types/mocha": "^5.2.5", @@ -67,6 +72,7 @@ "@typescript-eslint/parser": "~4.2.0", "concurrently": "^5.2.0", "copyfiles": "^2.1.0", + "cross-env": "^7.0.2", "eslint": "~7.9.0", "eslint-plugin-eslint-comments": "~3.2.0", "eslint-plugin-import": "~2.22.0", diff --git a/packages/loader/driver-utils/src/readAndParse.ts b/packages/loader/driver-utils/src/readAndParse.ts index be82c096a4d1..162a9278c91f 100644 --- a/packages/loader/driver-utils/src/readAndParse.ts +++ b/packages/loader/driver-utils/src/readAndParse.ts @@ -4,7 +4,7 @@ */ import { fromBase64ToUtf8 } from "@fluidframework/common-utils"; -import { IDocumentStorageService } from "@fluidframework/driver-definitions"; +import { DriverErrorType, IDocumentStorageService, IThrottlingWarning } from "@fluidframework/driver-definitions"; /** * Read a blob from IDocumentStorageService, decode it (from "base64") and JSON.parse it into object of type T @@ -14,7 +14,7 @@ import { IDocumentStorageService } from "@fluidframework/driver-definitions"; * @returns the object that we decoded and JSON.parse */ export async function readAndParse(storage: Pick, id: string): Promise { - const encoded = await storage.read(id); + const encoded = await readWithRetry(async () => storage.read(id)); const decoded = fromBase64ToUtf8(encoded); return JSON.parse(decoded) as T; } @@ -31,3 +31,25 @@ export function readAndParseFromBlobs(blobs: {[index: string]: string}, id: s const decoded = fromBase64ToUtf8(encoded); return JSON.parse(decoded) as T; } + +/** + * Utility to retry the read or fetch until it succeeds in it. + * @param api - Method to be retried. + */ +export async function readWithRetry(api: () => Promise): Promise { + let result: T; + try { + result = await api(); + } catch (error) { + let retryAfter = 0; + // If the error is throttling error, then wait for the specified time before retrying. + // eslint-disable-next-line no-null/no-null + if (error !== null && typeof error === "object" && error.errorType === DriverErrorType.throttlingError) { + retryAfter = (error as IThrottlingWarning).retryAfterSeconds; + } + result = await new Promise((resolve) => setTimeout(async () => { + resolve(await readWithRetry(api)); + }, retryAfter)); + } + return result; +} diff --git a/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts b/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts new file mode 100644 index 000000000000..315ade39b745 --- /dev/null +++ b/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts @@ -0,0 +1,60 @@ +/*! + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "assert"; +import { DriverErrorType } from "@fluidframework/driver-definitions"; +import { readWithRetry } from "../readAndParse"; + +describe("Read and parse Tests", () => { + it("Should succeed at first time", async () => { + let retryTimes: number = 1; + let success = false; + const api = async () => { + retryTimes -= 1; + return true; + }; + success = await readWithRetry(api); + assert.strictEqual(retryTimes, 0, "Should succeed at first time"); + assert.strictEqual(success, true, "Retry shoul succeed ultimately"); + }); + + it("Check that it retries infinitely", async () => { + let retryTimes: number = 5; + let success = false; + const api = async () => { + if (retryTimes > 0) { + retryTimes -= 1; + throw new Error("Throw error"); + } + return true; + }; + success = await readWithRetry(api); + assert.strictEqual(retryTimes, 0, "Should keep retrying until success"); + assert.strictEqual(success, true, "Retry shoul succeed ultimately"); + }); + + it("Check that it retries after retry seconds", async () => { + let retryTimes: number = 1; + let success = false; + let timerFinished = false; + setTimeout(() => { + timerFinished = true; + }, 250); + const api = async () => { + if (retryTimes > 0) { + retryTimes -= 1; + const error = new Error("Throttle Error"); + (error as any).errorType = DriverErrorType.throttlingError; + (error as any).retryAfterSeconds = 500; + throw error; + } + return true; + }; + success = await readWithRetry(api); + assert.strictEqual(timerFinished, true, "Timer should be destroyed"); + assert.strictEqual(retryTimes, 0, "Should retry once"); + assert.strictEqual(success, true, "Retry shoul succeed ultimately"); + }); +}); diff --git a/packages/runtime/container-runtime/src/blobManager.ts b/packages/runtime/container-runtime/src/blobManager.ts index eb9e41173949..2e7209869abc 100644 --- a/packages/runtime/container-runtime/src/blobManager.ts +++ b/packages/runtime/container-runtime/src/blobManager.ts @@ -5,6 +5,7 @@ import { IFluidHandle, IFluidHandleContext } from "@fluidframework/core-interfaces"; import { IDocumentStorageService } from "@fluidframework/driver-definitions"; +import { readWithRetry } from "@fluidframework/driver-utils"; import { AttachmentTreeEntry } from "@fluidframework/protocol-base"; import { ISnapshotTree, ITree } from "@fluidframework/protocol-definitions"; import { generateHandleContextPath } from "@fluidframework/runtime-utils"; @@ -53,7 +54,7 @@ export class BlobManager { return new BlobHandle( `${BlobManager.basePath}/${blobId}`, this.routeContext, - async () => this.getStorage().readBlob(blobId), + async () => readWithRetry(async () => this.getStorage().readBlob(blobId)), () => null, ); } From a9627cf3a842fa2499b7e133a5f9bd1f61a6a2c2 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Thu, 19 Nov 2020 23:10:16 -0800 Subject: [PATCH 02/18] don't retry if canRetry is not true --- .../loader/driver-utils/src/readAndParse.ts | 4 ++ .../src/test/readAndParseTests.spec.ts | 42 ++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/loader/driver-utils/src/readAndParse.ts b/packages/loader/driver-utils/src/readAndParse.ts index 162a9278c91f..7a91bbfe995c 100644 --- a/packages/loader/driver-utils/src/readAndParse.ts +++ b/packages/loader/driver-utils/src/readAndParse.ts @@ -41,6 +41,10 @@ export async function readWithRetry(api: () => Promise): Promise { try { result = await api(); } catch (error) { + // If it is not retriable, then just throw the error. + if (typeof error !== "object" || error.canRetry !== true) { + throw error; + } let retryAfter = 0; // If the error is throttling error, then wait for the specified time before retrying. // eslint-disable-next-line no-null/no-null diff --git a/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts b/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts index 315ade39b745..c122cad6cdb7 100644 --- a/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts +++ b/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts @@ -26,7 +26,9 @@ describe("Read and parse Tests", () => { const api = async () => { if (retryTimes > 0) { retryTimes -= 1; - throw new Error("Throw error"); + const error = new Error("Throw error"); + (error as any).canRetry = true; + throw error; } return true; }; @@ -48,6 +50,7 @@ describe("Read and parse Tests", () => { const error = new Error("Throttle Error"); (error as any).errorType = DriverErrorType.throttlingError; (error as any).retryAfterSeconds = 500; + (error as any).canRetry = true; throw error; } return true; @@ -57,4 +60,41 @@ describe("Read and parse Tests", () => { assert.strictEqual(retryTimes, 0, "Should retry once"); assert.strictEqual(success, true, "Retry shoul succeed ultimately"); }); + + it("If error is just a string, don't retry", async () => { + let retryTimes: number = 1; + let success = false; + const api = async () => { + if (retryTimes > 0) { + retryTimes -= 1; + // eslint-disable-next-line no-throw-literal + throw "error"; + } + return true; + }; + try { + success = await readWithRetry(api); + assert.fail("Should not succeed"); + } catch (error) {} + assert.strictEqual(retryTimes, 0, "Should not retry"); + assert.strictEqual(success, false, "Should not succeed as error was not an object"); + }); + + it("Should not retry if canRetry is set as false", async () => { + let retryTimes: number = 1; + let success = false; + const api = async () => { + if (retryTimes > 0) { + retryTimes -= 1; + throw new Error("error"); + } + return true; + }; + try { + success = await readWithRetry(api); + assert.fail("Should not succeed"); + } catch (error) {} + assert.strictEqual(retryTimes, 0, "Should not retry"); + assert.strictEqual(success, false, "Should not succeed as canRetry was not set"); + }); }); From fc94db96a0f25ab547bc71a2cc674c0a6502eb46 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Thu, 19 Nov 2020 23:50:30 -0800 Subject: [PATCH 03/18] chaange --- .../container-loader/src/prefetchDocumentStorageService.ts | 4 ++++ packages/runtime/container-runtime/src/blobManager.ts | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts index c4bb4d7bc859..5490b5b16f5b 100644 --- a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts +++ b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts @@ -29,6 +29,10 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy return p; } + public async readBlob(id: string): Promise { + return readWithRetry(async () => this.internalStorageService.readBlob(id)); + } + public async read(blobId: string): Promise { return readWithRetry(async () => this.cachedRead(blobId)); } diff --git a/packages/runtime/container-runtime/src/blobManager.ts b/packages/runtime/container-runtime/src/blobManager.ts index 2e7209869abc..eb9e41173949 100644 --- a/packages/runtime/container-runtime/src/blobManager.ts +++ b/packages/runtime/container-runtime/src/blobManager.ts @@ -5,7 +5,6 @@ import { IFluidHandle, IFluidHandleContext } from "@fluidframework/core-interfaces"; import { IDocumentStorageService } from "@fluidframework/driver-definitions"; -import { readWithRetry } from "@fluidframework/driver-utils"; import { AttachmentTreeEntry } from "@fluidframework/protocol-base"; import { ISnapshotTree, ITree } from "@fluidframework/protocol-definitions"; import { generateHandleContextPath } from "@fluidframework/runtime-utils"; @@ -54,7 +53,7 @@ export class BlobManager { return new BlobHandle( `${BlobManager.basePath}/${blobId}`, this.routeContext, - async () => readWithRetry(async () => this.getStorage().readBlob(blobId)), + async () => this.getStorage().readBlob(blobId), () => null, ); } From d1d5c574f13c66f0d1620f00a6a406b307d55fe0 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Thu, 19 Nov 2020 23:56:56 -0800 Subject: [PATCH 04/18] remove --- packages/loader/driver-utils/src/readAndParse.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/loader/driver-utils/src/readAndParse.ts b/packages/loader/driver-utils/src/readAndParse.ts index 7a91bbfe995c..0a21342f36ba 100644 --- a/packages/loader/driver-utils/src/readAndParse.ts +++ b/packages/loader/driver-utils/src/readAndParse.ts @@ -14,7 +14,7 @@ import { DriverErrorType, IDocumentStorageService, IThrottlingWarning } from "@f * @returns the object that we decoded and JSON.parse */ export async function readAndParse(storage: Pick, id: string): Promise { - const encoded = await readWithRetry(async () => storage.read(id)); + const encoded = await storage.read(id); const decoded = fromBase64ToUtf8(encoded); return JSON.parse(decoded) as T; } From 57416a27f108f7e3dc2ea71043ae5ac5f15692fa Mon Sep 17 00:00:00 2001 From: jatingarg Date: Tue, 24 Nov 2020 12:40:51 -0800 Subject: [PATCH 05/18] Pr sugg --- .../container-definitions/src/loader.ts | 3 +- .../loader/container-loader/src/container.ts | 12 +- .../container-loader/src/deltaManager.ts | 27 ++-- packages/loader/container-loader/src/index.ts | 1 + .../src/prefetchDocumentStorageService.ts | 8 +- .../src/retriableDocumentStorageService.ts | 57 ++++++++ .../retriableDocumentStorageService.spec.ts | 133 ++++++++++++++++++ packages/loader/driver-utils/package.json | 5 - .../loader/driver-utils/src/readAndParse.ts | 28 +--- .../src/test/readAndParseTests.spec.ts | 100 ------------- ...nsusOrderedCollectionEndToEndTests.spec.ts | 4 +- 11 files changed, 227 insertions(+), 151 deletions(-) create mode 100644 packages/loader/container-loader/src/retriableDocumentStorageService.ts create mode 100644 packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts delete mode 100644 packages/loader/driver-utils/src/test/readAndParseTests.spec.ts diff --git a/packages/loader/container-definitions/src/loader.ts b/packages/loader/container-definitions/src/loader.ts index f796d7e07382..34af24979245 100644 --- a/packages/loader/container-definitions/src/loader.ts +++ b/packages/loader/container-definitions/src/loader.ts @@ -21,7 +21,7 @@ import { import { IResolvedUrl } from "@fluidframework/driver-definitions"; import { IEvent, IEventProvider } from "@fluidframework/common-definitions"; import { IDeltaManager } from "./deltas"; -import { ICriticalContainerError, ContainerWarning } from "./error"; +import { ICriticalContainerError, ContainerWarning, IThrottlingWarning } from "./error"; import { IFluidModule } from "./fluidModule"; import { AttachState } from "./runtime"; @@ -93,6 +93,7 @@ export interface IContainerEvents extends IEvent { (event: "closed", listener: (error?: ICriticalContainerError) => void); (event: "warning", listener: (error: ContainerWarning) => void); (event: "op", listener: (message: ISequencedDocumentMessage) => void); + (event: "throttled", listener: (error: IThrottlingWarning) => void); } /** diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 50decafb727a..f897620eea51 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -94,6 +94,7 @@ import { Loader, RelativeLoader } from "./loader"; import { pkgVersion } from "./packageVersion"; import { PrefetchDocumentStorageService } from "./prefetchDocumentStorageService"; import { parseUrl, convertProtocolAndAppSummaryToSnapshotTree } from "./utils"; +import { RetriableDocumentStorageService } from "./retriableDocumentStorageService"; const detachedContainerRefSeqNumber = 0; @@ -275,6 +276,7 @@ export class Container extends EventEmitterWithErrorHandling i // Active chaincode and associated runtime private _storageService: IDocumentStorageService | undefined; + private retriableStorageService: RetriableDocumentStorageService | undefined; private get storageService() { if (this._storageService === undefined) { throw new Error("Attempted to access storageService before it was defined"); @@ -501,6 +503,8 @@ export class Container extends EventEmitterWithErrorHandling i this._deltaManager.close(error); + this.retriableStorageService?.stopRetry(); + this._protocolHandler?.close(); this._context?.dispose(error !== undefined ? new Error(error.message) : undefined); @@ -1140,11 +1144,13 @@ export class Container extends EventEmitterWithErrorHandling i const storageService = await this.service.connectToStorage(); // Enable prefetching for the service unless it has a caching policy set otherwise: - const service = new PrefetchDocumentStorageService(storageService); + const prefetchStorageService = new PrefetchDocumentStorageService(storageService); if (this.service.policies?.caching === LoaderCachingPolicy.NoCaching) { - service.stopPrefetch(); + prefetchStorageService.stopPrefetch(); } - return service; + + this.retriableStorageService = new RetriableDocumentStorageService(prefetchStorageService, this); + return this.retriableStorageService; } private async getDocumentAttributes( diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index dc2d977d3252..cc0d85900c93 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -46,6 +46,7 @@ import { CreateContainerError } from "@fluidframework/container-utils"; import { debug } from "./debug"; import { DeltaQueue } from "./deltaQueue"; import { logNetworkFailure, waitForConnectedState } from "./networkUtils"; +import { Container } from "./container"; const MaxReconnectDelaySeconds = 8; const InitialReconnectDelaySeconds = 1; @@ -58,7 +59,22 @@ const DefaultChunkSize = 16 * 1024; const ImmediateNoOpResponse = ""; // eslint-disable-next-line @typescript-eslint/no-unsafe-return -const getRetryDelayFromError = (error: any): number | undefined => error?.retryAfterSeconds; +export const getRetryDelayFromError = (error: any): number | undefined => error?.retryAfterSeconds; + +export function emitThrottlingWarning( + delayTime: number, + error: ICriticalContainerError, + emitter: Container | DeltaManager, +) { + if (delayTime > 0) { + const throttlingError: IThrottlingWarning = { + errorType: ContainerErrorType.throttlingError, + message: `Service busy/throttled: ${error.message}`, + retryAfterSeconds: delayTime, + }; + emitter.emit("throttled", throttlingError); + } +} function getNackReconnectInfo(nackContent: INackContent) { const reason = `Nack: ${nackContent.message}`; @@ -939,14 +955,7 @@ export class DeltaManager } const delayTime = Math.max(this.deltaStorageDelay, this.deltaStreamDelay); - if (delayTime > 0) { - const throttlingError: IThrottlingWarning = { - errorType: ContainerErrorType.throttlingError, - message: `Service busy/throttled: ${error.message}`, - retryAfterSeconds: delayTime, - }; - this.emit("throttled", throttlingError); - } + emitThrottlingWarning(delayTime, error, this); } private readonly opHandler = (documentId: string, messages: ISequencedDocumentMessage[]) => { diff --git a/packages/loader/container-loader/src/index.ts b/packages/loader/container-loader/src/index.ts index 80559a150bda..288b0f2761b3 100644 --- a/packages/loader/container-loader/src/index.ts +++ b/packages/loader/container-loader/src/index.ts @@ -9,3 +9,4 @@ export * from "./deltaManager"; export * from "./loader"; export * from "./networkUtils"; export * from "./utils"; +export * from "./retriableDocumentStorageService"; diff --git a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts index 5490b5b16f5b..6cde0a91d59c 100644 --- a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts +++ b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts @@ -9,7 +9,7 @@ import { ISnapshotTree, IVersion, } from "@fluidframework/protocol-definitions"; -import { DocumentStorageServiceProxy, readWithRetry } from "@fluidframework/driver-utils"; +import { DocumentStorageServiceProxy } from "@fluidframework/driver-utils"; import { debug } from "./debug"; export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy { @@ -18,7 +18,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy private prefetchEnabled = true; public async getSnapshotTree(version?: IVersion): Promise { - const p = readWithRetry(async () => this.internalStorageService.getSnapshotTree(version)); + const p = this.internalStorageService.getSnapshotTree(version); if (this.prefetchEnabled) { // We don't care if the prefetch succeed p.then((tree: ISnapshotTree | null | undefined) => { @@ -30,11 +30,11 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy } public async readBlob(id: string): Promise { - return readWithRetry(async () => this.internalStorageService.readBlob(id)); + return this.internalStorageService.readBlob(id); } public async read(blobId: string): Promise { - return readWithRetry(async () => this.cachedRead(blobId)); + return this.cachedRead(blobId); } public stopPrefetch() { diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts new file mode 100644 index 000000000000..7854b495c56e --- /dev/null +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -0,0 +1,57 @@ +/*! + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { CreateContainerError } from "@fluidframework/container-utils"; +import { IDocumentStorageService } from "@fluidframework/driver-definitions"; +import { canRetryOnError, DocumentStorageServiceProxy } from "@fluidframework/driver-utils"; +import { ISnapshotTree, IVersion } from "@fluidframework/protocol-definitions"; +import { Container } from "./container"; +import { emitThrottlingWarning, getRetryDelayFromError } from "./deltaManager"; + +export class RetriableDocumentStorageService extends DocumentStorageServiceProxy { + private shouldRetry = true; + constructor( + internalStorageService: IDocumentStorageService, + private readonly container: Container, + ) { + super(internalStorageService); + } + + public stopRetry() { + this.shouldRetry = false; + } + + public async getSnapshotTree(version?: IVersion): Promise { + return this.readWithRetry(async () => this.internalStorageService.getSnapshotTree(version)); + } + + public async read(blobId: string): Promise { + return this.readWithRetry(async () => this.internalStorageService.read(blobId)); + } + + public async readBlob(id: string): Promise { + return this.readWithRetry(async () => this.internalStorageService.readBlob(id)); + } + + private async readWithRetry(api: () => Promise, retryLimitSeconds: number = 500): Promise { + let result: T; + try { + result = await api(); + } catch (error) { + // If it is not retriable, then just throw the error. + const canRetry = canRetryOnError(error); + if (!(canRetry && this.shouldRetry)) { + throw error; + } + // If the error is throttling error, then wait for the specified time before retrying. + const retryAfter = getRetryDelayFromError(error) ?? Math.min(retryLimitSeconds * 2, 8000); + emitThrottlingWarning(retryAfter, CreateContainerError(error), this.container); + result = await new Promise((resolve) => setTimeout(async () => { + resolve(await this.readWithRetry(api, retryAfter)); + }, retryAfter)); + } + return result; + } +} diff --git a/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts new file mode 100644 index 000000000000..d93c118fd786 --- /dev/null +++ b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts @@ -0,0 +1,133 @@ +/*! + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { strict as assert } from "assert"; +import { IThrottlingWarning } from "@fluidframework/container-definitions"; +import { DriverErrorType, IDocumentStorageService } from "@fluidframework/driver-definitions"; +import { RetriableDocumentStorageService } from "../retriableDocumentStorageService"; +import { Container } from "../container"; + +describe("RetriableDocumentStorageService Tests", () => { + let retriableStorageService: RetriableDocumentStorageService; + let internalService: IDocumentStorageService; + beforeEach(() => { + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + internalService = {} as IDocumentStorageService; + const container = { emit: (event: string, error: IThrottlingWarning) => {} }; + retriableStorageService = new RetriableDocumentStorageService(internalService, container as Container); + }); + + it("Should succeed at first time", async () => { + let retryTimes: number = 1; + let success = "false"; + internalService.read = async (id: string) => { + retryTimes -= 1; + return "true"; + }; + success = await retriableStorageService.read(""); + assert.strictEqual(retryTimes, 0, "Should succeed at first time"); + assert.strictEqual(success, "true", "Retry shoul succeed ultimately"); + }); + + it("Check that it retries infinitely", async () => { + let retryTimes: number = 5; + let success = "false"; + internalService.read = async (id: string) => { + if (retryTimes > 0) { + retryTimes -= 1; + const error = new Error("Throw error"); + (error as any).retryAfterSeconds = 10; + throw error; + } + return "true"; + }; + success = await retriableStorageService.read(""); + assert.strictEqual(retryTimes, 0, "Should keep retrying until success"); + assert.strictEqual(success, "true", "Retry shoul succeed ultimately"); + }); + + it("Check that it retries after retry seconds", async () => { + let retryTimes: number = 1; + let success = "false"; + let timerFinished = false; + setTimeout(() => { + timerFinished = true; + }, 200); + internalService.read = async (id: string) => { + if (retryTimes > 0) { + retryTimes -= 1; + const error = new Error("Throttle Error"); + (error as any).errorType = DriverErrorType.throttlingError; + (error as any).retryAfterSeconds = 400; + (error as any).canRetry = true; + throw error; + } + return "true"; + }; + success = await retriableStorageService.read(""); + assert.strictEqual(timerFinished, true, "Timer should be destroyed"); + assert.strictEqual(retryTimes, 0, "Should retry once"); + assert.strictEqual(success, "true", "Retry shoul succeed ultimately"); + }); + + it("If error is just a string, should retry as canRetry is not false", async () => { + let retryTimes: number = 1; + let success = "false"; + internalService.read = async (id: string) => { + if (retryTimes > 0) { + retryTimes -= 1; + // eslint-disable-next-line no-throw-literal + throw "error"; + } + return "true"; + }; + try { + success = await retriableStorageService.read(""); + } catch (error) {} + assert.strictEqual(retryTimes, 0, "Should retry"); + assert.strictEqual(success, "true", "Should succeed as retry should be successful"); + }); + + it("Should not retry if canRetry is set as false", async () => { + let retryTimes: number = 1; + let success = "false"; + internalService.read = async (id: string) => { + if (retryTimes > 0) { + retryTimes -= 1; + const error = new Error("error"); + (error as any).canRetry = false; + throw error; + } + return "true"; + }; + try { + success = await retriableStorageService.read(""); + assert.fail("Should not succeed"); + } catch (error) {} + assert.strictEqual(retryTimes, 0, "Should not retry"); + assert.strictEqual(success, "false", "Should not succeed as canRetry was not set"); + }); + + it("Should not retry if it is disabled", async () => { + let retryTimes: number = 1; + let success = "false"; + retriableStorageService.stopRetry(); + internalService.read = async (id: string) => { + if (retryTimes > 0) { + retryTimes -= 1; + const error = new Error("error"); + (error as any).canRetry = true; + throw error; + } + return "true"; + }; + try { + success = await retriableStorageService.read(""); + assert.fail("Should not succeed"); + } catch (error) {} + assert.strictEqual(retryTimes, 0, "Should not retry"); + assert.strictEqual(success, "false", "Should not succeed as retrying was disabled"); + }); +}); diff --git a/packages/loader/driver-utils/package.json b/packages/loader/driver-utils/package.json index f3dbf615e8ef..cc8425ddbea6 100644 --- a/packages/loader/driver-utils/package.json +++ b/packages/loader/driver-utils/package.json @@ -23,10 +23,6 @@ "eslint:fix": "eslint --ext=ts,tsx --format stylish src --fix", "lint": "npm run eslint", "lint:fix": "npm run eslint:fix", - "test": "npm run test:mocha", - "test:coverage": "nyc npm test -- --reporter mocha-junit-reporter --reporter-options mochaFile=nyc/junit-report.xml", - "test:mocha": "mocha --recursive dist/test -r node_modules/@fluidframework/mocha-test-setup --unhandled-rejections=strict", - "test:mocha:verbose": "cross-env FLUID_TEST_VERBOSE=1 npm run test:mocha", "tsc": "tsc", "tsfmt": "tsfmt --verify", "tsfmt:fix": "tsfmt --replace" @@ -64,7 +60,6 @@ "devDependencies": { "@fluidframework/build-common": "^0.19.2", "@fluidframework/eslint-config-fluid": "^0.21.0", - "@fluidframework/mocha-test-setup": "^0.30.0", "@microsoft/api-extractor": "^7.7.2", "@types/assert": "^1.5.1", "@types/mocha": "^5.2.5", diff --git a/packages/loader/driver-utils/src/readAndParse.ts b/packages/loader/driver-utils/src/readAndParse.ts index 0a21342f36ba..be82c096a4d1 100644 --- a/packages/loader/driver-utils/src/readAndParse.ts +++ b/packages/loader/driver-utils/src/readAndParse.ts @@ -4,7 +4,7 @@ */ import { fromBase64ToUtf8 } from "@fluidframework/common-utils"; -import { DriverErrorType, IDocumentStorageService, IThrottlingWarning } from "@fluidframework/driver-definitions"; +import { IDocumentStorageService } from "@fluidframework/driver-definitions"; /** * Read a blob from IDocumentStorageService, decode it (from "base64") and JSON.parse it into object of type T @@ -31,29 +31,3 @@ export function readAndParseFromBlobs(blobs: {[index: string]: string}, id: s const decoded = fromBase64ToUtf8(encoded); return JSON.parse(decoded) as T; } - -/** - * Utility to retry the read or fetch until it succeeds in it. - * @param api - Method to be retried. - */ -export async function readWithRetry(api: () => Promise): Promise { - let result: T; - try { - result = await api(); - } catch (error) { - // If it is not retriable, then just throw the error. - if (typeof error !== "object" || error.canRetry !== true) { - throw error; - } - let retryAfter = 0; - // If the error is throttling error, then wait for the specified time before retrying. - // eslint-disable-next-line no-null/no-null - if (error !== null && typeof error === "object" && error.errorType === DriverErrorType.throttlingError) { - retryAfter = (error as IThrottlingWarning).retryAfterSeconds; - } - result = await new Promise((resolve) => setTimeout(async () => { - resolve(await readWithRetry(api)); - }, retryAfter)); - } - return result; -} diff --git a/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts b/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts deleted file mode 100644 index c122cad6cdb7..000000000000 --- a/packages/loader/driver-utils/src/test/readAndParseTests.spec.ts +++ /dev/null @@ -1,100 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. - */ - -import { strict as assert } from "assert"; -import { DriverErrorType } from "@fluidframework/driver-definitions"; -import { readWithRetry } from "../readAndParse"; - -describe("Read and parse Tests", () => { - it("Should succeed at first time", async () => { - let retryTimes: number = 1; - let success = false; - const api = async () => { - retryTimes -= 1; - return true; - }; - success = await readWithRetry(api); - assert.strictEqual(retryTimes, 0, "Should succeed at first time"); - assert.strictEqual(success, true, "Retry shoul succeed ultimately"); - }); - - it("Check that it retries infinitely", async () => { - let retryTimes: number = 5; - let success = false; - const api = async () => { - if (retryTimes > 0) { - retryTimes -= 1; - const error = new Error("Throw error"); - (error as any).canRetry = true; - throw error; - } - return true; - }; - success = await readWithRetry(api); - assert.strictEqual(retryTimes, 0, "Should keep retrying until success"); - assert.strictEqual(success, true, "Retry shoul succeed ultimately"); - }); - - it("Check that it retries after retry seconds", async () => { - let retryTimes: number = 1; - let success = false; - let timerFinished = false; - setTimeout(() => { - timerFinished = true; - }, 250); - const api = async () => { - if (retryTimes > 0) { - retryTimes -= 1; - const error = new Error("Throttle Error"); - (error as any).errorType = DriverErrorType.throttlingError; - (error as any).retryAfterSeconds = 500; - (error as any).canRetry = true; - throw error; - } - return true; - }; - success = await readWithRetry(api); - assert.strictEqual(timerFinished, true, "Timer should be destroyed"); - assert.strictEqual(retryTimes, 0, "Should retry once"); - assert.strictEqual(success, true, "Retry shoul succeed ultimately"); - }); - - it("If error is just a string, don't retry", async () => { - let retryTimes: number = 1; - let success = false; - const api = async () => { - if (retryTimes > 0) { - retryTimes -= 1; - // eslint-disable-next-line no-throw-literal - throw "error"; - } - return true; - }; - try { - success = await readWithRetry(api); - assert.fail("Should not succeed"); - } catch (error) {} - assert.strictEqual(retryTimes, 0, "Should not retry"); - assert.strictEqual(success, false, "Should not succeed as error was not an object"); - }); - - it("Should not retry if canRetry is set as false", async () => { - let retryTimes: number = 1; - let success = false; - const api = async () => { - if (retryTimes > 0) { - retryTimes -= 1; - throw new Error("error"); - } - return true; - }; - try { - success = await readWithRetry(api); - assert.fail("Should not succeed"); - } catch (error) {} - assert.strictEqual(retryTimes, 0, "Should not retry"); - assert.strictEqual(success, false, "Should not succeed as canRetry was not set"); - }); -}); diff --git a/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts b/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts index 2dd30bb34d09..a2e3c82071b0 100644 --- a/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts +++ b/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts @@ -55,12 +55,12 @@ function generate( beforeEach(async () => { // Create a Container for the first client. - container1 = await args.makeTestContainer(testContainerConfig); + container1 = await args.makeTestContainer(testContainerConfig) as IContainer; dataStore1 = await requestFluidObject(container1, "default"); sharedMap1 = await dataStore1.getSharedObject(mapId); // Load the Container that was created by the first client. - container2 = await args.loadTestContainer(testContainerConfig); + container2 = await args.loadTestContainer(testContainerConfig) as IContainer; dataStore2 = await requestFluidObject(container2, "default"); sharedMap2 = await dataStore2.getSharedObject(mapId); From e1d83a6d1e415511b20303a7889f780205b8b5a5 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Tue, 24 Nov 2020 12:44:38 -0800 Subject: [PATCH 06/18] Pr sugg --- .../src/prefetchDocumentStorageService.ts | 12 +----------- packages/loader/driver-utils/package.json | 1 - 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts index 6cde0a91d59c..64fb9055420e 100644 --- a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts +++ b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts @@ -29,10 +29,6 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy return p; } - public async readBlob(id: string): Promise { - return this.internalStorageService.readBlob(id); - } - public async read(blobId: string): Promise { return this.cachedRead(blobId); } @@ -82,13 +78,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy for (const commit of Object.keys(tree.commits)) { this.getVersions(tree.commits[commit], 1) - .then((moduleCommit) => { - this.internalStorageService.getSnapshotTree(moduleCommit[0]) - .then((snapshotTree: ISnapshotTree | null | undefined) => { - if (snapshotTree === null || snapshotTree === undefined) { return; } - this.prefetchTree(snapshotTree); - }); - }) + .then((moduleCommit) => this.getSnapshotTree(moduleCommit[0])) .catch((error) => debug("Ignored cached read error", error)); } diff --git a/packages/loader/driver-utils/package.json b/packages/loader/driver-utils/package.json index cc8425ddbea6..cfaee1ac5bc3 100644 --- a/packages/loader/driver-utils/package.json +++ b/packages/loader/driver-utils/package.json @@ -67,7 +67,6 @@ "@typescript-eslint/parser": "~4.2.0", "concurrently": "^5.2.0", "copyfiles": "^2.1.0", - "cross-env": "^7.0.2", "eslint": "~7.9.0", "eslint-plugin-eslint-comments": "~3.2.0", "eslint-plugin-import": "~2.22.0", From 9a837441b3039e27191b73d44e7a55107b5cf015 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Tue, 24 Nov 2020 12:53:45 -0800 Subject: [PATCH 07/18] Pr sugg --- .../container-loader/src/prefetchDocumentStorageService.ts | 2 +- .../container-loader/src/retriableDocumentStorageService.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts index 64fb9055420e..7e84244b6b7c 100644 --- a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts +++ b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts @@ -78,7 +78,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy for (const commit of Object.keys(tree.commits)) { this.getVersions(tree.commits[commit], 1) - .then((moduleCommit) => this.getSnapshotTree(moduleCommit[0])) + .then(async (moduleCommit) => this.getSnapshotTree(moduleCommit[0])) .catch((error) => debug("Ignored cached read error", error)); } diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index 7854b495c56e..21e598dfd47e 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -35,7 +35,7 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy return this.readWithRetry(async () => this.internalStorageService.readBlob(id)); } - private async readWithRetry(api: () => Promise, retryLimitSeconds: number = 500): Promise { + private async readWithRetry(api: () => Promise, retryLimitSeconds: number = 0): Promise { let result: T; try { result = await api(); @@ -46,6 +46,7 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy throw error; } // If the error is throttling error, then wait for the specified time before retrying. + // If the waitTime is not specified, then we start with retrying immediately to max of 8s. const retryAfter = getRetryDelayFromError(error) ?? Math.min(retryLimitSeconds * 2, 8000); emitThrottlingWarning(retryAfter, CreateContainerError(error), this.container); result = await new Promise((resolve) => setTimeout(async () => { From d5d5d74d77e4c7063a18f2bac593c2330832496b Mon Sep 17 00:00:00 2001 From: jatingarg Date: Wed, 25 Nov 2020 15:21:33 -0800 Subject: [PATCH 08/18] pr sugg --- .../loader/container-loader/src/container.ts | 2 +- .../src/retriableDocumentStorageService.ts | 51 +++++++++++-------- .../retriableDocumentStorageService.spec.ts | 2 +- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index f897620eea51..ee557ca799cd 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -503,7 +503,7 @@ export class Container extends EventEmitterWithErrorHandling i this._deltaManager.close(error); - this.retriableStorageService?.stopRetry(); + this.retriableStorageService?.dispose(); this._protocolHandler?.close(); diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index 21e598dfd47e..aae937b36405 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -11,7 +11,7 @@ import { Container } from "./container"; import { emitThrottlingWarning, getRetryDelayFromError } from "./deltaManager"; export class RetriableDocumentStorageService extends DocumentStorageServiceProxy { - private shouldRetry = true; + private disposed = false; constructor( internalStorageService: IDocumentStorageService, private readonly container: Container, @@ -19,8 +19,8 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy super(internalStorageService); } - public stopRetry() { - this.shouldRetry = false; + public dispose() { + this.disposed = true; } public async getSnapshotTree(version?: IVersion): Promise { @@ -35,24 +35,35 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy return this.readWithRetry(async () => this.internalStorageService.readBlob(id)); } + public async readString(id: string): Promise { + return this.readWithRetry(async () => this.internalStorageService.readString(id)); + } + + private async delay(timeMs: number): Promise { + return new Promise((resolve) => setTimeout(() => resolve(), timeMs)); + } + private async readWithRetry(api: () => Promise, retryLimitSeconds: number = 0): Promise { - let result: T; - try { - result = await api(); - } catch (error) { - // If it is not retriable, then just throw the error. - const canRetry = canRetryOnError(error); - if (!(canRetry && this.shouldRetry)) { - throw error; + let result: T | undefined; + let success = false; + do { + try { + result = await api(); + success = true; + } catch (err) { + // If it is not retriable, then just throw the error. + const canRetry = canRetryOnError(err); + if (!canRetry || this.disposed) { + throw err; + } + // If the error is throttling error, then wait for the specified time before retrying. + // If the waitTime is not specified, then we start with retrying immediately to max of 8s. + const retryAfter = getRetryDelayFromError(err) ?? Math.min(retryLimitSeconds * 2, 8000); + emitThrottlingWarning(retryAfter, CreateContainerError(err), this.container); + await this.delay(retryAfter); } - // If the error is throttling error, then wait for the specified time before retrying. - // If the waitTime is not specified, then we start with retrying immediately to max of 8s. - const retryAfter = getRetryDelayFromError(error) ?? Math.min(retryLimitSeconds * 2, 8000); - emitThrottlingWarning(retryAfter, CreateContainerError(error), this.container); - result = await new Promise((resolve) => setTimeout(async () => { - resolve(await this.readWithRetry(api, retryAfter)); - }, retryAfter)); - } - return result; + } while (!success); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return result!; } } diff --git a/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts index d93c118fd786..21cb7f136ea8 100644 --- a/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts +++ b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts @@ -113,7 +113,7 @@ describe("RetriableDocumentStorageService Tests", () => { it("Should not retry if it is disabled", async () => { let retryTimes: number = 1; let success = "false"; - retriableStorageService.stopRetry(); + retriableStorageService.dispose(); internalService.read = async (id: string) => { if (retryTimes > 0) { retryTimes -= 1; From 98d5e0c37c3784fd73b818f0126f062f74d6d506 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Mon, 30 Nov 2020 13:47:56 -0800 Subject: [PATCH 09/18] pr sugg --- .../loader/container-loader/src/container.ts | 49 ++++++++++++++-- .../container-loader/src/deltaManager.ts | 57 +++---------------- .../src/retriableDocumentStorageService.ts | 40 ++++++++++--- .../src/test/deltaManager.spec.ts | 5 ++ .../retriableDocumentStorageService.spec.ts | 9 +-- .../src/test/containerRuntime.spec.ts | 5 ++ 6 files changed, 100 insertions(+), 65 deletions(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index ee557ca799cd..9198f88a8226 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -29,6 +29,7 @@ import { ContainerWarning, IThrottlingWarning, AttachState, + ContainerErrorType, } from "@fluidframework/container-definitions"; import { CreateContainerError, GenericError } from "@fluidframework/container-utils"; import { @@ -102,6 +103,12 @@ interface ILocalSequencedClient extends ISequencedClient { shouldHaveLeft?: boolean; } +export enum RetryFor { + DeltaStream, + DeltaStorage, + Storage, +} + export interface IContainerConfig { resolvedUrl?: IResolvedUrl; canReconnect?: boolean; @@ -293,6 +300,9 @@ export class Container extends EventEmitterWithErrorHandling i private service: IDocumentService | undefined; private _connectionState = ConnectionState.Disconnected; private readonly _audience: Audience; + private deltaStorageDelay: number = 0; + private deltaStreamDelay: number = 0; + private storageDelay: number = 0; private _context: ContainerContext | undefined; private get context() { @@ -742,6 +752,40 @@ export class Container extends EventEmitterWithErrorHandling i this.emit("warning", warning); } + public cancelDelayInfo(retryEndpoint: RetryFor) { + if (retryEndpoint === RetryFor.DeltaStorage) { + this.deltaStorageDelay = 0; + } else if (retryEndpoint === RetryFor.DeltaStream) { + this.deltaStreamDelay = 0; + } else { + this.storageDelay = 0; + } + } + + public emitDelayInfo( + retryEndpoint: RetryFor, + delaySeconds: number, + error: ICriticalContainerError, + ) { + if (retryEndpoint === RetryFor.DeltaStorage) { + this.deltaStorageDelay = delaySeconds; + } else if (retryEndpoint === RetryFor.DeltaStream) { + this.deltaStreamDelay = delaySeconds; + } else { + this.storageDelay = delaySeconds; + } + + const delayTime = Math.max(this.deltaStorageDelay, this.deltaStreamDelay, this.storageDelay); + if (delayTime > 0) { + const throttlingError: IThrottlingWarning = { + errorType: ContainerErrorType.throttlingError, + message: `Service busy/throttled: ${error.message}`, + retryAfterSeconds: delayTime, + }; + this.raiseContainerWarning(throttlingError); + } + } + public async reloadContext(): Promise { return this.reloadContextCore().catch((error) => { this.close(CreateContainerError(error)); @@ -1323,6 +1367,7 @@ export class Container extends EventEmitterWithErrorHandling i this.client, ChildLogger.create(this.subLogger, "DeltaManager"), this._canReconnect, + this, ); deltaManager.on("connect", (details: IConnectionDetails, opsBehind?: number) => { @@ -1365,10 +1410,6 @@ export class Container extends EventEmitterWithErrorHandling i this.setConnectionState(ConnectionState.Disconnected, reason); }); - deltaManager.on("throttled", (warning: IThrottlingWarning) => { - this.raiseContainerWarning(warning); - }); - deltaManager.on("readonly", (readonly) => { this.emit("readonly", readonly); }); diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index cc0d85900c93..39e9367e106c 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -11,7 +11,6 @@ import { IDeltaManagerEvents, IDeltaQueue, ICriticalContainerError, - IThrottlingWarning, ContainerErrorType, } from "@fluidframework/container-definitions"; import { assert, performance, TypedEventEmitter } from "@fluidframework/common-utils"; @@ -46,7 +45,7 @@ import { CreateContainerError } from "@fluidframework/container-utils"; import { debug } from "./debug"; import { DeltaQueue } from "./deltaQueue"; import { logNetworkFailure, waitForConnectedState } from "./networkUtils"; -import { Container } from "./container"; +import { Container, RetryFor } from "./container"; const MaxReconnectDelaySeconds = 8; const InitialReconnectDelaySeconds = 1; @@ -61,21 +60,6 @@ const ImmediateNoOpResponse = ""; // eslint-disable-next-line @typescript-eslint/no-unsafe-return export const getRetryDelayFromError = (error: any): number | undefined => error?.retryAfterSeconds; -export function emitThrottlingWarning( - delayTime: number, - error: ICriticalContainerError, - emitter: Container | DeltaManager, -) { - if (delayTime > 0) { - const throttlingError: IThrottlingWarning = { - errorType: ContainerErrorType.throttlingError, - message: `Service busy/throttled: ${error.message}`, - retryAfterSeconds: delayTime, - }; - emitter.emit("throttled", throttlingError); - } -} - function getNackReconnectInfo(nackContent: INackContent) { const reason = `Nack: ${nackContent.message}`; const canRetry = ![403, 429].includes(nackContent.code); @@ -91,11 +75,6 @@ function createReconnectError(prefix: string, err: any) { return error2; } -enum RetryFor { - DeltaStream, - DeltaStorage, -} - export interface IConnectionArgs { mode?: ConnectionMode; fetchOpsFromStorage?: boolean; @@ -113,7 +92,6 @@ export enum ReconnectMode { * but not exposed on the public interface IDeltaManager */ export interface IDeltaManagerInternalEvents extends IDeltaManagerEvents { - (event: "throttled", listener: (error: IThrottlingWarning) => void); (event: "closed", listener: (error?: ICriticalContainerError) => void); } @@ -194,9 +172,6 @@ export class DeltaManager private connectFirstConnection = true; - private deltaStorageDelay: number = 0; - private deltaStreamDelay: number = 0; - // True if current connection has checkpoint information // I.e. we know how far behind the client was at the time of establishing connection private _hasCheckpointSequenceNumber = false; @@ -381,6 +356,7 @@ export class DeltaManager private client: IClient, private readonly logger: ITelemetryLogger, reconnectAllowed: boolean, + private readonly container: Pick, ) { super(); @@ -604,7 +580,7 @@ export class DeltaManager delay = retryDelayFromError ?? Math.min(delay * 2, MaxReconnectDelaySeconds); if (retryDelayFromError !== undefined) { - this.emitDelayInfo(RetryFor.DeltaStream, retryDelayFromError, error); + this.container.emitDelayInfo(RetryFor.DeltaStream, retryDelayFromError, error); } await waitForConnectedState(delay * 1000); } @@ -828,7 +804,7 @@ export class DeltaManager retryAfter = getRetryDelayFromError(origError); if (retryAfter !== undefined && retryAfter >= 0) { - this.emitDelayInfo(RetryFor.DeltaStorage, retryAfter, error); + this.container.emitDelayInfo(RetryFor.DeltaStorage, retryAfter, error); } } @@ -939,25 +915,6 @@ export class DeltaManager } } - private cancelDelayInfo(retryEndpoint: number) { - if (retryEndpoint === RetryFor.DeltaStorage) { - this.deltaStorageDelay = 0; - } else if (retryEndpoint === RetryFor.DeltaStream) { - this.deltaStreamDelay = 0; - } - } - - private emitDelayInfo(retryEndpoint: number, delaySeconds: number, error: ICriticalContainerError) { - if (retryEndpoint === RetryFor.DeltaStorage) { - this.deltaStorageDelay = delaySeconds; - } else if (retryEndpoint === RetryFor.DeltaStream) { - this.deltaStreamDelay = delaySeconds; - } - - const delayTime = Math.max(this.deltaStorageDelay, this.deltaStreamDelay); - emitThrottlingWarning(delayTime, error, this); - } - private readonly opHandler = (documentId: string, messages: ISequencedDocumentMessage[]) => { if (messages instanceof Array) { this.enqueueMessages(messages); @@ -1045,7 +1002,7 @@ export class DeltaManager assert(!readonly || this.connectionMode === "read", "readonly perf with write connection"); this.set_readonlyPermissions(readonly); - this.cancelDelayInfo(RetryFor.DeltaStream); + this.container.cancelDelayInfo(RetryFor.DeltaStream); if (this.closed) { // Raise proper events, Log telemetry event and close connection. @@ -1184,7 +1141,7 @@ export class DeltaManager if (this.reconnectMode === ReconnectMode.Enabled) { const delay = getRetryDelayFromError(error); if (delay !== undefined) { - this.emitDelayInfo(RetryFor.DeltaStream, delay, error); + this.container.emitDelayInfo(RetryFor.DeltaStream, delay, error); await waitForConnectedState(delay * 1000); } @@ -1380,7 +1337,7 @@ export class DeltaManager this.fetching = true; await this.getDeltas(telemetryEventSuffix, from, to, (messages) => { - this.cancelDelayInfo(RetryFor.DeltaStorage); + this.container.cancelDelayInfo(RetryFor.DeltaStorage); this.catchUpCore(messages, telemetryEventSuffix); }); diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index aae937b36405..eefaa758be68 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -7,14 +7,16 @@ import { CreateContainerError } from "@fluidframework/container-utils"; import { IDocumentStorageService } from "@fluidframework/driver-definitions"; import { canRetryOnError, DocumentStorageServiceProxy } from "@fluidframework/driver-utils"; import { ISnapshotTree, IVersion } from "@fluidframework/protocol-definitions"; -import { Container } from "./container"; -import { emitThrottlingWarning, getRetryDelayFromError } from "./deltaManager"; +import { Container, RetryFor } from "./container"; +import { getRetryDelayFromError } from "./deltaManager"; export class RetriableDocumentStorageService extends DocumentStorageServiceProxy { + private static callsWaiting: number = 0; + private static futureTimeTillWait: number = 0; private disposed = false; constructor( internalStorageService: IDocumentStorageService, - private readonly container: Container, + private readonly container: Pick, ) { super(internalStorageService); } @@ -43,12 +45,20 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy return new Promise((resolve) => setTimeout(() => resolve(), timeMs)); } - private async readWithRetry(api: () => Promise, retryLimitSeconds: number = 0): Promise { + private async readWithRetry(api: () => Promise): Promise { let result: T | undefined; - let success = false; + let success: boolean | undefined; + let retryAfter = 0; do { try { result = await api(); + if (success === false) { + RetriableDocumentStorageService.callsWaiting -= 1; + if (RetriableDocumentStorageService.callsWaiting === 0) { + RetriableDocumentStorageService.futureTimeTillWait = 0; + this.container.cancelDelayInfo(RetryFor.Storage); + } + } success = true; } catch (err) { // If it is not retriable, then just throw the error. @@ -56,10 +66,26 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy if (!canRetry || this.disposed) { throw err; } + if (success === undefined) { + // We are going to retry this call. + RetriableDocumentStorageService.callsWaiting += 1; + success = false; + } // If the error is throttling error, then wait for the specified time before retrying. // If the waitTime is not specified, then we start with retrying immediately to max of 8s. - const retryAfter = getRetryDelayFromError(err) ?? Math.min(retryLimitSeconds * 2, 8000); - emitThrottlingWarning(retryAfter, CreateContainerError(err), this.container); + retryAfter = getRetryDelayFromError(err) ?? Math.min(retryAfter * 2, 8000); + let waitTime = 0; + if (RetriableDocumentStorageService.futureTimeTillWait === 0) { + waitTime = retryAfter - RetriableDocumentStorageService.futureTimeTillWait; + RetriableDocumentStorageService.futureTimeTillWait = Date.now() + waitTime; + } else { + waitTime = Date.now() + retryAfter - RetriableDocumentStorageService.futureTimeTillWait; + RetriableDocumentStorageService.futureTimeTillWait += waitTime; + } + if (waitTime > 0) { + this.container.emitDelayInfo(RetryFor.Storage, waitTime, CreateContainerError(err)); + RetriableDocumentStorageService.futureTimeTillWait += waitTime; + } await this.delay(retryAfter); } } while (!success); diff --git a/packages/loader/container-loader/src/test/deltaManager.spec.ts b/packages/loader/container-loader/src/test/deltaManager.spec.ts index a918d504a541..f4ef8346a4cf 100644 --- a/packages/loader/container-loader/src/test/deltaManager.spec.ts +++ b/packages/loader/container-loader/src/test/deltaManager.spec.ts @@ -84,11 +84,16 @@ describe("Loader", () => { ); const client: Partial = { mode: "write", details: { capabilities: { interactive: true } } }; + const container = { + cancelDelayInfo: () => {}, + emitDelayInfo: () => undefined, + }; deltaManager = new DeltaManager( () => service, client as IClient, logger, false, + container, ); deltaManager.attachOpHandler(0, 0, 1, { process: (message) => intendedResult, diff --git a/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts index 21cb7f136ea8..71d836503549 100644 --- a/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts +++ b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts @@ -4,10 +4,8 @@ */ import { strict as assert } from "assert"; -import { IThrottlingWarning } from "@fluidframework/container-definitions"; import { DriverErrorType, IDocumentStorageService } from "@fluidframework/driver-definitions"; import { RetriableDocumentStorageService } from "../retriableDocumentStorageService"; -import { Container } from "../container"; describe("RetriableDocumentStorageService Tests", () => { let retriableStorageService: RetriableDocumentStorageService; @@ -15,8 +13,11 @@ describe("RetriableDocumentStorageService Tests", () => { beforeEach(() => { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions internalService = {} as IDocumentStorageService; - const container = { emit: (event: string, error: IThrottlingWarning) => {} }; - retriableStorageService = new RetriableDocumentStorageService(internalService, container as Container); + const container = { + cancelDelayInfo: () => {}, + emitDelayInfo: () => undefined, + }; + retriableStorageService = new RetriableDocumentStorageService(internalService, container); }); it("Should succeed at first time", async () => { diff --git a/packages/test/functional-tests/src/test/containerRuntime.spec.ts b/packages/test/functional-tests/src/test/containerRuntime.spec.ts index 2c493095a69b..38aaf4e5e7e7 100644 --- a/packages/test/functional-tests/src/test/containerRuntime.spec.ts +++ b/packages/test/functional-tests/src/test/containerRuntime.spec.ts @@ -89,11 +89,16 @@ describe("Container Runtime", () => { ); const client: Partial = { mode: "write", details: { capabilities: { interactive: true } } }; + const container = { + cancelDelayInfo: () => {}, + emitDelayInfo: () => undefined, + }; deltaManager = new DeltaManager( () => service, client as IClient, DebugLogger.create("fluid:testDeltaManager"), false, + container, ); const emitter = new EventEmitter(); From bdf9eae106e0eb7dff61fbba0229b2afe050fa8e Mon Sep 17 00:00:00 2001 From: jatingarg Date: Mon, 30 Nov 2020 21:49:01 -0800 Subject: [PATCH 10/18] pr sugg --- .../src/retriableDocumentStorageService.ts | 22 +++++-------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index eefaa758be68..65252fd8de5f 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -12,7 +12,6 @@ import { getRetryDelayFromError } from "./deltaManager"; export class RetriableDocumentStorageService extends DocumentStorageServiceProxy { private static callsWaiting: number = 0; - private static futureTimeTillWait: number = 0; private disposed = false; constructor( internalStorageService: IDocumentStorageService, @@ -55,15 +54,16 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy if (success === false) { RetriableDocumentStorageService.callsWaiting -= 1; if (RetriableDocumentStorageService.callsWaiting === 0) { - RetriableDocumentStorageService.futureTimeTillWait = 0; this.container.cancelDelayInfo(RetryFor.Storage); } } success = true; } catch (err) { + if (this.disposed) { + throw CreateContainerError("Storage service disposed!!"); + } // If it is not retriable, then just throw the error. - const canRetry = canRetryOnError(err); - if (!canRetry || this.disposed) { + if (!canRetryOnError(err)) { throw err; } if (success === undefined) { @@ -74,18 +74,8 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy // If the error is throttling error, then wait for the specified time before retrying. // If the waitTime is not specified, then we start with retrying immediately to max of 8s. retryAfter = getRetryDelayFromError(err) ?? Math.min(retryAfter * 2, 8000); - let waitTime = 0; - if (RetriableDocumentStorageService.futureTimeTillWait === 0) { - waitTime = retryAfter - RetriableDocumentStorageService.futureTimeTillWait; - RetriableDocumentStorageService.futureTimeTillWait = Date.now() + waitTime; - } else { - waitTime = Date.now() + retryAfter - RetriableDocumentStorageService.futureTimeTillWait; - RetriableDocumentStorageService.futureTimeTillWait += waitTime; - } - if (waitTime > 0) { - this.container.emitDelayInfo(RetryFor.Storage, waitTime, CreateContainerError(err)); - RetriableDocumentStorageService.futureTimeTillWait += waitTime; - } + this.container.emitDelayInfo(RetryFor.Storage, retryAfter, CreateContainerError(err)); + await this.delay(retryAfter); } } while (!success); From 04da12bf36f07bda3822845182ab23ac5746a430 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Mon, 30 Nov 2020 21:55:09 -0800 Subject: [PATCH 11/18] pr sugg --- packages/loader/container-definitions/src/loader.ts | 1 - .../src/prefetchDocumentStorageService.ts | 11 +++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/loader/container-definitions/src/loader.ts b/packages/loader/container-definitions/src/loader.ts index 34af24979245..96982297a0a5 100644 --- a/packages/loader/container-definitions/src/loader.ts +++ b/packages/loader/container-definitions/src/loader.ts @@ -93,7 +93,6 @@ export interface IContainerEvents extends IEvent { (event: "closed", listener: (error?: ICriticalContainerError) => void); (event: "warning", listener: (error: ContainerWarning) => void); (event: "op", listener: (message: ISequencedDocumentMessage) => void); - (event: "throttled", listener: (error: IThrottlingWarning) => void); } /** diff --git a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts index 7e84244b6b7c..5fd556e9f2f3 100644 --- a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts +++ b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts @@ -3,8 +3,6 @@ * Licensed under the MIT License. */ -/* eslint-disable @typescript-eslint/no-floating-promises */ - import { ISnapshotTree, IVersion, @@ -21,6 +19,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy const p = this.internalStorageService.getSnapshotTree(version); if (this.prefetchEnabled) { // We don't care if the prefetch succeed + // eslint-disable-next-line @typescript-eslint/no-floating-promises p.then((tree: ISnapshotTree | null | undefined) => { if (tree === null || tree === undefined) { return; } this.prefetchTree(tree); @@ -38,7 +37,8 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy this.prefetchCache.clear(); } - private async cachedRead(blobId: string): Promise { + // eslint-disable-next-line @typescript-eslint/promise-function-async + private cachedRead(blobId: string): Promise { if (this.prefetchEnabled) { const prefetchedBlobP: Promise | undefined = this.prefetchCache.get(blobId); if (prefetchedBlobP !== undefined) { @@ -57,6 +57,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy for (const blob of secondary) { // We don't care if the prefetch succeed + // eslint-disable-next-line @typescript-eslint/no-floating-promises this.cachedRead(blob); } } @@ -67,6 +68,7 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy if (blobKey.startsWith(".") || blobKey === "header" || blobKey.startsWith("quorum")) { if (blob !== null) { // We don't care if the prefetch succeed + // eslint-disable-next-line @typescript-eslint/no-floating-promises this.cachedRead(blob); } } else if (!blobKey.startsWith("deltas")) { @@ -78,7 +80,8 @@ export class PrefetchDocumentStorageService extends DocumentStorageServiceProxy for (const commit of Object.keys(tree.commits)) { this.getVersions(tree.commits[commit], 1) - .then(async (moduleCommit) => this.getSnapshotTree(moduleCommit[0])) + // eslint-disable-next-line @typescript-eslint/promise-function-async + .then((moduleCommit) => this.getSnapshotTree(moduleCommit[0])) .catch((error) => debug("Ignored cached read error", error)); } From a8490b69f95132d0d174ee7583ee8d791a2ec5da Mon Sep 17 00:00:00 2001 From: jatingarg Date: Mon, 30 Nov 2020 21:57:27 -0800 Subject: [PATCH 12/18] pr sugg --- packages/loader/container-definitions/src/loader.ts | 2 +- .../container-loader/src/prefetchDocumentStorageService.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/loader/container-definitions/src/loader.ts b/packages/loader/container-definitions/src/loader.ts index 96982297a0a5..f796d7e07382 100644 --- a/packages/loader/container-definitions/src/loader.ts +++ b/packages/loader/container-definitions/src/loader.ts @@ -21,7 +21,7 @@ import { import { IResolvedUrl } from "@fluidframework/driver-definitions"; import { IEvent, IEventProvider } from "@fluidframework/common-definitions"; import { IDeltaManager } from "./deltas"; -import { ICriticalContainerError, ContainerWarning, IThrottlingWarning } from "./error"; +import { ICriticalContainerError, ContainerWarning } from "./error"; import { IFluidModule } from "./fluidModule"; import { AttachState } from "./runtime"; diff --git a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts index 5fd556e9f2f3..4c57d7464c6c 100644 --- a/packages/loader/container-loader/src/prefetchDocumentStorageService.ts +++ b/packages/loader/container-loader/src/prefetchDocumentStorageService.ts @@ -2,7 +2,6 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. */ - import { ISnapshotTree, IVersion, From f4b1858254e9aad997276e7e6346c9446a5bc473 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Tue, 1 Dec 2020 15:52:01 -0800 Subject: [PATCH 13/18] pr sugg --- .../loader/container-loader/src/container.ts | 71 ++---------------- .../container-loader/src/deltaManager.ts | 73 +++++++++++++++++-- .../src/retriableDocumentStorageService.ts | 28 +++---- .../src/test/deltaManager.spec.ts | 5 -- .../retriableDocumentStorageService.spec.ts | 6 +- .../src/test/containerRuntime.spec.ts | 5 -- 6 files changed, 86 insertions(+), 102 deletions(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 97851273898b..48163e54100d 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -27,13 +27,11 @@ import { IRuntimeState, ICriticalContainerError, ContainerWarning, - IThrottlingWarning, AttachState, - ContainerErrorType, + IThrottlingWarning, } from "@fluidframework/container-definitions"; import { CreateContainerError, GenericError } from "@fluidframework/container-utils"; import { - LoaderCachingPolicy, IDocumentService, IDocumentStorageService, IFluidResolvedUrl, @@ -93,9 +91,7 @@ import { IConnectionArgs, DeltaManager, ReconnectMode } from "./deltaManager"; import { DeltaManagerProxy } from "./deltaManagerProxy"; import { Loader, RelativeLoader } from "./loader"; import { pkgVersion } from "./packageVersion"; -import { PrefetchDocumentStorageService } from "./prefetchDocumentStorageService"; import { parseUrl, convertProtocolAndAppSummaryToSnapshotTree } from "./utils"; -import { RetriableDocumentStorageService } from "./retriableDocumentStorageService"; const detachedContainerRefSeqNumber = 0; @@ -103,12 +99,6 @@ interface ILocalSequencedClient extends ISequencedClient { shouldHaveLeft?: boolean; } -export enum RetryFor { - DeltaStream, - DeltaStorage, - Storage, -} - export interface IContainerConfig { resolvedUrl?: IResolvedUrl; canReconnect?: boolean; @@ -283,7 +273,7 @@ export class Container extends EventEmitterWithErrorHandling i // Active chaincode and associated runtime private _storageService: IDocumentStorageService | undefined; - private retriableStorageService: RetriableDocumentStorageService | undefined; + private get storageService() { if (this._storageService === undefined) { throw new Error("Attempted to access storageService before it was defined"); @@ -300,9 +290,6 @@ export class Container extends EventEmitterWithErrorHandling i private service: IDocumentService | undefined; private _connectionState = ConnectionState.Disconnected; private readonly _audience: Audience; - private deltaStorageDelay: number = 0; - private deltaStreamDelay: number = 0; - private storageDelay: number = 0; private _context: ContainerContext | undefined; private get context() { @@ -513,8 +500,6 @@ export class Container extends EventEmitterWithErrorHandling i this._deltaManager.close(error); - this.retriableStorageService?.dispose(); - this._protocolHandler?.close(); this._context?.dispose(error !== undefined ? new Error(error.message) : undefined); @@ -752,40 +737,6 @@ export class Container extends EventEmitterWithErrorHandling i this.emit("warning", warning); } - public cancelDelayInfo(retryEndpoint: RetryFor) { - if (retryEndpoint === RetryFor.DeltaStorage) { - this.deltaStorageDelay = 0; - } else if (retryEndpoint === RetryFor.DeltaStream) { - this.deltaStreamDelay = 0; - } else { - this.storageDelay = 0; - } - } - - public emitDelayInfo( - retryEndpoint: RetryFor, - delaySeconds: number, - error: ICriticalContainerError, - ) { - if (retryEndpoint === RetryFor.DeltaStorage) { - this.deltaStorageDelay = delaySeconds; - } else if (retryEndpoint === RetryFor.DeltaStream) { - this.deltaStreamDelay = delaySeconds; - } else { - this.storageDelay = delaySeconds; - } - - const delayTime = Math.max(this.deltaStorageDelay, this.deltaStreamDelay, this.storageDelay); - if (delayTime > 0) { - const throttlingError: IThrottlingWarning = { - errorType: ContainerErrorType.throttlingError, - message: `Service busy/throttled: ${error.message}`, - retryAfterSeconds: delayTime, - }; - this.raiseContainerWarning(throttlingError); - } - } - public async reloadContext(): Promise { return this.reloadContextCore().catch((error) => { this.close(CreateContainerError(error)); @@ -1182,18 +1133,7 @@ export class Container extends EventEmitterWithErrorHandling i } private async getDocumentStorageService(): Promise { - if (this.service === undefined) { - throw new Error("Not attached"); - } - let service = await this.service.connectToStorage(); - - // Enable prefetching for the service unless it has a caching policy set otherwise: - if (this.service.policies?.caching !== LoaderCachingPolicy.NoCaching) { - service = new PrefetchDocumentStorageService(service); - } - - this.retriableStorageService = new RetriableDocumentStorageService(service, this); - return this.retriableStorageService; + return this._deltaManager.connectToStorage(); } private async getDocumentAttributes( @@ -1366,7 +1306,6 @@ export class Container extends EventEmitterWithErrorHandling i this.client, ChildLogger.create(this.subLogger, "DeltaManager"), this._canReconnect, - this, ); deltaManager.on("connect", (details: IConnectionDetails, opsBehind?: number) => { @@ -1409,6 +1348,10 @@ export class Container extends EventEmitterWithErrorHandling i this.setConnectionState(ConnectionState.Disconnected, reason); }); + deltaManager.on("throttled", (warning: IThrottlingWarning) => { + this.raiseContainerWarning(warning); + }); + deltaManager.on("readonly", (readonly) => { this.emit("readonly", readonly); }); diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index 39e9367e106c..fcc0f14aed01 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. */ +import uuid from "uuid"; import { ITelemetryLogger, IEventProvider } from "@fluidframework/common-definitions"; import { IConnectionDetails, @@ -12,6 +13,7 @@ import { IDeltaQueue, ICriticalContainerError, ContainerErrorType, + IThrottlingWarning, } from "@fluidframework/container-definitions"; import { assert, performance, TypedEventEmitter } from "@fluidframework/common-utils"; import { PerformanceEvent, TelemetryLogger, safeRaiseEvent } from "@fluidframework/telemetry-utils"; @@ -19,6 +21,8 @@ import { IDocumentDeltaStorageService, IDocumentService, IDocumentDeltaConnection, + IDocumentStorageService, + LoaderCachingPolicy, } from "@fluidframework/driver-definitions"; import { isSystemType, isSystemMessage } from "@fluidframework/protocol-base"; import { @@ -45,7 +49,8 @@ import { CreateContainerError } from "@fluidframework/container-utils"; import { debug } from "./debug"; import { DeltaQueue } from "./deltaQueue"; import { logNetworkFailure, waitForConnectedState } from "./networkUtils"; -import { Container, RetryFor } from "./container"; +import { RetriableDocumentStorageService } from "./retriableDocumentStorageService"; +import { PrefetchDocumentStorageService } from "./prefetchDocumentStorageService"; const MaxReconnectDelaySeconds = 8; const InitialReconnectDelaySeconds = 1; @@ -92,6 +97,7 @@ export enum ReconnectMode { * but not exposed on the public interface IDeltaManager */ export interface IDeltaManagerInternalEvents extends IDeltaManagerEvents { + (event: "throttled", listener: (error: IThrottlingWarning) => void); (event: "closed", listener: (error?: ICriticalContainerError) => void); } @@ -161,6 +167,9 @@ export class DeltaManager private clientSequenceNumber = 0; private clientSequenceNumberObserved = 0; private closed = false; + private storageService: RetriableDocumentStorageService | undefined; + private readonly deltaStreamDelayId = uuid(); + private readonly deltaStorageDelayId = uuid(); // track clientId used last time when we sent any ops private lastSubmittedClientId: string | undefined; @@ -171,6 +180,8 @@ export class DeltaManager private messageBuffer: IDocumentMessage[] = []; private connectFirstConnection = true; + private readonly idToDelayMap = new Map(); + private maxThrottlingDelay: number = 0; // True if current connection has checkpoint information // I.e. we know how far behind the client was at the time of establishing connection @@ -296,6 +307,22 @@ export class DeltaManager return this._reconnectMode; } + public async connectToStorage(): Promise { + const service = this.serviceProvider(); + if (service === undefined) { + throw new Error("Not attached"); + } + + let storageService = await service.connectToStorage(); + // Enable prefetching for the service unless it has a caching policy set otherwise: + if (service.policies?.caching !== LoaderCachingPolicy.NoCaching) { + storageService = new PrefetchDocumentStorageService(storageService); + } + + this.storageService = new RetriableDocumentStorageService(storageService, this); + return this.storageService; + } + /** * Enables or disables automatic reconnecting. * Will throw an error if reconnectMode set to Never. @@ -356,7 +383,6 @@ export class DeltaManager private client: IClient, private readonly logger: ITelemetryLogger, reconnectAllowed: boolean, - private readonly container: Pick, ) { super(); @@ -580,7 +606,7 @@ export class DeltaManager delay = retryDelayFromError ?? Math.min(delay * 2, MaxReconnectDelaySeconds); if (retryDelayFromError !== undefined) { - this.container.emitDelayInfo(RetryFor.DeltaStream, retryDelayFromError, error); + this.emitDelayInfo(this.deltaStreamDelayId, retryDelayFromError, error); } await waitForConnectedState(delay * 1000); } @@ -804,7 +830,7 @@ export class DeltaManager retryAfter = getRetryDelayFromError(origError); if (retryAfter !== undefined && retryAfter >= 0) { - this.container.emitDelayInfo(RetryFor.DeltaStorage, retryAfter, error); + this.emitDelayInfo(this.deltaStorageDelayId, retryAfter, error); } } @@ -866,7 +892,7 @@ export class DeltaManager return; } this.closed = true; - + this.storageService?.dispose(); this.stopSequenceNumberUpdate(); // This raises "disconnect" event if we have active connection. @@ -915,6 +941,37 @@ export class DeltaManager } } + public cancelDelayInfo(id: string) { + this.idToDelayMap.delete(id); + this.maxThrottlingDelay = Math.max(...this.idToDelayMap.values()); + } + + public emitDelayInfo( + id: string, + delaySeconds: number, + error: ICriticalContainerError, + ) { + let idForMap: string; + if (id !== undefined) { + idForMap = id; + this.idToDelayMap.set(id, delaySeconds); + } else { + idForMap = uuid(); + this.idToDelayMap.set(idForMap, delaySeconds); + } + + const delayTime = Math.max(this.maxThrottlingDelay, delaySeconds); + if (delayTime > 0) { + const throttlingError: IThrottlingWarning = { + errorType: ContainerErrorType.throttlingError, + message: `Service busy/throttled: ${error.message}`, + retryAfterSeconds: delayTime, + }; + this.emit("throttled", throttlingError); + } + return idForMap; + } + private readonly opHandler = (documentId: string, messages: ISequencedDocumentMessage[]) => { if (messages instanceof Array) { this.enqueueMessages(messages); @@ -1002,7 +1059,7 @@ export class DeltaManager assert(!readonly || this.connectionMode === "read", "readonly perf with write connection"); this.set_readonlyPermissions(readonly); - this.container.cancelDelayInfo(RetryFor.DeltaStream); + this.cancelDelayInfo(this.deltaStreamDelayId); if (this.closed) { // Raise proper events, Log telemetry event and close connection. @@ -1141,7 +1198,7 @@ export class DeltaManager if (this.reconnectMode === ReconnectMode.Enabled) { const delay = getRetryDelayFromError(error); if (delay !== undefined) { - this.container.emitDelayInfo(RetryFor.DeltaStream, delay, error); + this.emitDelayInfo(this.deltaStreamDelayId, delay, error); await waitForConnectedState(delay * 1000); } @@ -1337,7 +1394,7 @@ export class DeltaManager this.fetching = true; await this.getDeltas(telemetryEventSuffix, from, to, (messages) => { - this.container.cancelDelayInfo(RetryFor.DeltaStorage); + this.cancelDelayInfo(this.deltaStorageDelayId); this.catchUpCore(messages, telemetryEventSuffix); }); diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index 65252fd8de5f..f0aabd2a3029 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -3,19 +3,18 @@ * Licensed under the MIT License. */ +import uuid from "uuid"; import { CreateContainerError } from "@fluidframework/container-utils"; import { IDocumentStorageService } from "@fluidframework/driver-definitions"; import { canRetryOnError, DocumentStorageServiceProxy } from "@fluidframework/driver-utils"; import { ISnapshotTree, IVersion } from "@fluidframework/protocol-definitions"; -import { Container, RetryFor } from "./container"; -import { getRetryDelayFromError } from "./deltaManager"; +import { DeltaManager, getRetryDelayFromError } from "./deltaManager"; export class RetriableDocumentStorageService extends DocumentStorageServiceProxy { - private static callsWaiting: number = 0; private disposed = false; constructor( internalStorageService: IDocumentStorageService, - private readonly container: Pick, + private readonly deltaManager: Pick, ) { super(internalStorageService); } @@ -46,16 +45,14 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy private async readWithRetry(api: () => Promise): Promise { let result: T | undefined; - let success: boolean | undefined; + let success = false; let retryAfter = 0; + let id: string | undefined; do { try { result = await api(); - if (success === false) { - RetriableDocumentStorageService.callsWaiting -= 1; - if (RetriableDocumentStorageService.callsWaiting === 0) { - this.container.cancelDelayInfo(RetryFor.Storage); - } + if (id !== undefined) { + this.deltaManager.cancelDelayInfo(id); } success = true; } catch (err) { @@ -66,16 +63,13 @@ export class RetriableDocumentStorageService extends DocumentStorageServiceProxy if (!canRetryOnError(err)) { throw err; } - if (success === undefined) { - // We are going to retry this call. - RetriableDocumentStorageService.callsWaiting += 1; - success = false; - } // If the error is throttling error, then wait for the specified time before retrying. // If the waitTime is not specified, then we start with retrying immediately to max of 8s. retryAfter = getRetryDelayFromError(err) ?? Math.min(retryAfter * 2, 8000); - this.container.emitDelayInfo(RetryFor.Storage, retryAfter, CreateContainerError(err)); - + if (id === undefined) { + id = uuid(); + } + this.deltaManager.emitDelayInfo(id, retryAfter, CreateContainerError(err)); await this.delay(retryAfter); } } while (!success); diff --git a/packages/loader/container-loader/src/test/deltaManager.spec.ts b/packages/loader/container-loader/src/test/deltaManager.spec.ts index f4ef8346a4cf..a918d504a541 100644 --- a/packages/loader/container-loader/src/test/deltaManager.spec.ts +++ b/packages/loader/container-loader/src/test/deltaManager.spec.ts @@ -84,16 +84,11 @@ describe("Loader", () => { ); const client: Partial = { mode: "write", details: { capabilities: { interactive: true } } }; - const container = { - cancelDelayInfo: () => {}, - emitDelayInfo: () => undefined, - }; deltaManager = new DeltaManager( () => service, client as IClient, logger, false, - container, ); deltaManager.attachOpHandler(0, 0, 1, { process: (message) => intendedResult, diff --git a/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts index 71d836503549..d01c2c103c93 100644 --- a/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts +++ b/packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts @@ -13,11 +13,11 @@ describe("RetriableDocumentStorageService Tests", () => { beforeEach(() => { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions internalService = {} as IDocumentStorageService; - const container = { + const deltaManager = { cancelDelayInfo: () => {}, - emitDelayInfo: () => undefined, + emitDelayInfo: () => "", }; - retriableStorageService = new RetriableDocumentStorageService(internalService, container); + retriableStorageService = new RetriableDocumentStorageService(internalService, deltaManager); }); it("Should succeed at first time", async () => { diff --git a/packages/test/functional-tests/src/test/containerRuntime.spec.ts b/packages/test/functional-tests/src/test/containerRuntime.spec.ts index 38aaf4e5e7e7..2c493095a69b 100644 --- a/packages/test/functional-tests/src/test/containerRuntime.spec.ts +++ b/packages/test/functional-tests/src/test/containerRuntime.spec.ts @@ -89,16 +89,11 @@ describe("Container Runtime", () => { ); const client: Partial = { mode: "write", details: { capabilities: { interactive: true } } }; - const container = { - cancelDelayInfo: () => {}, - emitDelayInfo: () => undefined, - }; deltaManager = new DeltaManager( () => service, client as IClient, DebugLogger.create("fluid:testDeltaManager"), false, - container, ); const emitter = new EventEmitter(); From 3d120d04900d3bd3c00708e2fab4fb48a96bca93 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Tue, 1 Dec 2020 15:58:37 -0800 Subject: [PATCH 14/18] pr sugg --- packages/loader/container-loader/src/container.ts | 1 - packages/loader/container-loader/src/deltaManager.ts | 11 +---------- .../consensusOrderedCollectionEndToEndTests.spec.ts | 4 ++-- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/loader/container-loader/src/container.ts b/packages/loader/container-loader/src/container.ts index 48163e54100d..7b34fadc6dc0 100644 --- a/packages/loader/container-loader/src/container.ts +++ b/packages/loader/container-loader/src/container.ts @@ -273,7 +273,6 @@ export class Container extends EventEmitterWithErrorHandling i // Active chaincode and associated runtime private _storageService: IDocumentStorageService | undefined; - private get storageService() { if (this._storageService === undefined) { throw new Error("Attempted to access storageService before it was defined"); diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index fcc0f14aed01..f1f4d38c6dd0 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -951,15 +951,7 @@ export class DeltaManager delaySeconds: number, error: ICriticalContainerError, ) { - let idForMap: string; - if (id !== undefined) { - idForMap = id; - this.idToDelayMap.set(id, delaySeconds); - } else { - idForMap = uuid(); - this.idToDelayMap.set(idForMap, delaySeconds); - } - + this.idToDelayMap.set(id, delaySeconds); const delayTime = Math.max(this.maxThrottlingDelay, delaySeconds); if (delayTime > 0) { const throttlingError: IThrottlingWarning = { @@ -969,7 +961,6 @@ export class DeltaManager }; this.emit("throttled", throttlingError); } - return idForMap; } private readonly opHandler = (documentId: string, messages: ISequencedDocumentMessage[]) => { diff --git a/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts b/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts index a2e3c82071b0..2dd30bb34d09 100644 --- a/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts +++ b/packages/test/end-to-end-tests/src/test/consensusOrderedCollectionEndToEndTests.spec.ts @@ -55,12 +55,12 @@ function generate( beforeEach(async () => { // Create a Container for the first client. - container1 = await args.makeTestContainer(testContainerConfig) as IContainer; + container1 = await args.makeTestContainer(testContainerConfig); dataStore1 = await requestFluidObject(container1, "default"); sharedMap1 = await dataStore1.getSharedObject(mapId); // Load the Container that was created by the first client. - container2 = await args.loadTestContainer(testContainerConfig) as IContainer; + container2 = await args.loadTestContainer(testContainerConfig); dataStore2 = await requestFluidObject(container2, "default"); sharedMap2 = await dataStore2.getSharedObject(mapId); From 4d0ef18647d539553db4a2c9a482985b96f22f07 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Wed, 2 Dec 2020 10:21:22 -0800 Subject: [PATCH 15/18] changes --- packages/loader/container-loader/src/deltaManager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index f1f4d38c6dd0..57c708483fb6 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -952,12 +952,12 @@ export class DeltaManager error: ICriticalContainerError, ) { this.idToDelayMap.set(id, delaySeconds); - const delayTime = Math.max(this.maxThrottlingDelay, delaySeconds); - if (delayTime > 0) { + if (delaySeconds > 0 && delaySeconds > this.maxThrottlingDelay) { + this.maxThrottlingDelay = delaySeconds; const throttlingError: IThrottlingWarning = { errorType: ContainerErrorType.throttlingError, message: `Service busy/throttled: ${error.message}`, - retryAfterSeconds: delayTime, + retryAfterSeconds: delaySeconds, }; this.emit("throttled", throttlingError); } From 4081f5e1e512ec93dad644d2bafa4bbcdab0cb0e Mon Sep 17 00:00:00 2001 From: jatingarg Date: Thu, 3 Dec 2020 10:28:39 -0800 Subject: [PATCH 16/18] pr sugg --- packages/loader/container-loader/src/deltaManager.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index 57c708483fb6..ac0cb2a4be06 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -308,6 +308,9 @@ export class DeltaManager } public async connectToStorage(): Promise { + if (this.storageService !== undefined) { + return this.storageService; + } const service = this.serviceProvider(); if (service === undefined) { throw new Error("Not attached"); From 01b2fa21a21e96449b1b451f6ba9e20e3019a362 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Thu, 3 Dec 2020 10:50:15 -0800 Subject: [PATCH 17/18] pr sugg --- packages/loader/container-loader/src/deltaManager.ts | 3 ++- .../container-loader/src/retriableDocumentStorageService.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index ac0cb2a4be06..e1fbbaf07e6c 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -3,7 +3,8 @@ * Licensed under the MIT License. */ -import uuid from "uuid"; +// eslint-disable-next-line import/no-internal-modules +import uuid from "uuid/v4"; import { ITelemetryLogger, IEventProvider } from "@fluidframework/common-definitions"; import { IConnectionDetails, diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index f0aabd2a3029..df4a1889a3f6 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -3,7 +3,8 @@ * Licensed under the MIT License. */ -import uuid from "uuid"; +// eslint-disable-next-line import/no-internal-modules +import uuid from "uuid/v4"; import { CreateContainerError } from "@fluidframework/container-utils"; import { IDocumentStorageService } from "@fluidframework/driver-definitions"; import { canRetryOnError, DocumentStorageServiceProxy } from "@fluidframework/driver-utils"; From 5492291dc0c437372aa7c2c7f6a79b043549bad4 Mon Sep 17 00:00:00 2001 From: jatingarg Date: Thu, 3 Dec 2020 11:00:26 -0800 Subject: [PATCH 18/18] pr sugg --- packages/loader/container-loader/src/deltaManager.ts | 3 +-- .../container-loader/src/retriableDocumentStorageService.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/loader/container-loader/src/deltaManager.ts b/packages/loader/container-loader/src/deltaManager.ts index fcb166770113..eb60c9debee7 100644 --- a/packages/loader/container-loader/src/deltaManager.ts +++ b/packages/loader/container-loader/src/deltaManager.ts @@ -3,8 +3,7 @@ * Licensed under the MIT License. */ -// eslint-disable-next-line import/no-internal-modules -import uuid from "uuid/v4"; +import { v4 as uuid } from "uuid"; import { ITelemetryLogger, IEventProvider } from "@fluidframework/common-definitions"; import { IConnectionDetails, diff --git a/packages/loader/container-loader/src/retriableDocumentStorageService.ts b/packages/loader/container-loader/src/retriableDocumentStorageService.ts index df4a1889a3f6..4ae1c9fd988d 100644 --- a/packages/loader/container-loader/src/retriableDocumentStorageService.ts +++ b/packages/loader/container-loader/src/retriableDocumentStorageService.ts @@ -3,8 +3,7 @@ * Licensed under the MIT License. */ -// eslint-disable-next-line import/no-internal-modules -import uuid from "uuid/v4"; +import { v4 as uuid } from "uuid"; import { CreateContainerError } from "@fluidframework/container-utils"; import { IDocumentStorageService } from "@fluidframework/driver-definitions"; import { canRetryOnError, DocumentStorageServiceProxy } from "@fluidframework/driver-utils";