Roy Golan has posted comments on this change. Change subject: WIP Update balancers and add memory based load balancing ......................................................................
Patch Set 1: (5 comments) I assume the logic is ok as I saw you reaped out existing parts + some the additions of mem which takes precedence over CPU. overall just styling comments and few other toughts https://gerrit.ovirt.org/#/c/38189/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/CpuAndMemoryBalancingPolicyUnit.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/CpuAndMemoryBalancingPolicyUnit.java: Line 57: int hostCount = hosts == null ? 0 : hosts.size(); Line 58: log.debug("No balancing for cluster '{}', contains only {} host(s)", cluster.getName(), hostCount); Line 59: return null; Line 60: } Line 61: // get vds that over committed for the time defined minor - /** instead of // Line 62: /* returns list of Hosts with Line 63: * cpuUtilization >= highUtilization Line 64: * && cpuOverCommitMinutes >= CpuOverCommitDurationMinutes Line 65: */ Line 80: Line 81: FindVmAndDestinations findVmAndDestinations = getFindVmAndDestinations(cluster, parameters); Line 82: Line 83: // try balancing based on CPU first Line 84: if (overUtilizedCPUHosts != null && overUtilizedCPUHosts.size() > 0) { I suggest moving to use of return Collections.emtpyList() instead of nulls*. that should be cleaner instead of continuously doing null-checks all the time. * except from methods that return list which the result modified Line 85: // returns hosts with utilization lower than the specified threshold Line 86: List<VDS> underUtilizedHosts = getPrimaryDestinations(cluster, hosts, parameters); Line 87: Line 88: //if no host has a spare power, then there is nothing we can do to balance it.. https://gerrit.ovirt.org/#/c/38189/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/EvenDistributionWeightPolicyUnit.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/EvenDistributionWeightPolicyUnit.java: Line 50: score = Math.min((int) Math.round( Line 51: calcDistributeMetric(vds, vm, effectiveCpuCores)) + 1, MaxSchedulerWeight); Line 52: } Line 53: Line 54: // TODO Each 100 MB of free memory is equal to 1% of free CPU equals in terms of score? 100MB is also a score of 1? Line 55: score -= vds.getMaxSchedulingMemory() / 100; Line 56: Line 57: return score; Line 58: } https://gerrit.ovirt.org/#/c/38189/1/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 258: return null; Line 259: } Line 260: Line 261: @Override Line 262: protected FindVmAndDestinations getFindVmAndDestinations(VDSGroup cluster, Map<String, String> parameters) { hard to read. please use the eclipse formatter file under config/ directory if you are using eclipse or IntellijIdea Line 263: final int highUtilization = tryParseWithDefault(parameters.get("HighUtilization"), Config Line 264: .<Integer> getValue(ConfigValues.HighUtilizationForPowerSave)); Line 265: final long overUtilizedMemory = parameters.containsKey(EvenDistributionBalancePolicyUnit.LOW_MEMORY_LIMIT_FOR_OVER_UTILIZED) ? Line 266: Long.valueOf(parameters.get(EvenDistributionBalancePolicyUnit.LOW_MEMORY_LIMIT_FOR_OVER_UTILIZED)) : 0L; https://gerrit.ovirt.org/#/c/38189/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/utils/FindVmAndDestinations.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/utils/FindVmAndDestinations.java: Line 94: * @param vmDAO The data source to get the VM information Line 95: * @param hostId Id of a host the returned VMs run at Line 96: * @return list od VM that run on host and can be migrated Line 97: */ Line 98: private List<VM> getMigrableVmsRunningOnVds(final VmDAO vmDAO, final Guid hostId) { s/Migrable/Migratalbe Line 99: List<VM> vmsFromDB = vmDAO.getAllRunningForVds(hostId); Line 100: Line 101: List<VM> vms = LinqUtils.filter(vmsFromDB, new Predicate<VM>() { Line 102: @Override -- To view, visit https://gerrit.ovirt.org/38189 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fe13267feca89ab6c8fb9d85656f05930d0b333 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Sivák <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Tomer Saban <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
