Doron Fediuck has posted comments on this change. Change subject: engine: Update Power Saving policy with power management ......................................................................
Patch Set 10: (8 comments) Martin, Some improvement needed. See inline. http://gerrit.ovirt.org/#/c/22488/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PowerSavingBalancePolicyUnit.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PowerSavingBalancePolicyUnit.java: Line 49: Pair<VDS, VDSStatus> action = evaluatePowerManagementSituation( Line 50: getDownHosts(allHosts, parameters), Line 51: getMaintenanceHosts(allHosts, parameters), Line 52: getEmptyHosts(allHosts, parameters), Line 53: parameters); Single pass is needed here. Line 54: Line 55: if (action != null) { Line 56: processPmAction(action); Line 57: } Line 196: enableAutoPM = Config.<String> getValue(ConfigValues.EnableAutomaticHostPowerManagement); Line 197: } Line 198: Line 199: /* Automatic power management is disabled */ Line 200: if (enableAutoPM.compareTo("false") == 0) return null; - This is why EnableAutomaticHostPowerManagement sould be Boolean and not string. - Please use code blocks even for a single command. - Please log why you return null, so we'l be able to tell why we're not balancing. Line 201: Line 202: /* We need more hosts but there are no available for us */ Line 203: if (requiredReserve > emptyHosts.size() Line 204: && pmDownHosts.isEmpty() Line 201: Line 202: /* We need more hosts but there are no available for us */ Line 203: if (requiredReserve > emptyHosts.size() Line 204: && pmDownHosts.isEmpty() Line 205: && pmMaintenanceHosts.isEmpty()) return null; - Code block needed. - logging. Line 206: Line 207: /* We have enough free hosts so shut some hosts in maintenance down Line 208: keep at least one spare in maintenance during the process. Line 209: */ Line 223: && vds.getpm_enabled(); Line 224: } Line 225: }); Line 226: Line 227: return hostsWithAutoPM.isEmpty() ? null : new Pair<>(hostsWithAutoPM.get(0), VDSStatus.Maintenance); If null we should log it. Line 228: } Line 229: Line 230: /* We have the right amount of empty hosts to start shutting the Line 231: hosts that are resting in maintenance down. Line 239: in maintenance. We can easily activate those. Line 240: */ Line 241: else if (requiredReserve > emptyHosts.size() Line 242: && pmMaintenanceHosts.isEmpty() == false) { Line 243: return new Pair<>(pmMaintenanceHosts.get(0), VDSStatus.Up); log activation? Line 244: } Line 245: Line 246: /* We do not have enough free hosts and no hosts in pm maintenance, Line 247: so we need to start some hosts up. Line 247: so we need to start some hosts up. Line 248: */ Line 249: else if (requiredReserve > emptyHosts.size() Line 250: && pmMaintenanceHosts.isEmpty()) { Line 251: return new Pair<>(pmDownHosts.get(0), VDSStatus.Up); log that we need to power up? Line 252: } Line 253: Line 254: return null; Line 255: } Line 250: && pmMaintenanceHosts.isEmpty()) { Line 251: return new Pair<>(pmDownHosts.get(0), VDSStatus.Up); Line 252: } Line 253: Line 254: return null; why null? ie- we should possibly debug log how we ended up here. Line 255: } Line 256: Line 257: @Override Line 258: protected List<VDS> getOverUtilizedHosts(List<VDS> relevantHosts, http://gerrit.ovirt.org/#/c/22488/10/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java: Line 410: @TypeConverterAttribute(Integer.class) Line 411: @DefaultValueAttribute("0") Line 412: HostsInReserve, Line 413: @Reloadable Line 414: @TypeConverterAttribute(String.class) Should be Boolean Line 415: @DefaultValueAttribute("false") Line 416: EnableAutomaticHostPowerManagement, Line 417: @Reloadable Line 418: @TypeConverterAttribute(Integer.class) -- To view, visit http://gerrit.ovirt.org/22488 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2da9f4e019c3dd006caefbaa9baf9a533ce41f3d Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Eyal Edri <ee...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
