Infinite loop of updates caused in rare situation with React

1

I’m a novice when it comes to CRDT, so I’m just curious how this could happen given CRDT?

Yjs works great under normal circumstances, but a rare situation with React causes an infinite loop of updates where “A” becomes “AA” then “AAA” and so on. It only happens when using React dev server, not in production. Based on my logs, none of my code is called. The only place where my code inserts text into a document is in Binding and it is not called when the infinite loop happens.

I’m using Yjs, quill, y-quill, y-websocket, react-quill, quill-cursors.

In my app, quill editors are opened and destroyed frequently. This is not a problem. The only problems occur when the dev servers are restarted. Somehow Yjs gets into a state where it starts updating the doc without any user input and each update then triggers another update, creating an infinite loop.

I suspect that this is caused if doc has multiple bindings, but I would still think CRDT would prevent this. Why doesn’t it?

Any insights of what would cause this scenario would be much appreciated?

2

Hi @mattch,

It is perfectly fine to have multiple editors open with y-quill bindings. But it is likely that you forgot to destroy one of them. This might be due to reacts magical hot-reloading feature that replaces components under the hood.

There is likely an infinite loop somewhere in your codebase where binding1 updates an editor, this change is recognized by binding2, which will update the yjs type, which will trigger binding1 again to perform an update on the editor… and so on…

You shouldn’t have multiple bindings to the same editor.

3

Thanks, Kevin! Really appreciate what you have done with Yjs.

Yes, definitely related to the hot-reloading. I have quill wrapped in a functional component and only do the binding when it’s mounted and do a destroy when it’s unmounted. I also wrap it in a useMemo() so it’s not rerendered to prevent cursor repositions.

I suspect the hot-reloading must be re-mounting without ever unmounting the original component. This could cause the same editor to be bound twice.

Is there a way to detect if an editor is already bound (QuillBinding)?

1 Like
4

Not unfortunately not. But you could store the binding instance on the quill editor instance using private name. E.g. quill._yquillBinding?.destroy(); quill._yquillBinding = new QuillBinding(..).

6

@dmonad Thanks for the idea. I was able to get that to work though there was some weirdness with storing/retrieving the binding from the editor.

7

@dmonad Thanks for the suggestion. I implemented it as well as several others approaches to prevent duplicate inserts. I’ve greatly reduced the frequency of it happening, but it still happens once in a while.

I’m sure I’m not binding to the same editor twice, so I’m curious what other mechanism could cause a document to get duplicated?

All my updates to a doc happen with a mergeUpdates. I cache the updates for X amount of time and then do them all at once. Y.mergeUpdates([ydoc, ...ydocUpdates])

Even if React is causing some strange state in the editor, shouldn’t CRDT prevent duplicate updates? Or is there something else I’m missing?

8

Yjs doesn’t duplicate content. That is really not in the nature of the algorithm. Duplication always comes from users inserting the same content several times (e.g. because they populate the content twice).

Even if React is causing some strange state in the editor, shouldn’t CRDT prevent duplicate updates? Or is there something else I’m missing?

Yjs prevents applying duplicative updates. You can apply the same update from a user several times, but the effect will only apply once. However, Yjs can’t prevent duplicative insertions as it thinks these are distinct changes. So my recommendation is to figure out where the duplicative content is inserted.

10

Thanks @dmonad . Yeah, makes total sense. But what’s so baffling is that I’m not doing any inserts directly. Since I don’t have an offline mode, all the doc updates are done on the server in setPersistence bindState, writeState, yDoc.on(‘update’,…). Here’s all the code that does the updates:

In bindState, I retrieve the doc binary and do a:

            yDoc.getText().doc = new Y.Doc()   //Added for extra protection against duplicates.
            Y.applyUpdate(yDoc, yDocBinary)

In writeState and yDoc.on(‘update’), I store the binary updates and do :

            Y.mergeUpdates([yDoc, ...yDocUpdateList])

So somehow the duplicate text is happening via applyUpdate or mergeUpdates.

Again, it’s very rare when it happens. It may be a race condition when an editor is unmounted and mounted again quickly. Can you think of any scenario that would cause the duplicates via applyUpdate or mergeUpdates?

11

Hi @mattch, I had a similar problem here.
If you are using y-quill 0.1.5, try checking if y-quill 0.1.4 works.
I’m not familiar with React, but I had some problems when accidentally making the Quill instance reactive with Vue. Maybe something similar could be happening with React.

12

yDoc.getText().doc = new Y.Doc()

@mattch Don’t do that. This just messes up the generated transactions.

Can you think of any scenario that would cause the duplicates via applyUpdate or mergeUpdates?

