Skip to content

Comments

Improve asymptotics of merge-tree reconnect#11220

Merged
Abe27342 merged 27 commits intomicrosoft:mainfrom
Abe27342:reconnect-asymptotics-improvement
Aug 15, 2022
Merged

Improve asymptotics of merge-tree reconnect#11220
Abe27342 merged 27 commits intomicrosoft:mainfrom
Abe27342:reconnect-asymptotics-improvement

Conversation

@Abe27342
Copy link
Contributor

@Abe27342 Abe27342 commented Jul 21, 2022

Description

The previous implementation of regeneratePendingOp necessitated a linear walk over segments of the merge tree.
This strategy can be improved by augmenting the partialLengths structures stored at each block with information about unacked segments.
However, that information isn't particularly cheap or straightforward to update: it could be done at the same asymptotic cost, but would likely add some undesirable constant factors to the partial lengths bookkeeping (which already needs to be updated on each insert/delete/structural change).
As a compromise, this adds the ability to optionally compute local partial length information to PartialSequenceLengths, with the restriction that it's invalidated on update.
This is sufficient for the reconnect flow, which regenerates all outstanding local ops upon reconnection without interleaving new ops.

Overall, for a merge tree with N segments and some maximum collab window size, this reduces the cost of rebasing M local ops from O(NM) to O(N + Mlog(N)).

@github-actions github-actions bot added area: dds Issues related to distributed data structures base: main PRs targeted against main branch labels Jul 21, 2022
@Abe27342 Abe27342 marked this pull request as ready for review July 29, 2022 18:36
@Abe27342 Abe27342 requested a review from a team as a code owner July 29, 2022 18:36
@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Jul 29, 2022

@fluid-example/bundle-size-tests: +7.28 KB
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 389.93 KB 392.36 KB +2.43 KB
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 191.92 KB 191.92 KB No change
loader.js 151.12 KB 151.12 KB No change
map.js 42.63 KB 42.63 KB No change
matrix.js 129.27 KB 131.7 KB +2.43 KB
odspDriver.js 150.23 KB 150.23 KB No change
odspPrefetchSnapshot.js 38.39 KB 38.39 KB No change
sharedString.js 149.91 KB 152.34 KB +2.43 KB
Total Size 1.25 MB 1.25 MB +7.28 KB

Baseline commit: dca1cbe

Generated by 🚫 dangerJS against 3582751

@github-actions github-actions bot added the public api change Changes to a public API label Aug 1, 2022
@anthony-murphy
Copy link
Contributor

there is another place we do a similar costly walk: packages\dds\matrix\src\permutationvector.ts would be good to ensure we can support that case as well.

Copy link
Contributor

@anthony-murphy anthony-murphy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. added some comments that are mainly around maintainability here, but logic looks sound. please also get an issue filed for the matrix usage.

@Abe27342
Copy link
Contributor Author

Abe27342 commented Aug 12, 2022

looks good. added some comments that are mainly around maintainability here, but logic looks sound. please also get an issue filed for the matrix usage.

Filed AB#1577 for this. Looks not too bad.

@Abe27342 Abe27342 merged commit b9d5743 into microsoft:main Aug 15, 2022
@github-actions
Copy link
Contributor

This commit is queued for merging with the next branch! Please ignore this PR for now. Contact @microsoft/fluid-cr-infra for help.

@Abe27342 Abe27342 deleted the reconnect-asymptotics-improvement branch August 15, 2022 15:19
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