Repository navigation
feat(ateworker-microvm): add --enable-mem-agent flag - #2674
Maria Fernanda Romano Silva (mariafromano-25) wants to merge 1 commit into
Conversation
a38bc07 to
00c4785
Compare
|
/cc |
ajaysundar.k (ajaysundark)
left a comment
ajaysundar.k (ajaysundark)
left a comment
There was a problem hiding this comment.
Thanks for the change Maria Fernanda Romano Silva (@mariafromano-25) ! Can you switch this ready for review?
| // WithMemAgent enables the in-guest Kata memory agent (MGLRU aging, | ||
| // page-cache/zram eviction, and memory compaction). | ||
| func WithMemAgent(kernelParams string) string { | ||
| return appendKernelParams(kernelParams, "agent.mem_agent_enable=", "agent.mem_agent_enable=true agent.mem_agent_memcg_swap=true agent.mem_agent_memcg_period_secs=60 agent.mem_agent_memcg_eviction_run_aging_count_min=1") |
There was a problem hiding this comment.
Can we have these as tunables instead of hard coding them?
|
|
||
| // BaseKernelParams is the guest kernel command line parameters ateom boots with | ||
| const BaseKernelParams = "cgroup_no_v1=all systemd.unified_cgroup_hierarchy=1" | ||
| const BaseKernelParams = "cgroup_no_v1=all systemd.unified_cgroup_hierarchy=1 psi=1" |
There was a problem hiding this comment.
I agree it's good to enable psi by default. We need to check with substrate maintainers if this acceptable or need this behind enable-memagent flag as well
There was a problem hiding this comment.
We can start with a smaller more specific change and evaluate it as default down the line; I moved psi=1 out of BaseKernelParams and into WithMemAgent() for now so it only turns on when --enable-mem-agent is on.
| # since a bigger guest drags more page cache into every memory snapshot. Raise | ||
| # it for RAM-consuming suites. | ||
| ACTOR_MEMORY="256Mi" | ||
| SWEPERF_ACTOR_MEMORY="4Gi" |
There was a problem hiding this comment.
Nit: it maybe a good idea to split the benchmarking code from mem-agent feature.
There was a problem hiding this comment.
Good callout, moved those specific changes to https://github.com/agent-substrate/substrate/pull/2768/changes.
| // WithMemAgent enables the in-guest Kata memory agent (MGLRU aging, | ||
| // page-cache/zram eviction, and memory compaction). | ||
| func WithMemAgent(kernelParams string) string { | ||
| return appendKernelParams(kernelParams, "agent.mem_agent_enable=", "agent.mem_agent_enable=true agent.mem_agent_memcg_swap=true agent.mem_agent_memcg_period_secs=60 agent.mem_agent_memcg_eviction_run_aging_count_min=1") |
There was a problem hiding this comment.
Note this PR itself doesn't handle anything specific to zram contrary to what the description says. That said setting memcg_swap=true is safe even in the absence of one as it falls back to file-only cache reclaim
There was a problem hiding this comment.
Thank you, I updated the PR title to change the mentioning of zram. Those changes are in my zram-feature branch (commit here)
|
area/memory |
| chBinary = pflag.String("cloud-hypervisor-binary", "cloud-hypervisor", "Path to the cloud-hypervisor binary (used to relaunch on restore).") | ||
| guestDebug = pflag.Bool("guest-debug", false, "enable guest debugging (agent debug logs, debug console, and early serial console)") | ||
| enableMemAgent = pflag.Bool("enable-mem-agent", false, "enable the in-guest Kata memory agent (MGLRU aging, page-cache/zram eviction, and compaction)") | ||
| vmmMemReserve = pflag.Int("vmm-mem-reserve-mib", vmmMemReserveMiB, "Guest RAM (MiB) held back from the pod's memory limit for the cloud-hypervisor VMM + virtiofsd, which run as host processes in the pod cgroup alongside the guest RAM. Prevents the pod OOMing when the VM is sized to the pod's memory limit.") |
There was a problem hiding this comment.
We don't expose flags like this to users, these flags are all for substrate itself or debug. I don't think we're at a place where we want to make a ton of these tunable YET, because we're still working on the revamped actor lifecycle where the scheduler becomes aware of sandbox config in a new way.
We don't want to break a workload by scheduling to a worker with incompatible settings and we haven't designed exposing arbitrary settings yet for this reason.
There was a problem hiding this comment.
At this stage we'd rather make reasonable defaults. The other flags are either development only (the debug flag) or things the control plane always plumbs (pod-uid).
There was a problem hiding this comment.
We're generally not blocking OSS changes on downstream changes and vice versa, can we temporarily point any downstream CI at e.g. your fork where this is just enabled by default without the flag, instead of shipping a change we're not sure we want users to use?
There was a problem hiding this comment.
keep in mind GA is ~wednesday and we don't want to expose anything we're not intending to support longterm
Maria Fernanda Romano Silva (mariafromano-25)
changed the title
be04977 to
9caf6c1
Compare
Add kata.WithMemAgent() and the --enable-mem-agent flag on ateworker-microvm to enable PSI (psi=1) and configure the in-guest Kata memory agent (MGLRU aging, page-cache/swap eviction, and memory compaction) when set.
9caf6c1 to
4a6a0d7
Compare
Maria Fernanda Romano Silva (mariafromano-25)
changed the title
e03cba7 to
4a6a0d7
Compare
Bowei Du (bowei)
added
area/microVM
kind/feature
4 participants
Add
kata.WithMemAgent()and the--enable-mem-agentflag onateworker-microvmto enable PSI (psi=1) and configure the in-guest Kata memory agent (MGLRU aging, page-cache/swap eviction, and memory compaction) when--enable-mem-agentis set.