Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SharedInterval Throw on Reconnect Attempt #9776

Conversation

anthony-murphy
Copy link
Contributor

The current code is not safe. It doesn't do any rebasing of the operation it just naively resubmits it. This can lead to data corruption, so better to just fail in this case.

related to #8739

@anthony-murphy anthony-murphy requested a review from a team as a code owner April 7, 2022 17:17
@anthony-murphy anthony-murphy requested a review from pleath April 7, 2022 17:17
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: sharedstring labels Apr 7, 2022
@anthony-murphy anthony-murphy changed the title Remove IFluidObject from Default IFluidHandle Generic SharedInterval Throw on Reconnect Attempt Apr 7, 2022
@anthony-murphy anthony-murphy reopened this Apr 7, 2022
@github-actions github-actions bot added the base: main PRs targeted against main branch label Apr 7, 2022
// we don't know how to rebase these operations, so if any other op has come in
// we will fail.
if(this.lastProcessedSeq !== mapLocalMetadata?.lastProcessedSeq) {
throw new Error("SharedInterval does not support reconnect in presence of external changes");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'd forgotten about the read-write transition which happens frequently and depends on reconnect. so i added some simple logic that tracks the last op seq, and allows resend if there are no other changes targeting this dds, basically this allows reconnect as long as no one else is doing anything

@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +1.03 KB
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 386.59 KB 387.11 KB +526 Bytes
containerRuntime.js 193.14 KB 193.14 KB No change
loader.js 159.68 KB 159.68 KB No change
map.js 208.23 KB 208.23 KB No change
matrix.js 295.68 KB 295.68 KB No change
odspDriver.js 183 KB 183 KB No change
odspPrefetchSnapshot.js 76.72 KB 76.72 KB No change
sharedString.js 315.68 KB 316.19 KB +526 Bytes
Total Size 1.81 MB 1.81 MB +1.03 KB

Baseline commit: 6bc6e74

Generated by 🚫 dangerJS against 046436e

@anthony-murphy anthony-murphy merged commit 867e337 into microsoft:main Apr 7, 2022
@anthony-murphy anthony-murphy deleted the sharedinterval-throw-on-reconnect branch April 7, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: sharedstring area: dds Issues related to distributed data structures base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants