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

Kernel shutdown on page refresh #55

Closed
jtpio opened this issue Jan 27, 2019 · 9 comments
Closed

Kernel shutdown on page refresh #55

jtpio opened this issue Jan 27, 2019 · 9 comments

Comments

@jtpio
Copy link
Member

jtpio commented Jan 27, 2019

Looks like kernels are not shut down when refreshing the page (using the standalone app):

voila-kernels

Seems to be related to how the beforeunload event is triggered:

https://github.com/QuantStack/voila/blob/c437a7fc564aee20e76526fa3c9acc5a5fef326f/voila/static/main.js#L12-L14

@jf---
Copy link

jf--- commented Mar 13, 2019

Ran into the same thing today;

Especially when hitting reload for ~10 repeatedly, this indeed results in 10x new kernels spinning up.
Seems like this behaviour is the results of:

  • kernels not shutting down when there's no more connection to the client
  • when the page is refreshed, a new kernel is started -in some cases- rather than a session being recovered

The reason why the kernels arent shutting down due to:
https://github.com/QuantStack/voila/blob/c437a7fc564aee20e76526fa3c9acc5a5fef326f/voila/app.py#L154-L162

While its too easy to spin up / forkbomb the server, at least processes do get reap'd when the following options are thrown in the mix:

        self.kernel_manager = MappingKernelManager(
            cull_busy=False,
            cull_connected=False,
            cull_idle_timeout=6,
            kernel_info_timeout=12,
           ...

( PR follows shortly )


I noticed @maartenbreddels is working on a SPA PR, ( cool btw 🍹 ) perhaps an idea is to throttle refreshing the SPA page. In that line of thought, perhaps controlling how the page refreshes is relevant ( see here ). For example, a timedelta of 1 second -or 5- would be required before refreshing the page.

Seems to be related to how the beforeunload event is triggered:

interesting find...

@jtpio jtpio mentioned this issue Mar 13, 2019
@jtpio
Copy link
Member Author

jtpio commented Mar 14, 2019

@jf--- Looked at it earlier today with the intent of switching from using voila as a server extension to the standalone app. As a server extension the kernel culling can be set via the notebook config.

        self.kernel_manager = MappingKernelManager(
            cull_busy=False,
            cull_connected=False,
            cull_idle_timeout=6,
            kernel_info_timeout=12,
           ...

( PR follows shortly )

I opened #80, but feel free to comment or add anything missing against this PR.

@jf---
Copy link

jf--- commented Mar 14, 2019

hi @jtpio , so the PR misses out on a few things I think:

First, the cull_connected should be True ( yikes, excuse me... ) and is missing in the PR.
This is critical, since otherwise effectively no culling will take place.

           cull_busy=False,
            cull_connected=True,

Furthermore, I do think that healthy defaults need to be set for cull_idle_time and kernel_info_timeout

            cull_idle_timeout=6,
            kernel_info_timeout=12,

As the PR is as of now, the issue effectively isn't resolved, since the defaults yields the same program state as pre-PR...

That said, IMO there's quite a difference of running voila on a local network, where you might be more conservative in cleaning zombie kernels ( I did run into a situation the process pool did get exhausted / where I could no longer run any shell commands on a test server ).

Therefore since it is currently straightfwd to forkbomb a server, I think the defaults should ackowledge this.

@jtpio
Copy link
Member Author

jtpio commented Mar 14, 2019

cull_idle_timeout and cull_interval should be enough to mitigate the issue when hitting reload several times in a row.
These parameters are already available when using voila as a server extension and the kernels get culled as expected when the page is refreshed or the tab is closed.

Regarding the defaults, it depends on how the dashboards are being used. In some cases enabling cull_connected by default can be considered unexpected.

Overall this will become easier to manage when the voila standalone app supports the traitlets configuration system (cf #84).

@maartenbreddels
Copy link
Member

Coming back to the original issue, I do see that kernel shutdown is being triggered on page refresh with chrome, or with navigation away from a page. It does not trigger it when closing a tab. I think we can tackle this once we have a better idea on session/kernel reuse etc.

@maartenbreddels
Copy link
Member

Fixed in #147

@jf---
Copy link

jf--- commented Jul 29, 2019

@maartenbreddels , would you mind commenting briefly on how the changes in #147 wedge into resolving this issue? I'm curious since on the surface the PR doesn't deal with aspects of session/kernel reuse

@eromoe
Copy link

eromoe commented Apr 21, 2020

How do I prevernt shutdown and restart ?
Because data is big, I would like reuse data in memory .

@maartenbreddels
Copy link
Member

see #527

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

No branches or pull requests

4 participants