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

Yjs websocket reopened #11561

Closed
davidbrochart opened this issue Nov 26, 2021 · 10 comments
Closed

Yjs websocket reopened #11561

davidbrochart opened this issue Nov 26, 2021 · 10 comments

Comments

@davidbrochart
Copy link
Contributor

Description

I can see that in collaborative mode, a new Yjs websocket connection from a client to the server is regularly opened. Is it expected?

Reproduce

  1. Launch jupyter lab --collaborative.
  2. Open a notebook or create a new one.
  3. See in the browser's network analyzer a websocket connection opened at something like api/yjs/notebook:Untitled.ipynb.
  4. Type some text in a cell, see some messages being sent through the websocket.
  5. Wait for some time, you should see other websocket connections being opened. The messages go through the last one, not the previous ones.

Expected behavior

I don't see any reason to open new connections for a given client, but maybe I'm missing something @dmonad ?

Context

  • Operating System and version: Linux Ubuntu 21.04
  • Browser and version: Google Chrome 96.0.4664.45
  • JupyterLab version: 3.2.4
@jtpio
Copy link
Member

jtpio commented Nov 30, 2021

Thanks @davidbrochart for the report.

I am noticing the same behavior when testing on Binder with this gist and JupyterLab 3.2.4: https://gist.github.com/jtpio/6ce26381703355e0ef1da4af742b7f72

image

@davidbrochart
Copy link
Contributor Author

I'm seeing that in #12306. Is it a normal behavior @dmonad ?

@dmonad
Copy link
Member

dmonad commented Mar 31, 2022

A new websocket connection is opened when the current connection closed for some reason (e.g. a connection error) or when no message has been received for ~30 seconds. The client should receive at least its own awareness message from the server. I guess the issue is here. The server doesn't reply the awareness messages to its own client?

We don' necessarily need to do that. Otherwise, we simply have to send some other kind of ping to the client to keep the connection alive.

@davidbrochart
Copy link
Contributor Author

OK, thanks for the explanation. There is no issue with that really, I just found it strange. We have other websocket connections where we don't send any ping, and the connection stays alive, so I'm wondering if it's really necessary?

@dmonad
Copy link
Member

dmonad commented Mar 31, 2022

While pings are not required in WebSockets, or TCP in general, it is recommended to add them. They allow you to detect some network issues that would otherwise go unnoticed. Basically, there are several edge cases when clients don't notice that a TCP connection doesn't work anymore until you send a message.

@davidbrochart
Copy link
Contributor Author

Sounds good, I'll close this issue then. Thanks!

@dmonad
Copy link
Member

dmonad commented Mar 31, 2022

I still think we should fix this, right? This is one extra sync ever 30 seconds.

We probably only need to forward the awareness messages like I do on y-websocket.

@davidbrochart
Copy link
Contributor Author

Let's reopen this issue, although one extra sync every 30 seconds doesn't seem critical 😄
But yes, we can try to implement a more elegant solution to keep the connection alive.

@davidbrochart davidbrochart reopened this Apr 4, 2022
@davidbrochart
Copy link
Contributor Author

OK so the issue is that currently we don't forward the awareness of a client to itself. If we do, there are some UI glitches because the user sees their own cursor, but that's another issue. We should broadcast awareness messages to every client, including the client it originates from.

@davidbrochart
Copy link
Contributor Author

Fixed in y-crdt/ypy-websocket#42.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants