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

Outputs stop being streamed #230

Closed
davidbrochart opened this issue Jan 2, 2024 · 4 comments · Fixed by #231 or #232
Closed

Outputs stop being streamed #230

davidbrochart opened this issue Jan 2, 2024 · 4 comments · Fixed by #231 or #232
Labels
bug Something isn't working

Comments

@davidbrochart
Copy link
Collaborator

Description

The following code should stream outputs indefinitely:

from time import sleep

i = 0
while True:
    print(i)
    i += 1
    sleep(1)

But it stops after a while. This happens if there is a 1 second period between prints, and it suggests that there is an issue with dumping the notebook content to disk (which happens 1 second after the last change) at the same time there is a change. If the period is e.g. 0.9s or 1.1s, there is no issue. This doesn't seem to happen with Jupyverse, which has a different implementation than jupyter-collaboration.

Reproduce

  1. Run a notebook with the above code (no need to open multiple views of the notebook).
  2. See the output stream being stopped after a while.

Expected behavior

Outputs should continue being streamed.

Context

  • Operating System and version: Linux Ubuntu 22.04
  • Browser and version: any
  • JupyterLab version: 4.0.10
  • jupyter-collaboration version: 2.0.1
@davidbrochart davidbrochart added the bug Something isn't working label Jan 2, 2024
@davidbrochart
Copy link
Collaborator Author

Looking at the code, it seems that one can observe file changes here, passing a handler to react on file changes here. But the handler is called every second here, even if the file has not changed.
👀

@davidbrochart
Copy link
Collaborator Author

There is a save_content method and a maybe_save_content method, it seems that save_content is never used.
I think the issue happens when the "file change observer" loads the file after maybe_save_content saved the file but not yet updated the _last_modified attribute here, so the condition here evaluates to True and the change is seen as an out-of-band change, which is not the case.

@davidbrochart
Copy link
Collaborator Author

It seems the _update_lock lock prevents the shared document to be modified concurrently from the backend. But when it is modified by a client, a simple check is done here, and if it is locked then the document won't be saved, instead of using the async with self._update_lock: context manager that will wait until the lock gets unlocked before saving the document. This means that there are cases where a document change might not be saved.

@davidbrochart
Copy link
Collaborator Author

Reopening as #231 was not enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
1 participant