Send one full update from y-websocket #6650
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
from
2cfb371 to
4675077
Compare
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
4 times, most recently
from
1495034 to
f08a168
Compare
juliusknorr
reviewed
Nov 18, 2024
There was a problem hiding this comment.
Gave this a first try and basic syncing is still working fine, also some offline online switching on two browser sessions worked as expected.
One small difference I noticed (without reading the code yet in detail):
- When loading the file it seems we no longer wait until the content is loaded to hide the loading indicator, we start with an empty document and can type before any document state is propagated to us. @max-nextcloud is this expected or can it cause any side effects?
juliusknorr
reviewed
Nov 18, 2024
max-nextcloud
commented
Nov 19, 2024
lib/Service/DocumentService.php
Outdated
| $getStepsSinceVersion = count($querySteps) > 0 ? 0 : $version; | ||
| // If there were any queries in the steps send all steps since last save. | ||
| $getStepsSinceVersion = count($querySteps) > 0 | ||
| ? $document->getLastSavedVersion() |
There was a problem hiding this comment.
this needs to be min($document->getLastSavedVersion(), $version) to make sure we don't skip any steps
There was a problem hiding this comment.
addressed in the next commit by including the document state that matches $document->getLastSavedVersion
There was a problem hiding this comment.
Still using the smaller would mean we have a redundancy in case the client that did the save was out of sync.
|
What's missing:
|
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
from
f08a168 to
e786a18
Compare
Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Keep an internal ydoc tracking updates that came from the server. Send updates, that would sync this doc with the current doc state. Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
The pushed steps are echoed back with all other steps since version immediately. Processing them reduces the size of the following pushes and syncs. Signed-off-by: Max <[email protected]>
It has always been publicly called from the PollingBackend. Signed-off-by: Max <[email protected]>
Updates now include all the local structs that were not yet received from remote. No need to compute a separate update message anymore. Signed-off-by: Max <[email protected]>
Apply document state as a step. Process it like other steps received from the remote. In particular include it in the tracking of steps already applied and set the version accordingly. Signed-off-by: Max <[email protected]>
This was a very inefficient attempt to resync that we did not even process on the client side. Only the steps since the last save may not be enough to get back in sync. However we can expand this by including the document state or storing it as the first step after a save. Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Do not process document state from create response. During create the editor has not been initialized fully and the cursor position is 0 - which is invalid as it is not inside a node with inline content. (It is inside the doc before the initial paragraph.) This also allows processing document state later on in order to recover from out of sync situations. But we do not make use of that yet. Signed-off-by: Max <[email protected]>
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
3 times, most recently
from
ea1a4fb to
91450a2
Compare
I cannot reproduce this right now. |
max-nextcloud
force-pushed
the
fix/y-websocket-with-full-updates
branch
2 times, most recently
from
c0b1bd6 to
3bc7015
Compare
Store the steps that need to be send where we also do the debouncing. They will be updated whenever there is a new message from y-websocket. Signed-off-by: Max <[email protected]>
Signed-off-by: Max <[email protected]>
Signed-off-by: Jonas <[email protected]>
Signed-off-by: Max <[email protected]>
juliusknorr
approved these changes
Nov 26, 2024
|
/backport to stable30 |
|
/backport to stable29 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Always send full diff to server state.
received from the server.
Todo: