Roy Golan has posted comments on this change. Change subject: engine: scheduling optimization field ......................................................................
Patch Set 6: (3 comments) Generally looks ok. the flow is now running concurrently instead of sync. I've try to drill down the code to trace shared state. just one thing - the filters are sorted in flow. is this relevant that they should be filltered over and over again? also since the filter are shared, future changes to the sort will not play nice here. the data structure is not thread safe. just clear out this. I'll continue on with the review .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SchedulingManager.java Line 262: if (policy.getFunctions() == null || policy.getFunctions().isEmpty()) { Line 263: return vdsList.get(0).getId(); Line 264: } Line 265: Guid bestHost = null; Line 266: // weigh hosts iff there are more than 1 host in case optimize for speed typo iff/if Line 267: // is true and there are more than configurable requests pending skip weighing Line 268: if (vdsList.size() > 1 Line 269: && (!cluster.isSchedulerOptimizeForSpeed() Line 270: || clusterLockMap.get(cluster.getId()).getQueueLength() <= Line 267: // is true and there are more than configurable requests pending skip weighing Line 268: if (vdsList.size() > 1 Line 269: && (!cluster.isSchedulerOptimizeForSpeed() Line 270: || clusterLockMap.get(cluster.getId()).getQueueLength() <= Line 271: Config.<Integer> GetValue(ConfigValues.OptimizeSchedulerForSpeedPendingRequests))) { conig value name is not explantory: please concider ParallelSchedulingRequestThreshold instead also wrapping this withing a method would be clearer if (shoudWeighClusterHosts(vdsList, cluster)) { runFunctions... } /* * the comment from the code here */ shoudWeighClusterHosts(...){ ...} also add logs if we skipped and add debug log on why Line 272: bestHost = runFunctions(policy.getFunctions(), vdsList, vm, parameters); Line 273: } Line 274: if (bestHost == null && vdsList.size() > 0) { Line 275: bestHost = vdsList.get(0).getId(); Line 284: vm.getNumOfCpus()); Line 285: } Line 286: return bestHost; Line 287: } catch (InterruptedException e) { Line 288: // ignore log it Line 289: return null; Line 290: } finally { Line 291: clusterLockMap.get(cluster.getId()).release(); Line 292: } -- To view, visit http://gerrit.ovirt.org/19270 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0784f89648093e650be73fcc3d850a4854d53c1a Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@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