search

jupyterhub / jupyter-rsession-proxy

Jupyter extensions for running an RStudio rsession proxy
BSD 3-Clause "New" or "Revised" License
118 stars 87 forks source link

add 10s default timeout and environment varariable #128

Closed matuskosut closed 2 years ago

matuskosut commented 2 years ago

Increases start timeout from default 5s to 15s, and allows to override using ENV variables (RSERVER_TIMEOUT, RSESSION_TIMEOUT)

Fixes https://github.com/jupyterhub/jupyter-rsession-proxy/issues/127

welcome[bot] commented 2 years ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

matuskosut commented 2 years ago

hi @yuvipanda, could you review if this approach makes sense? It's pretty occasional that it takes longer than 5 seconds, but when timeout error appears it is confusing for users, because there is no action out. Having longer timeout reduces this issue significantly. Thanks.

zeehio commented 2 years ago

RStudio dumps workspace variables and the list of loaded packages to disk if the session is idle for a long time, and restores them when the user opens it again.

Restoring a large workspace may take a significant amount of time.

In https://docs.rstudio.com/ide/server-pro/access_and_security/running_with_a_proxy.html#check-the-connection-timeout, RStudio recommends a connect timeout of 60 seconds and a read timeout of 20 days.

Some users I worked with, when working with large workspaces (50GB), got timeout errors when restoring the Rastudio session. This is annoying because the running session becomes stuck, and they were unable to save their results after running a long task (that yielded a huge variable). I told them to change their code to avoid this, but still it would be nice to consider larger timeouts (at least 5-10 minutes) for this.

aaraney commented 2 years ago

@Castronova and I ran into a similar issue. This PR was able to resolve our use case.

welcome[bot] commented 2 years ago

Congrats on your first merged pull request in this project! :tada: congrats Thank you for contributing, we are very proud of you! :heart:

ryanlovett commented 2 years ago

Thanks @matuskosut for the PR, @aaraney and @Castronova for testing, and everyone for your patience!