Skip to content

Commit

Permalink
Simplify tracked state at each summarizerNode (microsoft#22095)
Browse files Browse the repository at this point in the history
This PR removes some redundant tracked states that are maintained by
each summarizerNode. Following are the list of changes:
- Disable incremental summaries and perform complete summarization
instead (the first time), for the cases where base snapshot was created
in an older document where isolated channels were disabled (i.e.
`.channels` concept was not there in a node path).
[`containerRuntime.ts`]

- `SummaryNode` is removed. SummaryNode was tracking base, additional
etc. path related information for each node, which was convoluting the
code too much. It has been simplified by always providing the path
information to the summarizerNode as a `_summaryHandleId`. Every time a
child node it created, it's path is stored as _summaryHandleId and thus
removes the need for `SummaryNode`.
[`summarizerNode.ts`, `summarizerNodeUtils.ts`].

- `latestSummary:SummaryNode` is replaced with just
`_lastSummaryreferenceSequenceNumber`.

- `PendingSummaryInfo` has been introduced to solve for the other half
of the logic that was fulfilled previously by `SummaryNode` i.e.
maintaining the state of pending summaries submitted so far. Instead of
saving path and referenceSequenceNumber both, we now just maintain
`referenceSequenceNumber`. Saves memory footprint, and simplifies code.
[`summarizerNode.ts`, `summarizerNodeUtils.ts`]

- The above changes also reflected in`summarizerNodeWithGC.ts`.
  • Loading branch information
pragya91 authored Oct 2, 2024
1 parent fb2f4e5 commit 7bfc59a
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 366 deletions.
14 changes: 8 additions & 6 deletions packages/runtime/container-runtime/src/containerRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1711,15 +1711,21 @@ export class ContainerRuntime
});

const loadedFromSequenceNumber = this.deltaManager.initialSequenceNumber;
// If the base snapshot was generated when isolated channels were disabled, set the summary reference
// sequence to undefined so that this snapshot will not be used for incremental summaries. This is for
// back-compat and will rarely happen so its okay to re-summarize everything in the first summary.
const summaryReferenceSequenceNumber =
baseSnapshot === undefined || metadata?.disableIsolatedChannels === true
? undefined
: loadedFromSequenceNumber;
this.summarizerNode = createRootSummarizerNodeWithGC(
createChildLogger({ logger: this.logger, namespace: "SummarizerNode" }),
// Summarize function to call when summarize is called. Summarizer node always tracks summary state.
async (fullTree: boolean, trackState: boolean, telemetryContext?: ITelemetryContext) =>
this.summarizeInternal(fullTree, trackState, telemetryContext),
// Latest change sequence number, no changes since summary applied yet
loadedFromSequenceNumber,
// Summary reference sequence number, undefined if no summary yet
baseSnapshot !== undefined ? loadedFromSequenceNumber : undefined,
summaryReferenceSequenceNumber,
{
// Must set to false to prevent sending summary handle which would be pointing to
// a summary with an older protocol state.
Expand All @@ -1733,10 +1739,6 @@ export class ContainerRuntime
async () => this.garbageCollector.getBaseGCDetails(),
);

if (baseSnapshot) {
this.summarizerNode.updateBaseSummaryState(baseSnapshot);
}

const parentContext = wrapContext(this);

if (snapshotWithContents !== undefined) {
Expand Down
3 changes: 0 additions & 3 deletions packages/runtime/container-runtime/src/dataStoreContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1034,9 +1034,6 @@ export class RemoteFluidDataStoreContext extends FluidDataStoreContext {
this._baseSnapshot = props.snapshot;
this.isSnapshotInISnapshotFormat = false;
}
if (this._baseSnapshot !== undefined) {
this.summarizerNode.updateBaseSummaryState(this._baseSnapshot);
}
}

/*
Expand Down
Loading

0 comments on commit 7bfc59a

Please sign in to comment.