Moti Asayag has posted comments on this change.
Change subject: core: scheduling: increase NetworkPolicyUnit performance
......................................................................
Patch Set 1:
(11 comments)
Cool patch.
Mostly cosmetics comments.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/policyunits/NetworkPolicyUnit.java
Line 43: vmNICs =
getVmNetworkInterfaceDao().getAllForVm(vm.getId());
Line 44: } else {
Line 45: return null;
Line 46: }
Line 47: Map<Guid, List<VdsNetworkInterface>> allInterfacesForVdsMap =
name can be simplified to hostNics instead of allInterfacesForVdsMap
Line 48:
getAllInterfacesForVdsGroupMap(hosts.get(0).getVdsGroupId());
Line 49: for (VDS host : hosts) {
Line 50: VdcBllMessages returnValue =
Line 51: validateRequiredNetworksAvailable(host,
Line 79: * @param vdsGroupId
Line 80: * @return
Line 81: * map key = hostId, value = list of host's interfaces
Line 82: */
Line 83: private Map<Guid, List<VdsNetworkInterface>>
getAllInterfacesForVdsGroupMap(Guid vdsGroupId) {
s/vdsGroupId/clusterId
Line 84: Map<Guid, List<VdsNetworkInterface>> map = new HashMap<Guid,
List<VdsNetworkInterface>>();
Line 85: List<VdsNetworkInterface> allInterfacesForVdsGroup =
Line 86:
getInterfaceDAO().getAllInterfacesForVdsGroup(vdsGroupId);
Line 87: for (VdsNetworkInterface vdsNetworkInterface :
allInterfacesForVdsGroup) {
Line 80: * @return
Line 81: * map key = hostId, value = list of host's interfaces
Line 82: */
Line 83: private Map<Guid, List<VdsNetworkInterface>>
getAllInterfacesForVdsGroupMap(Guid vdsGroupId) {
Line 84: Map<Guid, List<VdsNetworkInterface>> map = new HashMap<Guid,
List<VdsNetworkInterface>>();
per java 7 the type on the instantiation can be omitted: new HashMap<>();
should be enough.
Line 85: List<VdsNetworkInterface> allInterfacesForVdsGroup =
Line 86:
getInterfaceDAO().getAllInterfacesForVdsGroup(vdsGroupId);
Line 87: for (VdsNetworkInterface vdsNetworkInterface :
allInterfacesForVdsGroup) {
Line 88: Guid vdsId = vdsNetworkInterface.getVdsId();
Line 81: * map key = hostId, value = list of host's interfaces
Line 82: */
Line 83: private Map<Guid, List<VdsNetworkInterface>>
getAllInterfacesForVdsGroupMap(Guid vdsGroupId) {
Line 84: Map<Guid, List<VdsNetworkInterface>> map = new HashMap<Guid,
List<VdsNetworkInterface>>();
Line 85: List<VdsNetworkInterface> allInterfacesForVdsGroup =
s/allInterfacesForVdsGroup/clusterInterfaces or clusterNics or nicsInCluster?
Line 86:
getInterfaceDAO().getAllInterfacesForVdsGroup(vdsGroupId);
Line 87: for (VdsNetworkInterface vdsNetworkInterface :
allInterfacesForVdsGroup) {
Line 88: Guid vdsId = vdsNetworkInterface.getVdsId();
Line 89: if (map.get(vdsId) == null) {
Line 83: private Map<Guid, List<VdsNetworkInterface>>
getAllInterfacesForVdsGroupMap(Guid vdsGroupId) {
Line 84: Map<Guid, List<VdsNetworkInterface>> map = new HashMap<Guid,
List<VdsNetworkInterface>>();
Line 85: List<VdsNetworkInterface> allInterfacesForVdsGroup =
Line 86:
getInterfaceDAO().getAllInterfacesForVdsGroup(vdsGroupId);
Line 87: for (VdsNetworkInterface vdsNetworkInterface :
allInterfacesForVdsGroup) {
s/vdsNetworkInterface/nics
Line 88: Guid vdsId = vdsNetworkInterface.getVdsId();
Line 89: if (map.get(vdsId) == null) {
Line 90: map.put(vdsId, new ArrayList<VdsNetworkInterface>());
Line 91: }
Line 85: List<VdsNetworkInterface> allInterfacesForVdsGroup =
Line 86:
getInterfaceDAO().getAllInterfacesForVdsGroup(vdsGroupId);
Line 87: for (VdsNetworkInterface vdsNetworkInterface :
allInterfacesForVdsGroup) {
Line 88: Guid vdsId = vdsNetworkInterface.getVdsId();
Line 89: if (map.get(vdsId) == null) {
you can use map.containsKey(key) instead
Line 90: map.put(vdsId, new ArrayList<VdsNetworkInterface>());
Line 91: }
Line 92: map.get(vdsId).add(vdsNetworkInterface);
Line 93: }
Line 113: VM vm,
Line 114: List<VmNetworkInterface> vmNICs,
Line 115: List<Network> clusterNetworks,
Line 116: Map<String, Network> networksByName,
Line 117: List<VdsNetworkInterface> allInterfacesForVds) {
Please add the new parameter to the javadoc and rename it to be just
'interfaces'
Line 118:
Line 119: boolean onlyRequiredNetworks =
Line 120: Config.<Boolean>
getValue(ConfigValues.OnlyRequiredNetworksMandatoryForVdsSelection);
Line 121: for (final VmNetworkInterface vmIf : vmNICs) {
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDao.java
Line 74: * @param id
Line 75: * the cluster id
Line 76: * @return the list of interfaces
Line 77: */
Line 78: List<VdsNetworkInterface> getAllInterfacesForVdsGroup(Guid
vdsGroupId);
s/getAllInterfacesForVdsGroup/getAllInterfacesForCluster
s/vdsGroupId/clusterId
Line 79:
Line 80: /**
Line 81: * Retrieves all interfaces for the given VDS id with optional
filtering.
Line 82: *
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java
Line 169:
Line 170: @Override
Line 171: public List<VdsNetworkInterface> getAllInterfacesForVdsGroup(Guid
vdsGroupId) {
Line 172: MapSqlParameterSource parameterSource =
getCustomMapSqlParameterSource()
Line 173: .addValue("vds_group_id", vdsGroupId);
s/vds_group_id/cluster_id
Line 174:
Line 175: return
getCallsHandler().executeReadList("Getinterface_viewByVdsGroupId",
Line 176: vdsNetworkInterfaceRowMapper,
Line 177: parameterSource);
Line 171: public List<VdsNetworkInterface> getAllInterfacesForVdsGroup(Guid
vdsGroupId) {
Line 172: MapSqlParameterSource parameterSource =
getCustomMapSqlParameterSource()
Line 173: .addValue("vds_group_id", vdsGroupId);
Line 174:
Line 175: return
getCallsHandler().executeReadList("Getinterface_viewByVdsGroupId",
see suggested name in the sp file
Line 176: vdsNetworkInterfaceRowMapper,
Line 177: parameterSource);
Line 178: }
Line 179:
....................................................
File packaging/dbscripts/network_sp.sql
Line 356: WHERE user_id = v_user_id AND
entity_id = v_vds_id));
Line 357:
Line 358: END; $procedure$
Line 359: LANGUAGE plpgsql;
Line 360:
Please avoid using legacy naming:
s/Getinterface_viewByVdsGroupId/GetinterfacesByClusterId
s/v_vds_group_id/v_cluster_id
Line 361: Create or replace FUNCTION
Getinterface_viewByVdsGroupId(v_vds_group_id UUID)
Line 362: RETURNS SETOF vds_interface_view STABLE
Line 363: AS $procedure$
Line 364: BEGIN
--
To view, visit http://gerrit.ovirt.org/22463
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b394b5d50abcbf58c63cb2d2af3867197ddb788
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[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
