Tomer Saban has posted comments on this change. Change subject: core:Added affinity Group collision validation. ......................................................................
Patch Set 3: (8 comments) Fixed all the comments on the next patch (4) https://gerrit.ovirt.org/#/c/41805/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/commands/AffinityGroupCRUDCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/commands/AffinityGroupCRUDCommand.java: Line 67: private List<AffinityGroup> getAllAffinityGroups(Guid clusterId) { Line 68: return DbFacade.getInstance().getAffinityGroupDao().getAllAffinityGroupsByClusterId(clusterId); Line 69: } Line 70: Line 71: /** > I think there is a more readable algorithm eating about the same amount of As we discussed the algorithm will be left as is due to better performance on some cases. Line 72: # UAG = {{vm} for each vm} Line 73: # For each (+) affinity group(Sorted by group id): Line 74: ## unify VMs from the group in UAG(Sorted by vm id). Line 75: ## For each (-) affinity group(Sorted by group id): Line 82: Set<Set<VM>> uag = new HashSet(); Line 83: Line 84: VmDAO vm_dao = DbFacade.getInstance().getVmDao(); Line 85: Line 86: //uag = {{vm} for each vm} > change to comment (only adding VMs for each vm in any affinity group Done Line 87: for(AffinityGroup ag: affinityGroups) { Line 88: for(Guid id : ag.getEntityIds()) { Line 89: Set<VM> temp = new HashSet<VM>(); Line 90: temp.add(vm_dao.get(id)); Line 83: Line 84: VmDAO vm_dao = DbFacade.getInstance().getVmDao(); Line 85: Line 86: //uag = {{vm} for each vm} Line 87: for(AffinityGroup ag: affinityGroups) { > Do you put all positive and negative groups to the uag here? What is the re Because, VMs that are not part of any affinity group can't be responsible for affinity group collisions, It's irrelevant to put them in the uag. Line 88: for(Guid id : ag.getEntityIds()) { Line 89: Set<VM> temp = new HashSet<VM>(); Line 90: temp.add(vm_dao.get(id)); Line 91: uag.add(temp); Line 90: temp.add(vm_dao.get(id)); Line 91: uag.add(temp); Line 92: } Line 93: } Line 94: > Change "unify" to "merge" Done Line 95: //# For each (+) affinity group(Sorted by group id): Line 96: //## unify VMs from the group in UAG(Sorted by vm id). Line 97: for(AffinityGroup ag : affinityGroups) { Line 98: if(ag.isPositive()) { Line 92: } Line 93: } Line 94: Line 95: //# For each (+) affinity group(Sorted by group id): Line 96: //## unify VMs from the group in UAG(Sorted by vm id). > Clarify how the unification algorithm works. Done Line 97: for(AffinityGroup ag : affinityGroups) { Line 98: if(ag.isPositive()) { Line 99: Set<VM> vm_set; Line 100: vm_set = new HashSet<VM>(); Line 96: //## unify VMs from the group in UAG(Sorted by vm id). Line 97: for(AffinityGroup ag : affinityGroups) { Line 98: if(ag.isPositive()) { Line 99: Set<VM> vm_set; Line 100: vm_set = new HashSet<VM>(); > s 's/vm_set/mergeToUag/g' (Or to something involving merging Done Line 101: Line 102: for(Guid id : ag.getEntityIds()) { Line 103: Set<VM> vm_group = getVmGroupByGuid(uag, vm_dao.get(id)); Line 104: vm_set.addAll(vm_group); Line 99: Set<VM> vm_set; Line 100: vm_set = new HashSet<VM>(); Line 101: Line 102: for(Guid id : ag.getEntityIds()) { Line 103: Set<VM> vm_group = getVmGroupByGuid(uag, vm_dao.get(id)); > -make getVmGroupByGuid remove the group. Done Line 104: vm_set.addAll(vm_group); Line 105: } Line 106: Line 107: uag.add(vm_set); Line 128: } Line 129: } Line 130: Line 131: //Adding to UAG VMs that are not in it's Sets yet. Line 132: for(AffinityGroup ag : affinityGroups) { > Why are you doing this after the conflict check? It has no influence on the It was removed. Done Line 133: if(ag.isPositive()) { Line 134: continue; Line 135: } Line 136: -- To view, visit https://gerrit.ovirt.org/41805 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56c483023ed14e238c8a67ec0890812158ecd0b2 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomer Saban <tsa...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Tomer Saban <tsa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
