Gilad Chaplik has posted comments on this change. Change subject: engine: Update Power Saving policy with power management ......................................................................
Patch Set 4: (16 comments) .................................................... Commit Message Line 16: Line 17: When there are too many hosts with no VMs, the algorithm will Line 18: select a host and put it first to maintenance and then down. Line 19: Line 20: If we need more hosts, it will start them up. feature page link Line 21: Line 22: Change-Id: I2da9f4e019c3dd006caefbaa9baf9a533ce41f3d Line 23: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1035238 .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/FenceVdsBaseCommand.java Line 178: AlertIfPowerManagementOperationFailed(); Line 179: } Line 180: Line 181: // Successful fencing with reboot or shutdown op. Clear the power management policy flag Line 182: else if (EnumSet.of(FenceActionType.Restart, FenceActionType.Stop).contains(getParameters().getAction()) imo, simple == check is better, or make the enumSet static. Line 183: && getParameters().getKeepPolicyPMEnabled() == false){ Line 184: getVds().setPowerManagementControlledByPolicy(false); Line 185: DbFacade.getInstance().getVdsDynamicDao().update(getVds().getDynamicData()); Line 186: } Line 181: // Successful fencing with reboot or shutdown op. Clear the power management policy flag Line 182: else if (EnumSet.of(FenceActionType.Restart, FenceActionType.Stop).contains(getParameters().getAction()) Line 183: && getParameters().getKeepPolicyPMEnabled() == false){ Line 184: getVds().setPowerManagementControlledByPolicy(false); Line 185: DbFacade.getInstance().getVdsDynamicDao().update(getVds().getDynamicData()); same comment like in previous patch, no need to update entire vdsDynamic for a single field. Line 186: } Line 187: } Line 188: } Line 189: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceNumberOfVdssCommand.java Line 146: if (!getParameters().getKeepPolicyPMEnabled()) { Line 147: for (Guid vdsId : getParameters().getVdsIdList()) { Line 148: VDS vds = DbFacade.getInstance().getVdsDao().get(vdsId); Line 149: vds.setPowerManagementControlledByPolicy(false); Line 150: DbFacade.getInstance().getVdsDynamicDao().update(vds.getDynamicData()); same Line 151: } Line 152: } Line 153: Line 154: setSucceeded(true); .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/PowerSavingBalancePolicyUnit.java Line 42: Map<String, String> parameters, Line 43: ArrayList<String> messages) { Line 44: final Pair<List<Guid>, Guid> migrationRule = super.balance(cluster, hosts, parameters, messages); Line 45: Line 46: List<VDS> allHosts = DbFacade.getInstance().getVdsDao().getAllForVdsGroup(cluster.getId()); I prefer not handling host with migratable vms (###) and use the host list supplied in method params Line 47: Line 48: Pair<VDS, VDSStatus> action = evaluatePowerManagementSituation( Line 49: getDownHosts(allHosts, parameters), Line 50: getMaintenanceHosts(allHosts, parameters), Line 49: getDownHosts(allHosts, parameters), Line 50: getMaintenanceHosts(allHosts, parameters), Line 51: getEmptyHosts(allHosts, parameters), Line 52: parameters Line 53: ); format/ Line 54: Line 55: if (action != null) processPmAction(action); Line 56: Line 57: return migrationRule; Line 51: getEmptyHosts(allHosts, parameters), Line 52: parameters Line 53: ); Line 54: Line 55: if (action != null) processPmAction(action); parentheses and format (all over) Line 56: Line 57: return migrationRule; Line 58: } Line 59: Line 70: Line 71: if (targetStatus == VDSStatus.Maintenance && currentStatus == VDSStatus.Up) { Line 72: logAction(vds, AuditLogType.PM_POLICY_UP_TO_MAINTENANCE); Line 73: Line 74: /* Up -> Maint */ I don' t mind, but // is preferable than /* */ Line 75: List<Guid> vdsList = new ArrayList<>(1); Line 76: vdsList.add(vds.getId()); Line 77: MaintenanceNumberOfVdssParameters parameters = new MaintenanceNumberOfVdssParameters(vdsList, true, true); Line 78: Backend.getInstance().runInternalAction(VdcActionType.MaintenanceNumberOfVdss, Line 71: if (targetStatus == VDSStatus.Maintenance && currentStatus == VDSStatus.Up) { Line 72: logAction(vds, AuditLogType.PM_POLICY_UP_TO_MAINTENANCE); Line 73: Line 74: /* Up -> Maint */ Line 75: List<Guid> vdsList = new ArrayList<>(1); remove 1. Line 76: vdsList.add(vds.getId()); Line 77: MaintenanceNumberOfVdssParameters parameters = new MaintenanceNumberOfVdssParameters(vdsList, true, true); Line 78: Backend.getInstance().runInternalAction(VdcActionType.MaintenanceNumberOfVdss, Line 79: parameters, Line 106: parameters, Line 107: ExecutionHandler.createInternalJobContext()); Line 108: } Line 109: else { Line 110: /* Should not ever happen... */ and if we do log it with with host's status (cur and target) Line 111: } Line 112: } Line 113: Line 114: /** Line 122: @Override Line 123: public boolean eval(VDS vds) { Line 124: return vds.getStatus() == VDSStatus.Up Line 125: && vds.getVmCount() == 0 Line 126: && vds.getVmMigrating() == 0 ### Line 127: && vds.getPendingVmemSize() == 0 Line 128: && vds.getPendingVcpusCount() == 0; Line 129: } Line 130: }); Line 164: @Override Line 165: public boolean eval(VDS vds) { Line 166: return vds.getStatus() == VDSStatus.Down Line 167: && vds.isPowerManagementControlledByPolicy() Line 168: && vds.isDisablePowerManagementPolicy() == false; !vds.isDisablePowerManagementPolicy() Line 169: } Line 170: }); Line 171: Line 172: return emptyHosts; Line 186: Map<String, String> parameters) { Line 187: final int requiredReserve = tryParseWithDefault(parameters.get("HostsInReserve"), Config Line 188: .<Integer> getValue(ConfigValues.HostsInReserve)); Line 189: final int enableAutoPM = tryParseWithDefault(parameters.get("EnableAutomaticHostPowerManagement"), Config Line 190: .<Integer> getValue(ConfigValues.EnableAutomaticHostPowerManagement)); use NumberUtils.toInt(int, default) do me a favor and remove that method :-) Line 191: Line 192: /* Automatic power management is disabled */ Line 193: if (enableAutoPM == 0) return null; Line 194: .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/FenceVdsActionParameters.java Line 11: * pass true so we keep the powerManagementControlledByPolicy flag set. Line 12: * Line 13: * If the user triggered this action, clear the flag. Line 14: */ Line 15: private boolean _keepPolicyPMEnabled = false; remove '_', privates start in lower case, also initialized with false by default. Line 16: Line 17: public FenceVdsActionParameters(Guid vdsId, FenceActionType action) { Line 18: super(vdsId); Line 19: _action = action; .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MaintenanceNumberOfVdssParameters.java Line 16: * pass true so we keep the powerManagementControlledByPolicy flag set. Line 17: * Line 18: * If the user triggered this action, clear the flag. Line 19: */ Line 20: private boolean _keepPolicyPMEnabled = false; same. Line 21: Line 22: public MaintenanceNumberOfVdssParameters(java.util.List<Guid> vdsIdList, boolean isInternal) { Line 23: _vdsIdList = vdsIdList; Line 24: _isInternal = isInternal; .................................................... File packaging/dbscripts/upgrade/03_04_0350_add_power_management_to_cluster_policy.sql Line 2: "CpuOverCommitDurationMinutes" : "^([1-9])$", Line 3: "HighUtilization" : "^([5-9][0-9])$", Line 4: "LowUtilization" : "^([1-4][0-9])$", Line 5: "HostsInReserve": "^[0-9][0-9]*$", Line 6: "EnableAutomaticHostPowerManagement": "^[01]$" use ^(true|false)$ -> renders to dropdown -- 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: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@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: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
