You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/12/19 16:41:24 UTC
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #6809: Reserve memory for host
GutoVeronezi commented on code in PR #6809:
URL: https://github.com/apache/cloudstack/pull/6809#discussion_r1052398464
##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -450,6 +449,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
private Set<String> overprovisioningFactorsForValidation;
public static final String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length";
+ public static final String HOST_RESERVED_MEM_MB_STRING = "host.reserved.mem.mb";
Review Comment:
We could use `HOST_RESERVED_MEM_MB.key()` instead of declaring a constant with it.
##########
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java:
##########
@@ -472,6 +472,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
public static final ConfigKey<Boolean> ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN = new ConfigKey<Boolean>(Boolean.class, "enable.domain.settings.for.child.domain", "Advanced", "false",
"Indicates whether the settings of parent domain should be applied for child domain. If true, the child domain will get value from parent domain if its not configured in child domain else global value is taken.",
true, ConfigKey.Scope.Global, null);
+ public static final ConfigKey<Integer> HOST_RESERVED_MEM_MB = new ConfigKey<>("Advanced", Integer.class, HOST_RESERVED_MEM_MB_STRING, "1024",
+ "Set an upper limit for memory in megabytes which will be reserved for host and not used for VM allocation.", true);
Review Comment:
As hosts can behave differently between clusters, might the operators want to set a different value for each cluster?
##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -2190,6 +2191,12 @@ protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource
hostTags = implicitHostTags;
}
}
+
+ // Update host memory reported by agent
+ if (ssCmd.getHypervisorType().equals(HypervisorType.KVM) ||
+ ssCmd.getHypervisorType().equals(HypervisorType.LXC)) {
+ host.setDom0MinMemory(Long.parseLong(_configDao.getValue(HOST_RESERVED_MEM_MB_STRING)) * 1024L * 1024L);
Review Comment:
We could use `HOST_RESERVED_MEM_MB.value()` here, instead of retrieving with `_configDao`.
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,50 @@ public void propagateChangeToAgents(Map<String, String> params) {
sendCommandToAgents(hostsPerZone, params);
}
}
+
+ @Override
+ public void updateCapacityOfHosts() {
+ Map<Long, List<Long>> hostsByZone = new HashMap<>();
+ Map<String, String> params = new HashMap<>();
+ boolean status = true;
+ List<HostVO> allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing);
+ if (CollectionUtils.isEmpty(allHosts)) {
+ return;
+ }
+
+ String value = HOST_RESERVED_MEM_MB.value().toString();
+ params.put(HOST_RESERVED_MEM_MB.key(), value);
+ for (HostVO host : allHosts) {
+ Long zoneId = host.getDataCenterId();
+ try {
+ // Update the "ram" for all hosts
+ long updatedHostRam = (host.getTotalMemory() + host.getDom0MinMemory()) - (Integer.parseInt(value) * 1024L * 1024L);
+ if (updatedHostRam > 0) {
+ host.setTotalMemory(updatedHostRam);
+ // Update "dom0_memory" in host table
+ host.setDom0MinMemory(Integer.parseInt(value) * 1024L * 1024L);
Review Comment:
We can use `ByteScaleUtils` for scale conversions.
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1862,4 +1871,50 @@ public void propagateChangeToAgents(Map<String, String> params) {
sendCommandToAgents(hostsPerZone, params);
}
}
+
+ @Override
+ public void updateCapacityOfHosts() {
+ Map<Long, List<Long>> hostsByZone = new HashMap<>();
+ Map<String, String> params = new HashMap<>();
+ boolean status = true;
+ List<HostVO> allHosts = _resourceMgr.listAllHostsInAllZonesByType(Host.Type.Routing);
+ if (CollectionUtils.isEmpty(allHosts)) {
+ return;
+ }
+
+ String value = HOST_RESERVED_MEM_MB.value().toString();
+ params.put(HOST_RESERVED_MEM_MB.key(), value);
+ for (HostVO host : allHosts) {
+ Long zoneId = host.getDataCenterId();
+ try {
+ // Update the "ram" for all hosts
+ long updatedHostRam = (host.getTotalMemory() + host.getDom0MinMemory()) - (Integer.parseInt(value) * 1024L * 1024L);
Review Comment:
We can use `ByteScaleUtils` for scale conversions.
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -1783,6 +1791,7 @@ public void processConnect(final Host host, final StartupCommand cmd, final bool
Map<String, String> params = new HashMap<String, String>();
params.put(Config.RouterAggregationCommandEachTimeout.toString(), _configDao.getValue(Config.RouterAggregationCommandEachTimeout.toString()));
params.put(Config.MigrateWait.toString(), _configDao.getValue(Config.MigrateWait.toString()));
+ params.put(HOST_RESERVED_MEM_MB_STRING, _configDao.getValue(HOST_RESERVED_MEM_MB_STRING));
Review Comment:
```suggestion
params.put(HOST_RESERVED_MEM_MB.key(), HOST_RESERVED_MEM_MB.value());
```
##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -121,7 +124,10 @@
import com.cloud.utils.nio.Task;
import com.cloud.utils.time.InaccurateClock;
import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.collections.CollectionUtils;
+import static com.cloud.configuration.ConfigurationManagerImpl.HOST_RESERVED_MEM_MB;
+import static com.cloud.configuration.ConfigurationManagerImpl.HOST_RESERVED_MEM_MB_STRING;
Review Comment:
IMHO, using the normal import would let the code more readable than using `import static`, as we would know from where the attribute is just by looking at the class qualification.
##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -2190,6 +2191,12 @@ protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource
hostTags = implicitHostTags;
}
}
+
+ // Update host memory reported by agent
+ if (ssCmd.getHypervisorType().equals(HypervisorType.KVM) ||
+ ssCmd.getHypervisorType().equals(HypervisorType.LXC)) {
+ host.setDom0MinMemory(Long.parseLong(_configDao.getValue(HOST_RESERVED_MEM_MB_STRING)) * 1024L * 1024L);
Review Comment:
We can use `ByteScaleUtils` for scale conversions.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1371,11 +1373,19 @@ public boolean configureHostParams(final Map<String, String> params) {
}
if (params.get(Config.MigrateWait.toString()) != null) {
- String value = (String)params.get(Config.MigrateWait.toString());
+ String value = (String) params.get(Config.MigrateWait.toString());
Integer intValue = NumbersUtil.parseInt(value, -1);
storage.persist("vm.migrate.wait", String.valueOf(intValue));
_migrateWait = intValue;
}
+ if (params.get(HOST_RESERVED_MEM_MB_STRING) != null) {
+ long value = Long.parseLong(params.get(HOST_RESERVED_MEM_MB_STRING));
+ s_logger.info("Reserved memory for host is " + value + "MB");
+ _dom0MinMem = value * 1024L * 1024L;
Review Comment:
We can use `ByteScaleUtils` for scale conversions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org