Skip to content

Commit

Permalink
[RC2] GC: Port recent telemetry changes (#20795)
Browse files Browse the repository at this point in the history 
Port the following commits to RC2 branch:

* f2b744a
* 059fc18
* 059fc18
* 68e6f92

Our GC early adopters are seeing a handful (less than 10) docs that are
somehow trying to load deleted objects. The changes being ported here
will aid in that analysis, helping to confirm or refute some assumptions
we're having to make based on the current data.

The risk of these changes is low - it's scoped to GC plus some
heavily-tested ContainerRuntime codepaths, and is mostly logging changes
or light refactoring.

---------

Co-authored-by: tyler-cai-microsoft <[email protected]>
  • Loading branch information
@markfields @tyler-cai-microsoft
markfields and tyler-cai-microsoft authored Apr 23, 2024
1 parent dc3f29c commit 675f28f
Show file tree
Hide file tree
Showing 20 changed files with 1,014 additions and 257 deletions.
36 changes: 32 additions & 4 deletions packages/runtime/container-runtime/api-report/container-runtime.api.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ export interface ContainerRuntimeMessage {
// @alpha (undocumented)
export const DefaultSummaryConfiguration: ISummaryConfiguration;

// @alpha
export const DeletedResponseHeaderKey = "wasDeleted";

// @internal
export function detectOutboundReferences(envelope: IEnvelope, addedOutboundReference: (fromNodePath: string, toNodePath: string) => void): void;

Expand Down Expand Up @@ -294,10 +297,10 @@ export type GCFeatureMatrix = {

// @alpha
export const GCNodeType: {
DataStore: string;
SubDataStore: string;
Blob: string;
Other: string;
readonly DataStore: "DataStore";
readonly SubDataStore: "SubDataStore";
readonly Blob: "Blob";
readonly Other: "Other";
};

// @alpha (undocumented)
Expand Down Expand Up @@ -459,6 +462,19 @@ export interface IGCMetadata {
readonly tombstoneTimeoutMs?: number;
}

// @internal
export interface IGCNodeUpdatedProps {
headerData?: RuntimeHeaderData;
node: {
type: (typeof GCNodeType)["DataStore" | "Blob"];
path: string;
};
packagePath?: readonly string[];
reason: "Loaded" | "Changed";
request?: IRequest;
timestampMs?: number;
}

// @alpha (undocumented)
export interface IGCRuntimeOptions {
[key: string]: any;
Expand Down Expand Up @@ -763,6 +779,18 @@ export interface RecentlyAddedContainerRuntimeMessageDetails {
compatDetails: IContainerRuntimeMessageCompatDetails;
}

// @internal
export interface RuntimeHeaderData {
// (undocumented)
allowInactive?: boolean;
// (undocumented)
allowTombstone?: boolean;
// (undocumented)
viaHandle?: boolean;
// (undocumented)
wait?: boolean;
}

// @internal
export enum RuntimeHeaders {
viaHandle = "viaHandle",
Expand Down
150 changes: 107 additions & 43 deletions packages/runtime/container-runtime/src/channelCollection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@ import { AttachState } from "@fluidframework/container-definitions";
import { buildSnapshotTree } from "@fluidframework/driver-utils";
import { assert, Lazy, LazyPromise } from "@fluidframework/core-utils";
import { DataStoreContexts } from "./dataStoreContexts.js";
import { defaultRuntimeHeaderData, RuntimeHeaderData } from "./containerRuntime.js";
import {
DeletedResponseHeaderKey,
RuntimeHeaderData,
defaultRuntimeHeaderData,
} from "./containerRuntime.js";
import {
FluidDataStoreContext,
RemoteFluidDataStoreContext,
Expand All @@ -78,7 +82,8 @@ import {
import {
GCNodeType,
detectOutboundRoutesViaDDSKey,
trimLeadingAndTrailingSlashes,
IGCNodeUpdatedProps,
urlToGCNodePath,
} from "./gc/index.js";
import {
IContainerRuntimeMetadata,
Expand Down Expand Up @@ -262,19 +267,13 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
>();

private readonly contexts: DataStoreContexts;
private readonly aliasedDataStores: Set<string>;

constructor(
private readonly baseSnapshot: ISnapshotTree | undefined,
public readonly parentContext: IFluidParentContext,
baseLogger: ITelemetryBaseLogger,
private readonly gcNodeUpdated: (
nodePath: string,
reason: "Loaded" | "Changed",
timestampMs?: number,
packagePath?: readonly string[],
request?: IRequest,
headerData?: RuntimeHeaderData,
) => void,
private readonly gcNodeUpdated: (props: IGCNodeUpdatedProps) => void,
private readonly isDataStoreDeleted: (nodePath: string) => boolean,
private readonly aliasMap: Map<string, string>,
provideEntryPoint: (runtime: ChannelCollection) => Promise<FluidObject>,
Expand All @@ -291,6 +290,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
"",
this.parentContext.IFluidHandleContext,
);
this.aliasedDataStores = new Set(aliasMap.values());

// Extract stores stored inside the snapshot
const fluidDataStores = new Map<string, ISnapshotTree>();
Expand Down Expand Up @@ -520,6 +520,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
this.parentContext.addedGCOutboundReference?.(this.containerRuntimeHandle, handle);

this.aliasMap.set(aliasMessage.alias, context.id);
this.aliasedDataStores.add(context.id);
context.setInMemoryRoot();
return true;
}
Expand Down Expand Up @@ -850,17 +851,18 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {

// Notify that a GC node for the data store changed. This is used to detect if a deleted data store is
// being used.
this.gcNodeUpdated(
`/${envelope.address}`,
"Changed",
message.timestamp,
context.isLoaded ? context.packagePath : undefined,
);
this.gcNodeUpdated({
node: { type: "DataStore", path: `/${envelope.address}` },
reason: "Changed",
timestampMs: message.timestamp,
packagePath: context.isLoaded ? context.packagePath : undefined,
});
}

public async getDataStore(
private async getDataStore(
id: string,
requestHeaderData: RuntimeHeaderData,
originalRequest: IRequest,
): Promise<FluidDataStoreContext> {
const headerData = { ...defaultRuntimeHeaderData, ...requestHeaderData };
if (
Expand All @@ -870,13 +872,15 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
"Requested",
"getDataStore",
requestHeaderData,
originalRequest,
)
) {
// The requested data store has been deleted by gc. Create a 404 response exception.
const request: IRequest = { url: id };
throw responseToException(
createResponseError(404, "DataStore was deleted", request),
request,
createResponseError(404, "DataStore was deleted", originalRequest, {
[DeletedResponseHeaderKey]: true,
}),
originalRequest,
);
}

Expand Down Expand Up @@ -920,28 +924,69 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
* Checks if the data store has been deleted by GC. If so, log an error.
* @param id - The data store's id.
* @param context - The data store context.
* @param deletedLogSuffix - Whether it was Changed or Requested (will go into the eventName)
* @param callSite - The function name this is called from.
* @param requestHeaderData - The request header information to log if the data store is deleted.
* @param originalRequest - The original request (could be for a child of the DataStore)
* @returns true if the data store is deleted. Otherwise, returns false.
*/
private checkAndLogIfDeleted(
id: string,
context: FluidDataStoreContext | undefined,
deletedLogSuffix: string,
deletedLogSuffix: "Changed" | "Requested",
callSite: string,
requestHeaderData?: RuntimeHeaderData,
originalRequest?: IRequest,
) {
const dataStoreNodePath = `/${id}`;
if (!this.isDataStoreDeleted(dataStoreNodePath)) {
return false;
}

const idToLog =
originalRequest !== undefined
? urlToGCNodePath(originalRequest.url)
: dataStoreNodePath;

// Log the package details asynchronously since getInitialSnapshotDetails is async
const recentelyDeletedContext = this.contexts.getRecentlyDeletedContext(id);
if (recentelyDeletedContext !== undefined) {
recentelyDeletedContext
.getInitialSnapshotDetails()
.then((details) => {
return details.pkg.join("/");
})
.then(
(pkg) => ({ pkg, error: undefined }),
(error) => ({ pkg: undefined, error }),
)
.then(({ pkg, error }) => {
this.mc.logger.sendTelemetryEvent(
{
eventName: `GC_DeletedDataStore_PathInfo`,
...tagCodeArtifacts({
id: idToLog,
pkg,
}),
callSite,
},
error,
);
})
.catch(() => {});
}

this.mc.logger.sendErrorEvent({
eventName: `GC_Deleted_DataStore_${deletedLogSuffix}`,
...tagCodeArtifacts({ id }),
...tagCodeArtifacts({ id: idToLog }),
callSite,
headers: JSON.stringify(requestHeaderData),
exists: context !== undefined,
details: {
url: originalRequest?.url,
headers: JSON.stringify(originalRequest?.headers),
aliased: this.aliasedDataStores.has(id),
},
});
return true;
}
Expand Down Expand Up @@ -1198,14 +1243,39 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
continue;
}
const dataStoreId = pathParts[1];
assert(this.contexts.has(dataStoreId), 0x2d7 /* No data store with specified id */);
assert(
this.contexts.has(dataStoreId),
"updateUnusedRoutes: No data store with specified id",
);
// Delete the contexts of unused data stores.
this.contexts.delete(dataStoreId);
// Delete the summarizer node of the unused data stores.
this.parentContext.deleteChildSummarizerNode?.(dataStoreId);
}
}

private deleteChild(dataStoreId: string) {
const dataStoreContext = this.contexts.get(dataStoreId);
assert(dataStoreContext !== undefined, 0x2d7 /* No data store with specified id */);

if (dataStoreContext.isLoaded) {
this.mc.logger.sendTelemetryEvent({
eventName: "GC_DeletingLoadedDataStore",
...tagCodeArtifacts({
id: dataStoreId,
pkg: dataStoreContext.packagePath.join("/"),
}),
});
}

dataStoreContext.delete();

// Delete the contexts of sweep ready data stores.
this.contexts.delete(dataStoreId);
// Delete the summarizer node of the sweep ready data stores.
this.parentContext.deleteChildSummarizerNode?.(dataStoreId);
}

/**
* Delete data stores and its objects that are sweep ready.
* @param sweepReadyDataStoreRoutes - The routes of data stores and its objects that are sweep ready and should
Expand Down Expand Up @@ -1239,12 +1309,7 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
continue;
}

dataStoreContext.delete();

// Delete the contexts of sweep ready data stores.
this.contexts.delete(dataStoreId);
// Delete the summarizer node of the sweep ready data stores.
this.parentContext.deleteChildSummarizerNode?.(dataStoreId);
this.deleteChild(dataStoreId);
}
return Array.from(sweepReadyDataStoreRoutes);
}
Expand Down Expand Up @@ -1282,8 +1347,9 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
*/
private async getOutboundRoutes(): Promise<string[]> {
const outboundRoutes: string[] = [];
// Getting this information is a performance optimization that reduces network calls for virtualized datastores
for (const [contextId, context] of this.contexts) {
const isRootDataStore = await context.isRoot();
const isRootDataStore = await context.isRoot(this.aliasedDataStores);
if (isRootDataStore) {
outboundRoutes.push(`/${contextId}`);
}
Expand Down Expand Up @@ -1344,31 +1410,29 @@ export class ChannelCollection implements IFluidDataStoreChannel, IDisposable {
headerData.allowInactive = request.headers[AllowInactiveRequestHeaderKey];
}

// We allow Tombstone requests for sub-DataStore objects
// We allow Tombstone/Inactive requests for sub-DataStore objects
if (requestForChild) {
headerData.allowTombstone = true;
headerData.allowInactive = true;
}

await this.waitIfPendingAlias(id);
const internalId = this.internalId(id);
const dataStoreContext = await this.getDataStore(internalId, headerData);
const dataStoreContext = await this.getDataStore(internalId, headerData, request);

// Remove query params, leading and trailing slashes from the url. This is done to make sure the format is
// the same as GC nodes id.
const urlWithoutQuery = trimLeadingAndTrailingSlashes(request.url.split("?")[0]);
// Get the initial snapshot details which contain the data store package path.
const details = await dataStoreContext.getInitialSnapshotDetails();

// Note that this will throw if the data store is inactive or tombstoned and throwing on incorrect usage
// is configured.
this.gcNodeUpdated(
`/${urlWithoutQuery}`,
"Loaded",
undefined /* timestampMs */,
details.pkg,
// When notifying GC of this node being loaded, we only indicate the DataStore itself, not the full subDataStore url if applicable.
// This is in case the url is to a route that Fluid doesn't understand or track for GC (e.g. if suited for a custom request handler)
this.gcNodeUpdated({
node: { type: "DataStore", path: `/${id}` },
reason: "Loaded",
packagePath: details.pkg,
request,
headerData,
);
});

const dataStore = await dataStoreContext.realize();

const subRequest = requestParser.createSubRequest(1);
Expand Down
Loading

0 comments on commit 675f28f

Please sign in to comment.