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

Extensions running in the main Theia container should update memory limit for Theia #14511

Open
amisevsk opened this issue Sep 11, 2019 · 8 comments
Labels
kind/enhancement A feature request - must adhere to the feature request template. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@amisevsk
Copy link
Contributor

Is your enhancement related to a problem? Please describe.

While most extensions we currently support have additional dependencies and thus have to run in a sidecar, some extensions run fine in the main Theia container. For these extensions, it would be better to not provision an additional sidecar with memory limit if possible.

This however, could run into an issue where installing plugins into the main Theia container causes memory issues. If a particularly memory-hungry plugin is added, it could cause the container to be killed, crashing the workspace.

Describe the solution you'd like

  1. Revise the plugin meta.yaml schema to support specifying a memory requirement for extensions that do not run in a sidecar

  2. When starting a workspace, add the specified memory requirement for all sidecar plugins to the memory limit given for the main Theia container. For example, if my workspace has

    • Theia, with a default memory limit of 512M
    • vscode-yaml, with a memory request of 256Mi (note, the vscode-yaml extension in this example would not have a containers field, and specify its memory requirement in another way)

    the workspace should be started with a default memory limit of 512 + 256 = 768Mi to prevent memory issues.

Describe alternatives you've considered

Doing nothing also works, but could result in a case where adding a plugin to the workspace causes the Theia container to crash. This would have to be resolved by manually increasing the memory allocated to the main Theia container. This could also be managed by specifying a generous default for Theia to accommodate a few local plugins.

Additional context

#13555

@amisevsk amisevsk added the kind/enhancement A feature request - must adhere to the feature request template. label Sep 11, 2019
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Sep 11, 2019
@slemeur slemeur added team/ide2 and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Sep 12, 2019
@slemeur
Copy link
Contributor

slemeur commented Sep 12, 2019

This is a nice ux improvement IMO. We should also take care of this when the user wants to install a plug-in in an already running workspace (from the IDE - plugin panel).

@che-bot
Copy link
Contributor

che-bot commented Mar 11, 2020

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 11, 2020
@amisevsk
Copy link
Contributor Author

/remove-lifecycle stale

@amisevsk
Copy link
Contributor Author

cc: @davidfestal @l0rd seems that this could be something taken into account with a new devfile spec

@che-bot che-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 11, 2020
@l0rd
Copy link
Contributor

l0rd commented Mar 20, 2020

@amisevsk we definitely need to address this issue. I am not sure how we could address that in the new spec though. I would rather consider it an implementation issue.

@amisevsk
Copy link
Contributor Author

@l0rd the spec-specific fix I was thinking about was that non-sidecar containers currently cannot specify their memory/cpu requirements, so in the example we wouldn't know how much to increase the main Theia container's limit.

@l0rd
Copy link
Contributor

l0rd commented Mar 26, 2020

Ok I see. We currently specify mem/cpu requirements at the container level. For a plugin with no containers we cannot specify the mem/cpu requirements.

@che-bot
Copy link
Contributor

che-bot commented Oct 1, 2020

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 1, 2020
@l0rd l0rd added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

6 participants