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

RStudio should be shut down on container exit #2878

Open
cboettig opened this issue Oct 13, 2021 · 9 comments
Open

RStudio should be shut down on container exit #2878

cboettig opened this issue Oct 13, 2021 · 9 comments
Assignees
Labels
bug enhancement Issues around improving existing functionality

Comments

@cboettig
Copy link

Bug description

When a student leaves a session idle (e.g. at the end of a class lab session etc), datahub shuts down the instance (naturally). However, it seems that it does not first shutdown the running RStudio session. When the student logs back in, RStudio reports that:

The previous R session was abnormally terminated due to an unexpected crash. You may have lost work as a result.

This warning would be scary but mostly harmless. However, if the student is using an active "Project" in RStudio, RStudio also closes the current open project (presumably as precaution, since restoring the previous state in the event of a real crash could create a vicious cycle). Students may not notice their most recent project is no longer active, until they look for a git menu or such.

For comparison, in the rocker project, the RStudio instance is run by an init service (s6), which handles a shutdown of the service on container exit: https://github.com/rocker-org/rocker-versioned2/blob/master/scripts/install_rstudio.sh#L108-L110

Thanks

Environment & setup

RStudio on r.datahub.berkeley.edu

How to reproduce

  1. Launch RStudio instance on r.datahub.berkeley.edu
  2. Create a project in the New Project menu
  3. Walk away and wait for kubernetes to kill the server
  4. Log back in
@cboettig cboettig added the bug label Oct 13, 2021
@felder felder added the enhancement Issues around improving existing functionality label Oct 13, 2021
@felder felder removed their assignment Oct 13, 2021
@felder
Copy link
Contributor

felder commented Oct 13, 2021

There has been some discussion previously about having rstudio shutdown more gracefully. This is probably something the datahub service team should discuss and add to the feature enhancement roadmap.

@balajialg
Copy link
Contributor

balajialg commented Oct 14, 2021

@cboettig Thanks for making the enhancement request! To understand the severity of the student impact and scope this request accordingly in our backlog, Can you answer the below questions,

  • How many students are affected by this issue? How severe is the impact?

  • Are there existing alternatives that they are using to avoid this scenario?

@ryanlovett Is this a request that could be scoped this month (from a feasibility standpoint)

@cboettig
Copy link
Author

Sure!

  • every student is affected (54 students)
  • It's easy enough to work around; students know to dismiss the warning and log back into their project.

@balajialg
Copy link
Contributor

Thanks, @cboettig! A similar issue is already part of our backlog (more from a documentation perspective)

yuvipanda added a commit to yuvipanda/datahub that referenced this issue Oct 19, 2021
RStudio daemonizes itself when it starts, so it isn't being
passed the SIGTERM when the user pod stops. So it exits
uncleanly, causing corruption and other state issues. tini should
properly reap the rstudio processes during exit.

Ref berkeley-dsep-infra#2878
Ref berkeley-dsep-infra#2891
yuvipanda added a commit to yuvipanda/datahub that referenced this issue Oct 19, 2021
We get it from apt, so it should be somewhere else in $PATH

Ref berkeley-dsep-infra#2878
Ref berkeley-dsep-infra#2891
yuvipanda added a commit to yuvipanda/datahub that referenced this issue Oct 19, 2021
We inherit from the geospatial rocker images, which ship
with the s6 process manager. However, JupyterHub doesn't
launch them using that. We consistently use tini across
all our images instead.

Ref berkeley-dsep-infra#2878
Ref berkeley-dsep-infra#2891
@yuvipanda
Copy link
Contributor

With #2908 and #2905 I think tini is now on all the hubs. This should reduce instances of RStudio state corruption, and hopefully help #2891 and #2819

@yuvipanda
Copy link
Contributor

@cboettig ok, can you re-open this if you encounter that error again?

@cboettig
Copy link
Author

@yuvipanda launched a session at r.datahub.berkeley.edu this morning, switched into an active project, and then just closed the tab. Returning to it this afternoon, the session restarts and again displays the "The previous R session was abnormally terminated ...." message.

Doesn't look like I have permissions to reopen the issue though.

@ryanlovett
Copy link
Collaborator

ryanlovett commented Nov 4, 2021

Apparently the warning is enabled when the "abend" session setting is set in ~/.rstudio/sessions/active/*/session-persistent-state, https://community.rstudio.com/t/restarting-rstudio-server-in-docker-avoid-error-message/10349/2. The warning can reportedly be squelched by changing the session setting, though I agree it'd be best to close RStudio properly.

See rstudio_abend for example.

@cboettig
Copy link
Author

cboettig commented Nov 4, 2021

@ryanlovett thanks! Note that the other side-effect of closing RStudio improperly which is more of a nuisance than the warning itself is the fact that RStudio exits the currently active project (if any), though it keeps open the active file. This means the user often does not immediately realize they are no longer working inside the correct project (i.e. git repo) until later.

(RStudio presumably exits the active project in case it was responsible for the crash; thereby allowing the user to get back into RStudio without getting into an endless crash loop; so that much makes sense. I do think RStudio's crash message would be better if it mentioned this behavior though, or at least also closed the active documents).

shaneknapp pushed a commit to berkeley-dsep-infra/biology-user-image that referenced this issue Sep 6, 2024
RStudio daemonizes itself when it starts, so it isn't being
passed the SIGTERM when the user pod stops. So it exits
uncleanly, causing corruption and other state issues. tini should
properly reap the rstudio processes during exit.

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
shaneknapp pushed a commit to berkeley-dsep-infra/biology-user-image that referenced this issue Sep 6, 2024
We get it from apt, so it should be somewhere else in $PATH

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
shaneknapp pushed a commit to berkeley-dsep-infra/publichealth-user-image that referenced this issue Sep 11, 2024
We inherit from the geospatial rocker images, which ship
with the s6 process manager. However, JupyterHub doesn't
launch them using that. We consistently use tini across
all our images instead.

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
shaneknapp pushed a commit to berkeley-dsep-infra/julia-user-image that referenced this issue Sep 12, 2024
RStudio daemonizes itself when it starts, so it isn't being
passed the SIGTERM when the user pod stops. So it exits
uncleanly, causing corruption and other state issues. tini should
properly reap the rstudio processes during exit.

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
shaneknapp pushed a commit to berkeley-dsep-infra/julia-user-image that referenced this issue Sep 12, 2024
We get it from apt, so it should be somewhere else in $PATH

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
shaneknapp pushed a commit to berkeley-dsep-infra/ischool-user-image that referenced this issue Sep 24, 2024
We inherit from the geospatial rocker images, which ship
with the s6 process manager. However, JupyterHub doesn't
launch them using that. We consistently use tini across
all our images instead.

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
shaneknapp pushed a commit to berkeley-dsep-infra/datahub-user-image that referenced this issue Sep 25, 2024
RStudio daemonizes itself when it starts, so it isn't being
passed the SIGTERM when the user pod stops. So it exits
uncleanly, causing corruption and other state issues. tini should
properly reap the rstudio processes during exit.

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
shaneknapp pushed a commit to berkeley-dsep-infra/datahub-user-image that referenced this issue Sep 25, 2024
We get it from apt, so it should be somewhere else in $PATH

Ref berkeley-dsep-infra/datahub#2878
Ref berkeley-dsep-infra/datahub#2891
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement Issues around improving existing functionality
Projects
None yet
Development

No branches or pull requests

5 participants