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

Feat: Add fast boot #2808

Closed
wants to merge 3 commits into from
Closed

Conversation

SmartManoj
Copy link
Contributor

@SmartManoj SmartManoj commented Jul 5, 2024

What is the problem that this fixes or functionality that this introduces? Does it fix any open issues?

It is separated from #2455. Currently, it takes ~20 seconds to initialize plugins.

image

Give a brief summary of what the PR does, explaining any non-trivial design decisions

Why is initialize_plugins=0 not enough?

  • When the container is stopped, the jupyter server is stopped too. So, I introduced a new sandbox config fast_boot to not stop containers for persistent sandboxes.

Is fast_boot == not initialize plugins?

  • No. For the first time, if one sets initialize_plugins, then "not initialize_plugins" will fail and the sandbox will be stopped. The effect is that the jupyter server will also be stopped. Then one needs to set initialize_plugins again and it will lead to an endless loop.

Copy link
Collaborator

@yufansong yufansong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to close this PR. I don't think this feature is necessary. Using those configs to do trival control on our logic is bad. Will add too much tech debt on codebase.

@SmartManoj
Copy link
Contributor Author

Currently, it takes ~20 seconds to initialize plugins.

Then how to solve this?

@yufansong
Copy link
Collaborator

yufansong commented Jul 11, 2024

Currently, it takes ~20 seconds to initialize plugins.

Then how to solve this?

We are working on refactor the runtime-sandbox-docker architecture now. When it get finished, part of the work of initialization will did in docker when build image. Then it will be much quicker. And we also will deprecated the ssh box logic.

@SmartManoj SmartManoj marked this pull request as draft July 11, 2024 04:25
@yufansong
Copy link
Collaborator

After discussion, close it now.

@yufansong yufansong closed this Jul 11, 2024
@SmartManoj
Copy link
Contributor Author

SmartManoj commented Jul 11, 2024

refactor the runtime-sandbox-docker architecture now.

Is there any issue tracking this?

#2059 (comment)

#2360 (comment)

@yufansong
Copy link
Collaborator

#2404 track our refactor work.

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

Successfully merging this pull request may close these issues.

2 participants