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.

Sign up for GitHub

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

Jump to bottom

Add ballooning monitor for VMs #945

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add ballooning monitor for VMs #945

wants to merge 7 commits into from
+84 −7

Conversation

slakkala
Copy link
Contributor

@slakkala slakkala commented Jan 9, 2025 •
edited
Loading

Description of changes

Add ballooning monitor and monitor all microvms that have a balloon size defined.
Enable balloon for app VMs, with half of their memory made dynamic.

microvm.nix PR: astro/microvm.nix#324
ghaf-mem-manager PR: tiiuae/ghafpkgs#19

Checklist for things done

  • Summary of the proposed changes in the PR description
  • More detailed description in the commit message(s)
  • Commits are squashed into relevant entities - avoid a lot of minimal dev time commits in the PR
  • Contribution guidelines followed
  • Ghaf documentation updated with the commit - https://tiiuae.github.io/ghaf/
  • PR linked to architecture documentation and requirement(s) (ticket id)
  • Test procedure described (or includes tests). Select one or more:
    • Tested on Lenovo X1 x86_64
    • Tested on Jetson Orin NX or AGX aarch64
    • Tested on Polarfire riscv64
  • Author has run make-checks and it passes
  • All automatic Github Action checks pass - see actions
  • Author has added reviewers and removed PR draft status
  • Change requires full re-installation
  • Change can be updated with nixos-rebuild ... switch

Instructions for Testing

  • List all targets that this applies to:
  • Is this a new feature
    • List the test steps to verify:
  • If it is an improvement how does it impact existing functionality?

@slakkala slakkala temporarily deployed to internal-build-workflow January 9, 2025 09:13 — with GitHub Actions Inactive
@slakkala slakkala marked this pull request as draft January 9, 2025 09:13
@slakkala slakkala force-pushed the dev/balloon-monitor branch from 603800c to 289a00d Compare January 9, 2025 09:15
@slakkala slakkala temporarily deployed to internal-build-workflow January 9, 2025 09:15 — with GitHub Actions Inactive
@slakkala slakkala force-pushed the dev/balloon-monitor branch from 289a00d to 57b2326 Compare January 16, 2025 09:39
@slakkala slakkala temporarily deployed to internal-build-workflow January 16, 2025 09:39 — with GitHub Actions Inactive
@ktusawrk
Copy link
Collaborator

ghaf-pre-merge-pipeline build failed because of a glitch in the HW test agent, not because of this PR.

@slakkala slakkala force-pushed the dev/balloon-monitor branch from 57b2326 to 9bc131b Compare January 17, 2025 13:33
@slakkala slakkala temporarily deployed to internal-build-workflow January 17, 2025 13:33 — with GitHub Actions Inactive
@slakkala
Copy link
Contributor Author

I'd like some preliminary testing results on this; just normal usage mixed with heavy web apps in browser.

@slakkala slakkala added the Needs Testing CI Team to pre-verify label Jan 20, 2025
@leivos-unikie
Copy link
Contributor

I will do some preliminary testing on this.

@leivos-unikie
Copy link
Contributor

leivos-unikie commented Jan 20, 2025 •
edited
Loading

I made some testing on this. Opened all apps. Then opened 3 chrome windows, run youtube video in one, New York Times in one, played online video game in one. This caused the original memory capacity (2.79G) of chrome-vm to be exceeded but then obviously memory ballooning works since it dynamically increased the available memory in chrome-vm to around 4G. In addition the extra memory was released from chrome-vm when I closed the chrome windows.

@leivos-unikie
Copy link
Contributor

I ran also ci-test-automation performance tests on this. Results are ok.

@leivos-unikie leivos-unikie removed the Needs Testing CI Team to pre-verify label Jan 20, 2025
brianmcgillion
brianmcgillion reviewed Jan 23, 2025
View reviewed changes
flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
modules/microvm/virtualization/microvm/adminvm.nix Outdated Show resolved Hide resolved
modules/microvm/virtualization/microvm/appvm.nix
@@ -168,7 +168,8 @@ let

microvm = {
optimize.enable = false;
mem = vm.ramMb;
mem = vm.ramMb / 2;
balloonMem = vm.ramMb / 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this just a test case? of is it an effort at optimization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bit of both. The current plan is to only enable ballooning for appvms; but the 50-50 split is just a guess.

modules/reference/appvms/comms.nix Outdated Show resolved Hide resolved
@slakkala slakkala force-pushed the dev/balloon-monitor branch from 9bc131b to c123e12 Compare January 24, 2025 10:44
@slakkala slakkala temporarily deployed to internal-build-workflow January 24, 2025 10:44 — with GitHub Actions Inactive
@slakkala slakkala force-pushed the dev/balloon-monitor branch from c123e12 to 2c74f98 Compare January 24, 2025 10:47
@slakkala slakkala temporarily deployed to internal-build-workflow January 24, 2025 10:47 — with GitHub Actions Inactive
@slakkala slakkala temporarily deployed to internal-build-workflow January 24, 2025 14:05 — with GitHub Actions Inactive
@slakkala slakkala force-pushed the dev/balloon-monitor branch from 1e94bd5 to e537741 Compare January 27, 2025 10:00
@slakkala slakkala temporarily deployed to internal-build-workflow January 27, 2025 10:00 — with GitHub Actions Inactive
@slakkala slakkala temporarily deployed to internal-build-workflow January 27, 2025 10:06 — with GitHub Actions Inactive
@slakkala
Add ballooning monitor
83aabd5 
Add ghaf-memory-monitor and start it for every VM that has a balloon size defined.

Signed-off-by: Santtu Lakkala <[email protected]>
@slakkala
Relay minimum and maximum memory sizes to balloon monitor
9789271 
Signed-off-by: Santtu Lakkala <[email protected]>
@slakkala
Adapt to dependency changes
e76cdf0 
Signed-off-by: Santtu Lakkala <[email protected]>
@slakkala
Move mem-manager to ghafpkgs
e1a1959 
Signed-off-by: Santtu Lakkala <[email protected]>
@slakkala
ghafpkgs intergrated, microvm PR updated
e1861b0 
Signed-off-by: Santtu Lakkala <[email protected]>
@slakkala
Only use ballooning for appvms.
09a4d32 
Signed-off-by: Santtu Lakkala <[email protected]>
@slakkala slakkala force-pushed the dev/balloon-monitor branch from 6ee55cc to 7071c24 Compare January 27, 2025 13:30
@slakkala slakkala temporarily deployed to internal-build-workflow January 27, 2025 13:30 — with GitHub Actions Inactive
@slakkala slakkala changed the title WIP: Add ballooning monitor for VMs Add ballooning monitor for VMs Jan 27, 2025
@slakkala slakkala marked this pull request as ready for review January 27, 2025 13:33
@slakkala slakkala requested a review from vadika January 27, 2025 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brianmcgillion brianmcgillion brianmcgillion left review comments

@vadika vadika Awaiting requested review from vadika

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@slakkala @ktusawrk @leivos-unikie @brianmcgillion