Calculate memory_mb_max_unit based on instance's memory_mb#507
Calculate memory_mb_max_unit based on instance's memory_mb#507leust wants to merge 1 commit intostable/2023.2-m3from
Conversation
leust
force-pushed
the
memory-mb-max-unit-improve
branch
from
20c85fe to
ee181da
Compare
leust
force-pushed
the
memory-mb-max-unit-improve
branch
from
ee181da to
c5798a3
Compare
joker-at-work
left a comment
joker-at-work
left a comment
There was a problem hiding this comment.
When we report max_unit for memory_mb to be smaller than what a VM currently already has, does Placement still support live-migration off that host? I remember us patching Placement to allow for switching over resources to another resource-provider, but I also remember that recently seeing some problems with max_unit. Can you please check if we need to patch Placement to not do the max_unit check?
The thing I don not like about this, is that we duplicate the "valid host" checking (inMaintenance, connection_state). I would prefer - maybe as a follow-up - if we could have that in only one place. Otherwise, e.g. when we add a check for hosts going into MM in addition to already being in MM, we might miss a place where this is checked.
leust
force-pushed
the
memory-mb-max-unit-improve
branch
from
c5798a3 to
95ff72c
Compare
leust
force-pushed
the
memory-mb-max-unit-improve
branch
from
95ff72c to
c46674a
Compare
nova/virt/vmwareapi/driver.py
Outdated
| memory_mb_max_unit = max( | ||
| mem for mem in | ||
| self._vmops.get_available_memory_per_host().values()) |
There was a problem hiding this comment.
mem for mem in looks superfluous here
nova/virt/vmwareapi/vmops.py
Outdated
| (host_mors, _) = vm_util.get_hosts_and_reservations_for_cluster( | ||
| self._session, self._cluster) |
There was a problem hiding this comment.
Why do we re-fetch this instead of using the information already gather when we get called in _get_cluster_stats()? I guess because we do not save the morefs of our hosts anywhere and they are only available inside VCState.upate_status(). Might be worth mentioning or changing.
There was a problem hiding this comment.
I have refactored this function to rely on the previously fetched hosts information.
nova/virt/vmwareapi/vmops.py
Outdated
| host_ref_value = vutil.get_moref_value(obj.obj) | ||
| hardware_summary = host_props.get("summary.hardware") | ||
| mem_size = getattr(hardware_summary, "memorySize", 0) | ||
| ram_per_host[host_ref_value] = mem_size // units.Mi |
There was a problem hiding this comment.
Should we reduce the memory by the reservations? We get the reservations above in vm_util.get_hosts_and_reservations_for_cluster(), but ignore them. Default is to reserve 5% on every host. We support percentage-based admission control. Maybe this could become relevant?
There was a problem hiding this comment.
It now accounts for the reserved memory as well.
Previously the memory_mb_max_unit was relying on the overallMemoryUsage reported by the ESXi, which was wrong, because the host could still have a big VM with reserved memory, that was not consuming any RAM and thus not reflected in the memory usage. We are now looking for the instances spawned on that ESXi and subtract their configured memory_mb from the total RAM capacity of the host, resulting in a realistic memory_mb_max_unit. Change-Id: I72821c97d80f9f675dca943d65f94e36824d81b6
leust
force-pushed
the
memory-mb-max-unit-improve
branch
from
c46674a to
5186c82
Compare
|
Added a support for a new trait called |
2 participants
Previously the memory_mb_max_unit was relying on the overallMemoryUsage reported by the ESXi, which was wrong, because the host could still have a big VM with reserved memory, that was not consuming any RAM and thus not reflected in the memory usage.
We are now looking for the instances spawned on that ESXi and subtract their configured memory_mb from the total RAM capacity of the host, resulting in a realistic memory_mb_max_unit.
Change-Id: I72821c97d80f9f675dca943d65f94e36824d81b6