Oved Ourfali has posted comments on this change.
Change subject: engine: HA VN Reservation feature
......................................................................
Patch Set 2:
(7 comments)
See some comments.
Also, note that the build failed with regards to findbugs.
When you'll rebase, it will fail with less (as we cleared all the findbugs
already), but still your code added a few LOW and NORMAL findbugs.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/HaReservationHandling.java
Line 23: /**
Line 24: * @param cluster
Line 25: * - Cluster to check
Line 26: * @param failedHosts
Line 27: * - a list to return all the hosts that failed the
check, must be initialize outside this method
s/initialize/initialized
Line 28: * @return true: Cluster is HaReservation safe. false: a fail over
in one of the Clusters VDSs could negatively
Line 29: * impacting performance.
Line 30: */
Line 31: public boolean checkHaReservationStatusForCluster(VDSGroup
cluster, List<VDS> failedHosts) {
Line 24: * @param cluster
Line 25: * - Cluster to check
Line 26: * @param failedHosts
Line 27: * - a list to return all the hosts that failed the
check, must be initialize outside this method
Line 28: * @return true: Cluster is HaReservation safe. false: a fail over
in one of the Clusters VDSs could negatively
s/fail over/failover/
s/VDSs/Hosts/
Line 29: * impacting performance.
Line 30: */
Line 31: public boolean checkHaReservationStatusForCluster(VDSGroup
cluster, List<VDS> failedHosts) {
Line 32: List<VDS> hosts =
DbFacade.getInstance().getVdsDao().getAllForVdsGroup(cluster.getId());
Line 34: // No hosts, return true
Line 35: if (hosts == null) {
Line 36: return true;
Line 37: }
Line 38: // HA Reservation is not possible with less the 2 hosts
less than
Line 39: if (hosts.size() < 2) {
Line 40: return false;
Line 41: }
Line 42:
Line 42:
Line 43: // List of host id and cpu/ram free resources
Line 44: // for the outer Pair, first is host id second is a Pair of
cpu and ram
Line 45: // for the inner Pair, first is cpu second is ram
Line 46: List<Pair<Guid, Pair<Integer, Integer>>>
hostsUnutilizedResources = getUnutilizedResources(hosts);
if the vmIncluster is null then you're not using this list, so you can move
that to later in this method.
Line 47:
Line 48: List<VM> vmInCluster =
DbFacade.getInstance().getVmDao().getAllForVdsGroup(cluster.getId());
Line 49:
Line 50: if (vmInCluster == null) {
Line 69: failedHosts.add(host);
Line 70: }
Line 71: }
Line 72:
Line 73: if (failedHosts.size() == 0) {
you can just return failedHosts.size() == 0;
Line 74: return true;
Line 75: } else {
Line 76: return false;
Line 77: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SchedulingManager.java
Line 749:
SchedulerUtilQuartzImpl.getInstance().scheduleAFixedDelayJob(instance,
Line 750: "performHaResevationCheck",
Line 751: new Class[] {},
Line 752: new Object[] {},
Line 753: Config.<Integer>
getValue(ConfigValues.VdsHaReservationIntervalInMinutes),
Consider getting this value once, and save it, instead of calling it twice.
Line 754: Config.<Integer>
getValue(ConfigValues.VdsHaReservationIntervalInMinutes),
Line 755: TimeUnit.MINUTES);
Line 756: log.info("Finished HA Reservation check");
Line 757: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/HaReservationBalancePolicyUnit.java
Line 20: import org.ovirt.engine.core.dal.dbbroker.DbFacade;
Line 21: import org.ovirt.engine.core.utils.linq.LinqUtils;
Line 22: import org.ovirt.engine.core.utils.linq.Predicate;
Line 23:
Line 24:
some javadic of this class would be nice, just to understand the policy.
Line 25: public class HaReservationBalancePolicyUnit extends PolicyUnitImpl {
Line 26:
Line 27: public HaReservationBalancePolicyUnit(PolicyUnit policyUnit) {
Line 28: super(policyUnit);
--
To view, visit http://gerrit.ovirt.org/22929
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I44e698e58ff5e4a0b74249d3fe3cf4acf042c324
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Kobi Ianko <k...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@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
