Martin Sivák has uploaded a new change for review. Change subject: core: Change how Affinity behaves if the assumptions are invalid ......................................................................
core: Change how Affinity behaves if the assumptions are invalid This fixes the situation when VMs belonging to the same hard constraint positive affinity group are actually running on more than one host. It used to fail the scheduling task for any VM belonging to the same affinity group (both new VMs and migrating ones). This patch just passes all hosts with VM from that group to the weighting mechanism making it possible for the balancer or administrator to recover from this situation. This change also makes it easier to report per host scheduling information which we need for the following patches. Change-Id: I506bcb6e96c622694e6bf7b8ce7a0f54a82e5713 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1080521 Signed-off-by: Martin Sivak <msi...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/VmAffinityFilterPolicyUnit.java 1 file changed, 46 insertions(+), 45 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/19/26619/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/VmAffinityFilterPolicyUnit.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/VmAffinityFilterPolicyUnit.java index ef216bf..f514c93 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/VmAffinityFilterPolicyUnit.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/VmAffinityFilterPolicyUnit.java @@ -41,8 +41,8 @@ return hosts; } - Set<Guid> allVmIdsPositive = new HashSet<Guid>(); - Set<Guid> allVmIdsNegative = new HashSet<Guid>(); + Set<Guid> allVmIdsPositive = new HashSet<>(); + Set<Guid> allVmIdsNegative = new HashSet<>(); List<String> positiveAffinityGroupNames = new ArrayList<>(); // Group by all vms in affinity groups per positive or negative @@ -69,12 +69,12 @@ } // Get all running VMs in cluster - Map<Guid, VM> runningVMsMap = new HashMap<Guid, VM>(); + Map<Guid, VM> runningVMsMap = new HashMap<>(); for (VM iter : getVmDao().getAllRunningByCluster(vm.getVdsGroupId())) { runningVMsMap.put(iter.getId(), iter); } - Set<Guid> acceptableHosts = new HashSet<Guid>(); + Set<Guid> acceptableHosts = new HashSet<>(); // Group all hosts for VMs with positive affinity for (Guid id : allVmIdsPositive) { VM runVm = runningVMsMap.get(id); @@ -82,53 +82,54 @@ acceptableHosts.add(runVm.getRunOnVds()); } } + + Set<Guid> unacceptableHosts = new HashSet<>(); + // Group all hosts for VMs with negative affinity + for (Guid id : allVmIdsNegative) { + VM runVm = runningVMsMap.get(id); + if (runVm != null && runVm.getRunOnVds() != null) { + unacceptableHosts.add(runVm.getRunOnVds()); + } + } + Map<Guid, VDS> hostMap = new HashMap<>(); for (VDS host : hosts) { hostMap.put(host.getId(), host); } - boolean hasPositiveConstraint = false; - // No hosts associated with positive affinity, all hosts is applicable. - if (acceptableHosts.isEmpty()) { - acceptableHosts.addAll(hostMap.keySet()); - } else if (acceptableHosts.size() == 1 && hostMap.containsKey(acceptableHosts.iterator().next())) { - hasPositiveConstraint = true; - // Only one host is allowed for positive affinity, i.e. if the VM contained in a positive - // affinity group he must run on the host that all the other members are running, if the - // VMs spread across hosts, the affinity rule isn't applied. - } else { - messages.add(String.format("$affinityGroupName %1$s", StringUtils.join(positiveAffinityGroupNames, ", "))); - List<String> hostsNames = new ArrayList<>(); - for (Guid hostId : acceptableHosts) { - if (hostMap.containsKey(hostId)) { - hostsNames.add(hostMap.get(hostId).getName()); - } else { - hostsNames.add(getVdsStaticDao().get(hostId).getName()); - } - } - messages.add(String.format("$hostName %1$s", StringUtils.join(hostsNames, ", "))); - messages.add(VdcBllMessages.ACTION_TYPE_FAILED_POSITIVE_AFFINITY_GROUP.toString()); - return null; - } - // Handle negative affinity - StringBuilder negativeLogMessage = new StringBuilder("Negative Affinity remove host(s):"); - for (Guid id : allVmIdsNegative) { - VM runVm = runningVMsMap.get(id); - if (runVm != null && runVm.getRunOnVds() != null) { - acceptableHosts.remove(runVm.getRunOnVds()); - negativeLogMessage.append(MessageFormat.format(" {0} (vm {1}),", runVm.getRunOnVds(), vm.getName())); - } - } - if (acceptableHosts.isEmpty()) { - if (hasPositiveConstraint) { - messages.add(VdcBllMessages.ACTION_TYPE_FAILED_MIX_POSITIVE_NEGATIVE_AFFINITY_GROUP.toString()); - } else { - messages.add(VdcBllMessages.ACTION_TYPE_FAILED_NEGATIVE_AFFINITY_GROUP.toString()); - } - log.info(negativeLogMessage); - return null; + + // Compute the intersection of hosts with positive and negative affinity and report that + // contradicting rules to the log + unacceptableHosts.retainAll(acceptableHosts); + for (Guid id: unacceptableHosts) { + log.warnFormat("Host {1} ({2}) belongs to both positive and negative affinity list" + + " while scheduling VM {3} ({4})", + hostMap.get(id).getName(), id.toString(), + vm.getName(), vm.getId()); } - List<VDS> retList = new ArrayList<VDS>(); + // No hosts associated with positive affinity, all hosts are applicable. + if (acceptableHosts.isEmpty()) { + acceptableHosts.addAll(hostMap.keySet()); + } + else if (acceptableHosts.size() > 1) { + log.warnFormat("Invalid affinity situation was detected while scheduling VM {1} ({2})." + + " VMs belonging to the same affinity groups are running on more than one host.", + vm.getName(), vm.getId()); + } + + } + } + + // Remove hosts that contain VMs with negaive affinity to the currently scheduled Vm + for (Guid id : allVmIdsNegative) { + VM runVm = runningVMsMap.get(id); + if (runVm != null && runVm.getRunOnVds() != null + && acceptableHosts.contains(runVm.getRunOnVds())) { + acceptableHosts.remove(runVm.getRunOnVds()); + } + } + + List<VDS> retList = new ArrayList<>(); for (VDS host : hosts) { if (acceptableHosts.contains(host.getId())) { retList.add(host); -- To view, visit http://gerrit.ovirt.org/26619 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I506bcb6e96c622694e6bf7b8ce7a0f54a82e5713 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Sivák <msi...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
