FluidFramework
FluidFramework copied to clipboard
Use fixed value for summarizer node's SummaryHandle id
Background
This change is aimed at removing the tracking of basePath, localPath and additionalPath from SummarizerNode. These are used in case a node did not change and it's summary is a SummaryHandle. These paths are used to generate the SummaryHandle's id.
There is no need to track these three paths across summaries of this node because they don't change anymore. They make the summarizer node code complicated and hard to understand / make changes. Previously, these paths could change from one summary to another because of 2 reasons:
- When transitioning to writing summary trees under
.channelssub-tree. We have to support loading from snapshot where summaries were not written under.channelssub-tree. This PR adds a fix for that - Regenerate the first summary (i.e., don't use incremental summaries) when loading from such snapshots. - Differential summaries where if a node failed to summarize, we would use the previous summary + a blob where its ops would be stored. This is totally removed now and is not supported.
Description
This PR is one of 2 ways we can acheive removing these paths. The second one is https://github.com/microsoft/FluidFramework/pull/19014.
Note: It does not remove basePath, localPath and additionalPath from SummarizerNode yet. It removes its usage and can be removed. I didn't remove it to make the change smaller and easier to review.
In this PR, instead of tracking the above paths, the summarizer node is given the handle id to be used for SummaryHandle. This does not change thoughout the life time of the node.
When creating a child node, its id and a prefix to be used for handle id is given as a parameter to the parent. The parent node uses this information to build the handle id for the child and passes it during creation.
⯅ @fluid-example/bundle-size-tests: +1.45 KB
| Metric Name | Baseline Size | Compare Size | Size Diff |
|---|---|---|---|
| aqueduct.js | 741.54 KB | 741.79 KB | ⯅ +250 Bytes |
| connectionState.js | 680 Bytes | 680 Bytes | ■ No change |
| containerRuntime.js | 540.51 KB | 540.74 KB | ⯅ +228 Bytes |
| loader.js | 166.8 KB | 166.82 KB | ⯅ +22 Bytes |
| map.js | 559.06 KB | 559.29 KB | ⯅ +239 Bytes |
| matrix.js | 657.29 KB | 657.52 KB | ⯅ +239 Bytes |
| odspDriver.js | 90.24 KB | 90.26 KB | ⯅ +22 Bytes |
| odspPrefetchSnapshot.js | 41.82 KB | 41.83 KB | ⯅ +11 Bytes |
| sharedString.js | 676.01 KB | 676.25 KB | ⯅ +239 Bytes |
| sharedTree.js | 790.65 KB | 790.87 KB | ⯅ +228 Bytes |
| Total Size | 4.28 MB | 4.28 MB | ⯅ +1.45 KB |
Baseline commit: db64250d680465f09f4b0872a5e89417c3538d94
Generated by :no_entry_sign: dangerJS against dfcc1e6c105e528937b9b34e22eb955b592f8c9c
Also, are you confident in existing test coverage? The actual change here is small, but fundamental (switching from fullPath to the passed in handle).