Add preload memory threshold spin button #272
Conversation
qubes_config/global_config.glade
Outdated
| <object class="GtkLabel"> | ||
| <property name="visible">True</property> | ||
| <property name="can-focus">False</property> | ||
| <property name="label" translatable="yes">Memory threshold that is deducted from the calculation of system's available memory. Useful to prevent memory exhaustion.</property> |
There was a problem hiding this comment.
Hello @adw, could you help me make this clear in case it is not? Could you understand on the first reading?
The calculation is simple:
PRELOAD_MEM = DVM_TEMPLTE.MEM_PROP
USEFUL_MEM = CURRENT_AVAIL_MEM - THRESHOLD
IF USEFUL_MEM > PRELOAD_MEM:
PRELOAD
But I think it would become more confusing if I were to into the details on the text.
There was a problem hiding this comment.
wrong Andrew ;) @andrewdavidwong
Maybe this could be something like "Memory reserved for preloaded disposables"?
There was a problem hiding this comment.
Oh no, that's not it - maybe something like 'Minimum free memory that has to be available for preloading to start"? In any case, this feels like something we as devs should calculate smartly, not leave it to the user to decide...
There was a problem hiding this comment.
Yep, wrong Andrew, sorry...
Minimum free memory that has to be available for preloading to start
It is not that. It is a threshold (static), that value is deducted from the current available memory in the system (volatile), to calculate on runtime if a preload can be created.
Consider this:
- system maximum memory: 16GB
- system available memory : 2GB
- threshold memory: 1500 MiB
- result: 2GB - 1500MiB = ~500MiB
This would allow one qube that has memory property of 400 MiB to be preloaded.
About calculating smartly, I sure would like for this to be set automatically via GUI but customizable via CLI (some OpenQA runs are capped to 8GB of RAM, so I'd not set the threshold or set it to 0). The GUI then, would not modify the value if the feature is already set via CLI. This works as long as the calculating algorithm doesn't change, cause then, we'd have no way to updated it without overriding user's choice.
If you see the commit history, there was a time that I was defaulting to 1000MiB, but I thought it was too arbitrary.
I have a 32GB system, xl info total_memory reports 32404 MB.
>>> def MB_to_MiB(size):
... return size * 1000**2 / 1024**2
>>> total_memory_MB = 32404
>>> total_memory_MiB = MB_to_MiB(total_memory_MB)
>>> value = int(total_memory_MiB * 0.05)
>>> value
1545
>>> min(max(value, 1000), 6000) # 1000 <= value <= 6000For comparison:
| total mem | reserved mem |
|---|---|
| 8GB | 1000MiB* |
| 16GB | 1000MiB |
| 24GB | 1158MiB |
| 32GB | 1545MiB |
| 64GB | 3090MiB |
| 96GB | 4635MiB |
| 128GB | 6000MiB |
* disabled for openqa
Is this 5% enough or too much? Depends on each user's workflow. On a 16GB system, I notice significant slowdowns on the system when there is less than one 1GB of free memory, so I think 5% should be enough.
@marmarek had some problems with starting out of memory by the preload qube attempting to start at the same time as the normal disposable qube, a delay of some seconds was added to that to check the memory again, but a threshold seems best to prevent the memory starvation case.
But I am not sure if this number should be decided by the total memory of the system, because I don't think GUIVM can do xl info total_memory.
There was a problem hiding this comment.
How about: "The amount of free system memory that will never be used for preloading disposables. Ensures preloaded disposables do not consume all available memory."
There was a problem hiding this comment.
How about: "The amount of free system memory that will never be used for preloading disposables. Ensures preloaded disposables do not consume all available memory."
That is very good.
There was a problem hiding this comment.
Because GUI domain doesn't now the system's total memory, the installer will set via Salt on dom0 the threshold feature based on the total memory of the system. This way, there is no need to duplicate the logic here.
ben-grande
force-pushed
the
preload-mem-threshold
branch
from
e7285a9 to
17a1edc
Compare
|
pylint complains (and not because of core-admin-client PR not merged yet) |
ben-grande
force-pushed
the
preload-mem-threshold
branch
from
17a1edc to
119f859
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025080809-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025061004-4.3&flavor=update
Failed tests15 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/142375#dependencies 11 fixed
Unstable testsPerformance TestsPerformance degradation:3 performance degradations
Remaining performance tests:130 tests
|
ben-grande
force-pushed
the
preload-mem-threshold
branch
from
119f859 to
dd2b3a9
Compare
|
PipelineRetryFailed |
|
PipelineRetry |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #272 +/- ##
==========================================
+ Coverage 93.00% 93.01% +0.01%
==========================================
Files 64 64
Lines 13217 13269 +52
==========================================
+ Hits 12292 12342 +50
- Misses 925 927 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Passed checks: codecov/patch is considering |
| <property name="visible">True</property> | ||
| <property name="can-focus">False</property> | ||
| <child> | ||
| <object class="GtkSpinButton" id="basics_preload_dispvm_threshold"> |
There was a problem hiding this comment.
I think this needs margin-start? Because in the finished window it's a couple of pixels to the left of the other controls in its column.
ben-grande
force-pushed
the
preload-mem-threshold
branch
from
dd2b3a9 to
a8a15ca
Compare
5 participants
For: QubesOS/qubes-issues#1512
For: QubesOS/qubes-issues#10027
Bad resolution because of VNC:
It starts the value as
0but doesn't save it if the feature is not set yet. Yes, there is no way to remove the feature, but value0is equal to a deleted feature in this case, as it does not the behavior ofpreload-dispvm-maxbeing set or not.