Skip to content

Commit

Permalink
[main > release/client/2.0]: Extract serialized blobs from the ISnaps…
Browse files Browse the repository at this point in the history
…hot instead of fetching from driver which could make network calls #21908 (#21924)

## Description

Extract serialized blobs from the ISnapshot which contains blob contents
instead of fetching from driver which could make network calls. This
affects the cases in Data virtualization where initial blob contents for
some trees are missing in the snapshot and therefore driver does not
have the contents cached, so it ends up making a bunch of network calls.

---------

Co-authored-by: tyler-cai-microsoft <[email protected]>
Co-authored-by: Jatin Garg <[email protected]>
  • Loading branch information
3 people authored Jul 24, 2024
1 parent 7802e98 commit efab53e
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 11 deletions.
22 changes: 16 additions & 6 deletions packages/loader/container-loader/src/containerStorageAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
ISnapshotTree,
IVersion,
} from "@fluidframework/driver-definitions/internal";
import { UsageError } from "@fluidframework/driver-utils/internal";
import { isInstanceOfISnapshot, UsageError } from "@fluidframework/driver-utils/internal";
import { ITelemetryLoggerExt } from "@fluidframework/telemetry-utils/internal";

// eslint-disable-next-line import/no-deprecated
Expand All @@ -31,7 +31,7 @@ import type {
ISerializedStateManagerDocumentStorageService,
ISnapshotInfo,
} from "./serializedStateManager.js";
import { convertSnapshotInfoToSnapshot, getDocumentAttributes } from "./utils.js";
import { convertSnapshotInfoToSnapshot } from "./utils.js";

/**
* Stringified blobs from a summary/snapshot tree.
Expand Down Expand Up @@ -175,8 +175,10 @@ export class ContainerStorageAdapter
const localSnapshot =
this.loadingGroupIdSnapshotsFromPendingState[snapshotFetchOptions.loadingGroupIds[0]];
assert(localSnapshot !== undefined, 0x970 /* Local snapshot must be present */);
const attributes = await getDocumentAttributes(this, localSnapshot.baseSnapshot);
snapshot = convertSnapshotInfoToSnapshot(localSnapshot, attributes.sequenceNumber);
snapshot = convertSnapshotInfoToSnapshot(
localSnapshot,
localSnapshot.snapshotSequenceNumber,
);
} else {
if (this._storageService.getSnapshot === undefined) {
throw new UsageError(
Expand Down Expand Up @@ -308,11 +310,19 @@ const redirectTableBlobName = ".redirectTable";
* Get blob contents of a snapshot tree from storage (or, ideally, cache)
*/
export async function getBlobContentsFromTree(
snapshot: ISnapshotTree,
snapshot: ISnapshot | ISnapshotTree,
storage: Pick<IDocumentStorageService, "readBlob">,
): Promise<ISerializableBlobContents> {
const blobs = {};
await getBlobContentsFromTreeCore(snapshot, blobs, storage);
if (isInstanceOfISnapshot(snapshot)) {
const blobContents = snapshot.blobContents;
for (const [id, content] of blobContents.entries()) {
// ArrayBufferLike will not survive JSON.stringify()
blobs[id] = bufferToString(content, "utf8");
}
} else {
await getBlobContentsFromTreeCore(snapshot, blobs, storage);
}
return blobs;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,7 @@ export class SerializedStateManager {
const baseSnapshotTree: ISnapshotTree | undefined = getSnapshotTree(baseSnapshot);
// non-interactive clients will not have any pending state we want to save
if (this.offlineLoadEnabled) {
const snapshotBlobs = await getBlobContentsFromTree(
baseSnapshotTree,
this.storageAdapter,
);
const snapshotBlobs = await getBlobContentsFromTree(baseSnapshot, this.storageAdapter);
const attributes = await getDocumentAttributes(this.storageAdapter, baseSnapshotTree);
this.snapshot = {
baseSnapshot: baseSnapshotTree,
Expand Down Expand Up @@ -459,7 +456,7 @@ export async function getLatestSnapshotInfo(

const baseSnapshotTree: ISnapshotTree | undefined = getSnapshotTree(baseSnapshot);
const snapshotFetchedTime = Date.now();
const snapshotBlobs = await getBlobContentsFromTree(baseSnapshotTree, storageAdapter);
const snapshotBlobs = await getBlobContentsFromTree(baseSnapshot, storageAdapter);
const attributes: IDocumentAttributes = await getDocumentAttributes(
storageAdapter,
baseSnapshotTree,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*!
* Copyright (c) Microsoft Corporation and contributors. All rights reserved.
* Licensed under the MIT License.
*/

import {
describeCompat,
TestDataObjectType,
type ITestDataObject,
} from "@fluid-private/test-version-utils";
import { type IContainerRuntimeOptions } from "@fluidframework/container-runtime/internal";
import type { IContainerRuntimeBase } from "@fluidframework/runtime-definitions/internal";
import { MockLogger } from "@fluidframework/telemetry-utils/internal";
import {
type ITestObjectProvider,
createSummarizer,
createTestConfigProvider,
summarizeNow,
} from "@fluidframework/test-utils/internal";

import { TestSnapshotCache } from "../../testSnapshotCache.js";

describeCompat("Odsp Network calls", "NoCompat", (getTestObjectProvider) => {
// Allow us to control summaries
const runtimeOptions: IContainerRuntimeOptions = {
summaryOptions: {
summaryConfigOverrides: {
state: "disabled",
},
},
};
const configProvider = createTestConfigProvider({
"Fluid.Container.UseLoadingGroupIdForSnapshotFetch2": true,
"Fluid.Container.enableOfflineLoad": true,
});

let provider: ITestObjectProvider;
const testSnapshotCache = new TestSnapshotCache();

beforeEach("setup", async function () {
provider = getTestObjectProvider({ persistedCache: testSnapshotCache });
if (provider.driver.type !== "odsp") {
this.skip();
}
});

const loadingGroupId = "loadingGroupId";
const createDataObjectsWithGroupIds = async (
mainObject: ITestDataObject,
containerRuntime: IContainerRuntimeBase,
) => {
const dataStoreA = await containerRuntime.createDataStore(
TestDataObjectType,
loadingGroupId,
);
const dataStoreB = await containerRuntime.createDataStore(
TestDataObjectType,
loadingGroupId,
);

mainObject._root.set("dataObjectA", dataStoreA.entryPoint);
mainObject._root.set("dataObjectB", dataStoreB.entryPoint);
};

it("Should not make odsp network calls", async () => {
const container = await provider.makeTestContainer({
runtimeOptions,
loaderProps: { configProvider },
});
const mainObject = (await container.getEntryPoint()) as ITestDataObject;
const containerRuntime = mainObject._context.containerRuntime;

// Testing all apis for creating a data store with a loadingGroupId
await createDataObjectsWithGroupIds(mainObject, containerRuntime);
const { summarizer } = await createSummarizer(provider, container, {
loaderProps: { configProvider },
});
await provider.ensureSynchronized();
await summarizeNow(summarizer);

testSnapshotCache.clearCache();
const logger = new MockLogger();
await provider.loadTestContainer({
loaderProps: { configProvider, logger },
});
if (provider.driver.type === "odsp") {
logger.assertMatchNone(
[
{
eventName: "fluid:telemetry:OdspDriver:readDataBlob_end",
},
],
"Should not have any odps network calls",
);
}
});
});

0 comments on commit efab53e

Please sign in to comment.