Skip to content

Prevent SharedTree trunk commits from being evicted if they are referenced by branches#15177

Merged
taylorsw04 merged 10 commits into
microsoft:mainfrom
noencke:moldy-branch
Apr 24, 2023
Merged

Prevent SharedTree trunk commits from being evicted if they are referenced by branches#15177
taylorsw04 merged 10 commits into
microsoft:mainfrom
noencke:moldy-branch

Conversation

@noencke
Copy link
Copy Markdown
Contributor

@noencke noencke commented Apr 19, 2023

Description

This PR updates the EditManager's trunk eviction code to retain all commits on the trunk that are currently the base for any outstanding branch/fork. It attempts to do so while preserving separation-of-concerns by adding additional events to the Branch class, but not putting any trunk-specific code into the Branch class (e.g. sequence numbers). It also:

  • Deletes the EditManager module in Core and moves EditManager to be part of SharedTreeCore. This is done since EditManager now has a dependency on Branch, which would otherwise introduce a cyclic dependency. Regardless, it is probably an improvement to the module structure even without the motivation provided by this PR.
  • Made some methods on SharedTreeCore protected instead of public to reduce the API surface for eventual/future subclassers of SharedTreeCore (e.g. "other SharedTrees"). These methods are now tested via a test subclass in shared-tree-core/utils.ts

@github-actions github-actions Bot added area: dds Issues related to distributed data structures area: dds: tree base: main PRs targeted against main branch changeset-present labels Apr 19, 2023
@msfluid-bot
Copy link
Copy Markdown
Collaborator

msfluid-bot commented Apr 19, 2023

@fluid-example/bundle-size-tests: +32 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 437 KB 437 KB +6 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 229.71 KB 229.71 KB +2 Bytes
loader.js 160.81 KB 160.82 KB +6 Bytes
map.js 44.07 KB 44.07 KB +2 Bytes
matrix.js 137.65 KB 137.65 KB +2 Bytes
odspDriver.js 92.35 KB 92.36 KB +6 Bytes
odspPrefetchSnapshot.js 43.58 KB 43.59 KB +4 Bytes
sharedString.js 158.21 KB 158.22 KB +2 Bytes
Total Size 1.38 MB 1.38 MB +32 Bytes

Baseline commit: 1f13911

Generated by 🚫 dangerJS against ec656a7

Comment thread packages/dds/tree/src/shared-tree-core/editManager.ts
Comment thread packages/dds/tree/src/shared-tree-core/editManager.ts Outdated
Comment thread packages/dds/tree/src/shared-tree-core/editManager.ts
Comment thread packages/dds/tree/src/test/shared-tree-core/sharedTreeCore.spec.ts
@github-actions github-actions Bot added public api change Changes to a public API and removed area: dds: tree labels Apr 21, 2023
@noencke noencke marked this pull request as ready for review April 21, 2023 03:45
@noencke noencke requested a review from a team as a code owner April 21, 2023 03:45
@noencke
Copy link
Copy Markdown
Contributor Author

noencke commented Apr 21, 2023

/azp run Build - client packages

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@noencke
Copy link
Copy Markdown
Contributor Author

noencke commented Apr 21, 2023

/azp run repo-policy-check

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@noencke noencke requested a review from taylorsw04 April 22, 2023 04:46
@taylorsw04 taylorsw04 merged commit dd6f6e6 into microsoft:main Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: dds Issues related to distributed data structures base: main PRs targeted against main branch public api change Changes to a public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants