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

fix for report of auto-save not working? #16798

Open
kellyrowland opened this issue Sep 20, 2024 · 7 comments
Open

fix for report of auto-save not working? #16798

kellyrowland opened this issue Sep 20, 2024 · 7 comments
Labels

Comments

@kellyrowland
Copy link

hello - did the fix proposed in https://discourse.jupyter.org/t/auto-save-does-not-always-work/24756 ever get submitted (and merged into) JupyterLab?

I have gotten a similar report from a user on one of our Jupyter instances (JupyterLab 4.2.4, JupyterHub 5.1.0), though we have not been able to reproduce the issue.

@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to new issues that need triage label Sep 20, 2024
@kellyrowland
Copy link
Author

I have been able to reproduce this behavior by turning off my computer's wifi connection, leaving it off for more than 2 minutes, then reconnecting and restarting the server. Any changes to the notebook in the server aren't saved, and the access/modify/change times of the notebook file don't change either.

This doesn't appear to happen on our Jupyter instance running JupyterLab 3.6.x, so I'm wondering if this is caused by the changes introduced in #10156.

@JasonWeill
Copy link
Contributor

@kellyrowland Thank you for opening this issue! It doesn't look like the patch mentioned in that Discourse thread ever made it into mainline code, but we would welcome a pull request to add it in. Please let us know if you need any help or find any other issues. Thanks again!

@kellyrowland
Copy link
Author

@JasonWeill thanks for taking a look here - it's not at all clear to me what was actually suggested in the linked Discourse post, so I'm not sure about a PR just yet.

@vkaidalov-rft could you weigh in on this as the author of #10156? I'm wondering if this check could be removed or if it would be preferable to decrease the save interval multiplier from 10 to something like 1.1.

@kellyrowland
Copy link
Author

Thinking about this a little more, I don't think that changing the save interval multiplier would have any impact, because it seems that the call to _isConnectedCallback() in _setTimer returns false when the connection is lost for longer than the save interval and then fails to return true again when a connection is established (even after the save interval and then 10x the save interval):

[I 2024-09-25 10:45:24.596 ServerApp] Connecting to kernel a009b65b-603e-4dbe-959a-5db85d3c18e6.
[I 2024-09-25 10:47:22.447 ServerApp] Saving file at /global/homes/r/rowlandk/Untitled47.ipynb
[I 2024-09-25 10:47:22.451 ServerApp] Saving Untitled47.ipynb
[W 2024-09-25 10:49:24.505 ServerApp] WebSocket ping timeout after 119936 ms.
[I 2024-09-25 10:49:29.509 ServerApp] Starting buffering for a009b65b-603e-4dbe-959a-5db85d3c18e6:7b22b705-163a-4286-b415-239d9d0b2a5d
[W 2024-09-25 10:49:29.777 TerminalsExtensionApp] WebSocket ping timeout after 119895 ms.
[I 2024-09-25 11:21:27.452 ServerApp] Connecting to kernel a009b65b-603e-4dbe-959a-5db85d3c18e6.
[I 2024-09-25 11:21:27.454 ServerApp] Restoring connection for a009b65b-603e-4dbe-959a-5db85d3c18e6:7b22b705-163a-4286-b415-239d9d0b2a5d
[I 2024-09-25 11:24:37.895 ServerApp] Creating new notebook in /global/homes/r/rowlandk
[I 2024-09-25 11:24:37.906 ServerApp] Saving Untitled48.ipynb
[I 2024-09-25 11:24:39.276 ServerApp] Kernel started: 581074eb-042b-40fe-baa4-74270a83baa1

You can see here that I created Untitled47.ipynb, had it auto-save successfully, then disconnected for a period of time and then I don't see an auto-save, even after connection to the kernel is restored.

I am not familiar with TypeScript, so I'm not sure why this callback is not getting set back to true once the kernel connection has been reestablished. It seems to me like that a proper bugfix would have this working as expected (as opposed to a PR which simply removes the check), but I could really use some help from a developer with more TypeScript experience who could follow the logic in the code.

@mlhenderson
Copy link
Contributor

First, let me describe the problematic behavior here. The scenario is that a client is disconnected from a jupyter server running behind jupyterhub, and the disconnect is long enough that autosave has stopped from the client. After reconnecting, the kernel comes back, and the user gets the false impression that everything is back to normal. However, autosave has not resumed. So at this point, especially if you don't have the filebrowser open and notice that your notebook hasn't been saved, you will continue to work as usual, not realizing that the work has not been saved to disk on the backend.

I also have a simple recipe for replicating the behavior using a vanilla jupyter repo.

  1. git clone https://github.com/jupyterhub/jupyterhub-deploy-docker
  2. cd jupyterhub-deploy-docker/basic-example
  3. docker-compose up
  4. Create a user
  5. Login
  6. Go into settings in the document manager and lower the autosave interval to something shorter than 2 minutes to keep the test short. I suggest 10 seconds or less for testing.
  7. Create a new notebook, add some content, make sure it saved.
  8. Use 'docker network ls' and 'docker container ls' to find the docker network ID and container ID for the hub container.
  9. Disconnect the network of the hub container with 'docker network disconnect <network_id> <container_id>' for at least 2x the autosave interval.
  10. Reconnect the network to the hub container with 'docker network connect <network_id> <container_id>'
  11. Make edits to the notebook, run cells, etc, and observe the age of the file. You can also open a second browser tab to the same notebook and see that the notebook is still read from the saved state before the network disconnect.
  12. Now click the save button in the toolbar. You will be able to save the document, and then after this, autosave will resume.

If you click the save button, you can get the document to save, and that does kick off the autosave again. However, users will not realize they need to do this, and there is not a clear indication anywhere that 1) autosave is no longer happening and 2) a manual save is needed.

This is a situation where information will be lost for unsuspecting users.

@mlhenderson
Copy link
Contributor

@rcthomas @minrk

Also, I should add that this behavior is different for direct to jupyterlab, locally, or in binder. When I tested jupyterlab with and without RTC enabled, without jupyterhub, I did not see this behavior with autosave. I have only been able to replicate it so far using jupyterhub and jupyterlab combined.

@holzman
Copy link
Contributor

holzman commented Oct 24, 2024

Just had this issue affect us here - a user reported that his changes hadn't been saved after he reconnected when his wifi disappeared. (Thanks to @mlhenderson for the clear reproducible instructions above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants