Skip to content

Commit

Permalink
Put Memory Blob Storage Under a Flag
Browse files Browse the repository at this point in the history
  • Loading branch information
anthony-murphy committed Jun 25, 2024
1 parent 3c37ff0 commit d64451a
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 31 deletions.
16 changes: 11 additions & 5 deletions packages/loader/container-loader/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ export class Container
private readonly scope: FluidObject;
private readonly subLogger: ITelemetryLoggerExt;
// eslint-disable-next-line import/no-deprecated
private readonly detachedBlobStorage: IDetachedBlobStorage;
private readonly detachedBlobStorage: IDetachedBlobStorage | undefined;
private readonly protocolHandlerBuilder: ProtocolHandlerBuilder;
private readonly client: IClient;

Expand Down Expand Up @@ -805,7 +805,6 @@ export class Container
// Tracking alternative ways to handle this in AB#4129.
this.options = { ...options };
this.scope = scope;
this.detachedBlobStorage = detachedBlobStorage ?? createMemoryDetachedBlobStorage();
this.protocolHandlerBuilder =
protocolHandlerBuilder ??
((
Expand Down Expand Up @@ -984,6 +983,12 @@ export class Container
this.mc.config.getBoolean("Fluid.Container.summarizeProtocolTree2") ??
options.summarizeProtocolTree;

this.detachedBlobStorage =
detachedBlobStorage ??
(this.mc.config.getBoolean("Fluid.Container.MemoryBlobStorageEnabled") === true
? createMemoryDetachedBlobStorage()
: undefined);

this.storageAdapter = new ContainerStorageAdapter(
this.detachedBlobStorage,
this.mc.logger,
Expand Down Expand Up @@ -1245,7 +1250,8 @@ export class Container
baseSnapshot,
snapshotBlobs,
pendingRuntimeState,
hasAttachmentBlobs: this.detachedBlobStorage.size > 0,
hasAttachmentBlobs:
this.detachedBlobStorage !== undefined && this.detachedBlobStorage.size > 0,
attachmentBlobs: serializeMemoryDetachedBlobStorage(this.detachedBlobStorage),
};
return JSON.stringify(detachedContainerState);
Expand Down Expand Up @@ -1362,7 +1368,7 @@ export class Container
const snapshotWithBlobs = await attachP;
this.serializedStateManager.setInitialSnapshot(snapshotWithBlobs);
if (!this.closed) {
this.detachedBlobStorage.dispose?.();
this.detachedBlobStorage?.dispose?.();
this.handleDeltaConnectionArg(attachProps?.deltaConnection, {
fetchOpsFromStorage: false,
reason: { text: "createDetached" },
Expand Down Expand Up @@ -1811,7 +1817,7 @@ export class Container
tryInitializeMemoryDetachedBlobStorage(this.detachedBlobStorage, attachmentBlobs);
}
assert(
this.detachedBlobStorage.size > 0,
this.detachedBlobStorage !== undefined && this.detachedBlobStorage.size > 0,
0x250 /* "serialized container with attachment blobs must be rehydrated with detached blob storage" */,
);
}
Expand Down
12 changes: 8 additions & 4 deletions packages/loader/container-loader/src/memoryBlobStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ interface MemoryDetachedBlobStorage extends IDetachedBlobStorage {

function isMemoryDetachedBlobStorage(
// eslint-disable-next-line import/no-deprecated
detachedStorage: IDetachedBlobStorage,
detachedStorage: IDetachedBlobStorage | undefined,
): detachedStorage is MemoryDetachedBlobStorage {
return (
isObject(detachedStorage) &&
Expand All @@ -33,16 +33,20 @@ function isMemoryDetachedBlobStorage(

export function serializeMemoryDetachedBlobStorage(
// eslint-disable-next-line import/no-deprecated
detachedStorage: IDetachedBlobStorage,
detachedStorage: IDetachedBlobStorage | undefined,
): string | undefined {
if (detachedStorage.size > 0 && isMemoryDetachedBlobStorage(detachedStorage)) {
if (
detachedStorage !== undefined &&
detachedStorage.size > 0 &&
isMemoryDetachedBlobStorage(detachedStorage)
) {
return detachedStorage.serialize();
}
}

export function tryInitializeMemoryDetachedBlobStorage(
// eslint-disable-next-line import/no-deprecated
detachedStorage: IDetachedBlobStorage,
detachedStorage: IDetachedBlobStorage | undefined,
attachmentBlobs: string,
): void {
if (!isMemoryDetachedBlobStorage(detachedStorage)) {
Expand Down
56 changes: 36 additions & 20 deletions packages/test/test-end-to-end-tests/src/test/blobs.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,7 @@ import {
ContainerMessageType,
DefaultSummaryConfiguration,
} from "@fluidframework/container-runtime/internal";
import {
ConfigTypes,
IConfigProviderBase,
IErrorBase,
IFluidHandle,
} from "@fluidframework/core-interfaces";
import { IErrorBase, IFluidHandle } from "@fluidframework/core-interfaces";
import { Deferred } from "@fluidframework/core-utils/internal";
import { IDocumentServiceFactory } from "@fluidframework/driver-definitions/internal";
import { ReferenceType } from "@fluidframework/merge-tree/internal";
Expand All @@ -33,6 +28,7 @@ import {
ChannelFactoryRegistry,
ITestContainerConfig,
ITestObjectProvider,
createTestConfigProvider,
getContainerEntryPointBackCompat,
waitForContainerConnection,
} from "@fluidframework/test-utils/internal";
Expand All @@ -46,10 +42,6 @@ import {
getUrlFromDetachedBlobStorage,
} from "./mockDetachedBlobStorage.js";

const configProvider = (settings: Record<string, ConfigTypes>): IConfigProviderBase => ({
getRawConfig: (name: string): ConfigTypes => settings[name],
});

function makeTestContainerConfig(registry: ChannelFactoryRegistry): ITestContainerConfig {
return {
runtimeOptions: {
Expand Down Expand Up @@ -447,6 +439,9 @@ function serializationTests({
loaderProps: {
detachedBlobStorage,
options: { summarizeProtocolTree },
configProvider: createTestConfigProvider({
"Fluid.Container.MemoryBlobStorageEnabled": true,
}),
},
});
const container = await loader.createDetachedContainer(
Expand Down Expand Up @@ -499,7 +494,12 @@ function serializationTests({
it("serialize/rehydrate container with blobs", async function () {
const loader = provider.makeTestLoader({
...testContainerConfig,
loaderProps: { detachedBlobStorage },
loaderProps: {
detachedBlobStorage,
configProvider: createTestConfigProvider({
"Fluid.Container.MemoryBlobStorageEnabled": true,
}),
},
});
const serializeContainer = await loader.createDetachedContainer(
provider.defaultCodeDetails,
Expand Down Expand Up @@ -548,10 +548,10 @@ function serializationTests({
loaderProps: {
detachedBlobStorage,
documentServiceFactory,
configProvider: {
getRawConfig: (name) =>
name === "Fluid.Container.RetryOnAttachFailure" ? true : undefined,
},
configProvider: createTestConfigProvider({
"Fluid.Container.MemoryBlobStorageEnabled": true,
"Fluid.Container.RetryOnAttachFailure": true,
}),
},
});
const serializeContainer = await loader.createDetachedContainer(
Expand Down Expand Up @@ -602,13 +602,19 @@ function serializationTests({
ContainerCloseUsageError,
async function () {
// test with and without offline load enabled
const offlineCfg = configProvider({
const offlineCfg = {
"Fluid.Container.enableOfflineLoad": true,
});
};
for (const cfg of [undefined, offlineCfg]) {
const loader = provider.makeTestLoader({
...testContainerConfig,
loaderProps: { detachedBlobStorage, configProvider: cfg },
loaderProps: {
detachedBlobStorage,
configProvider: createTestConfigProvider({
"Fluid.Container.MemoryBlobStorageEnabled": true,
...offlineCfg,
}),
},
});
const detachedContainer = await loader.createDetachedContainer(
provider.defaultCodeDetails,
Expand Down Expand Up @@ -673,7 +679,12 @@ function serializationTests({
async function () {
const loader = provider.makeTestLoader({
...testContainerConfig,
loaderProps: { detachedBlobStorage },
loaderProps: {
detachedBlobStorage,
configProvider: createTestConfigProvider({
"Fluid.Container.MemoryBlobStorageEnabled": true,
}),
},
});
const serializeContainer = await loader.createDetachedContainer(
provider.defaultCodeDetails,
Expand Down Expand Up @@ -722,7 +733,12 @@ function serializationTests({
async function () {
const loader = provider.makeTestLoader({
...testContainerConfig,
loaderProps: { detachedBlobStorage },
loaderProps: {
detachedBlobStorage,
configProvider: createTestConfigProvider({
"Fluid.Container.MemoryBlobStorageEnabled": true,
}),
},
});
let container = await loader.createDetachedContainer(provider.defaultCodeDetails);

Expand Down
6 changes: 4 additions & 2 deletions packages/test/test-utils/src/TestConfigs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ export interface ITestConfigProvider extends IConfigProviderBase {
* Creates a test config provider with the ability to set configs values and clear all config values.
* @internal
*/
export const createTestConfigProvider = (): ITestConfigProvider => {
const settings: Record<string, ConfigTypes> = {};
export const createTestConfigProvider = (
defaults: Record<string, ConfigTypes> = {},
): ITestConfigProvider => {
const settings: Record<string, ConfigTypes> = { ...defaults };
return {
getRawConfig: (name: string): ConfigTypes => settings[name],
set: (key: string, value: ConfigTypes) => {
Expand Down

0 comments on commit d64451a

Please sign in to comment.