As I said before, Yjs doesn’t duplicate content. It is not in the nature of the algorithm.

13

thanks @gustavotoyota . I’m using y-quill 0.1.4.

14

thanks @dmonad . Sorry, I didn’t mean to imply that Yjs is duplicating the content. What I meant to say is that somehow (via a React change in state of the editor) duplicate content/event is sent to Yjs, and Yjs is not recognizing it as duplicate content.

So my question should have been, what situation would cause Yjs to not recognize duplicate content/event such that Y.mergeUpdate would add it to the doc instead of ignoring it?

yDoc.getText().doc = new Y.Doc() This actually greatly reduced the incident of duplicate content because since a newly connected editor connecting thru bindState should never have content (I don’t have an offline mode). It’s always initialized from the binary doc in the DB. Do you still think this is a bad idea? Why?

15

yDoc.getText().doc = new Y.Doc() This actually greatly reduced the incident of duplicate content because since a newly connected editor connecting thru bindState should never have content (I don’t have an offline mode). It’s always initialized from the binary doc in the DB. Do you still think this is a bad idea? Why?

The reasoning behind this is unclear to me. You can’t just bind a type to a different document like this. It doesn’t do what you think it does. I recommend removing these kinds of weird hacks and finding the actual crux of the problem. As I said this operation is not supported and will mess up transactions.

thanks @dmonad . Sorry, I didn’t mean to imply that Yjs is duplicating the content. What I meant to say is that somehow (via a React change in state of the editor) duplicate content/event is sent to Yjs, and Yjs is not recognizing it as duplicate content.

As I said before, Yjs does not recognize duplication of content either. If some outside source inserts content, it counts as an insertion. Every insertion is unique (even if they insert the same content). Yjs can’t automatically figure out whether a similar insertion happens, (or has happened) at some point in the future/past. It doesn’t work like that…

16

Thanks @dmonad , appreciate your help. Yeah, I’d like to get rid of all my hacks too. :slight_smile: Unfortunately, the nature of React’s unpredictable re-rendering causes several corner-cases with quill and yjs. Simply doing a binding during a mount and a destroy during an unmount doesn’t work consistently.

I’ll take another look and see if I can figure it out. Part of the problem is the number of libraries involved: quill, quill-cursors, react-quill, y-quill, y-websocket, and yjs. The other problem is that it seems to be caused by a race condition since it only happens on rare occasions.

17

Hi @dmonad, I think I figured it out. The duplicate text was only happening when the yjs server rebooted. Even though the client didn’t do another bind, it still triggers another bindState on the server.

To fix this, in bindState I added a condition to only initialize a doc if it’s the doc is empty.

const ydocUint = getDoc(room)
if (yDoc.getText().toDelta().length === 0) {
     Y.applyUpdate(yDoc, ydocUint); 
}

What do you think, is this another bad hack? :slight_smile: If so, what do you recommend?

18

@mattch There is likely something else wrong with your code.

As I explained in other threads, you should only populate content once (not once a day, or once a session, just once). Ideally, this happens when a user creates a document. This is the only time when you should populate content (i.e. insert the textual content into the Yjs document - ytext.insert(0, 'initial content')).

Once you have populated the Yjs document, you should retain the Yjs document forever (not just for today, or for this session, retain it forever and never delete it). Whenever the Yjs document is changed, you can encode it to a binary structure (Y.encodeStateAsUpdate(ydoc)) and overwrite the previous version in your persistent database. However, never delete the document or repopulate it, as this will lead to duplication.

You can apply the encoded Yjs document as often as you want, as Yjs updates are idempotent (successive applying of the same update won’t have any effect). Hence, the if-condition is not necessary and shouldn’t change anything.

const ydocUint = getDoc(room)
if (yDoc.getText().toDelta().length === 0) { // not necessary
     Y.applyUpdate(yDoc, ydocUint);  // applying the same `ydocUint` shouldn't have any effect
}

I suggest that you read the documentation on document updates (Document Updates - Yjs Docs) very carefully and convince yourself that updates never duplicate content.

Lastly, I want to say that you should not use the length of the text document as an indication whether content has been populated. For any reason the length could be 0 (e.g. because the user deleted all content). The following code is very dangerous.

// DON'T IMPLEMENT IT LIKE THIS!
const ydocUint = getDoc(room)
if (yDoc.getText().toDelta().length === 0) {
     // whenever the server is restarted, it will repopulate the content as the length is always 0 at the beginning
     ydoc.getText().insert(0, 'initial content')
}
19

thanks @dmonad. Got it. Ironically, this hack seems to have solved my duplicate text issue and everything seems to be working perfectly now. I think this works for me cause I have another hack to reduce the size doc and my unique application requirements.