Michael Kublin has uploaded a new change for review. Change subject: core: Performance improvements - replacing search by query ......................................................................
core: Performance improvements - replacing search by query The following patch will replace a search of host in status Up by query which will retrieve all hosts in status Up from Db. A query should be more efficient than search Change-Id: I1c1adda55bd27beef2907bf0ed8a0f8bc274e495 Signed-off-by: Michael Kublin <[email protected]> --- M backend/manager/dbscripts/vds_sp.sql M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java D backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAOWrapperImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java 8 files changed, 53 insertions(+), 237 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/57/8457/1 diff --git a/backend/manager/dbscripts/vds_sp.sql b/backend/manager/dbscripts/vds_sp.sql index 8e02e67..c1243a9 100644 --- a/backend/manager/dbscripts/vds_sp.sql +++ b/backend/manager/dbscripts/vds_sp.sql @@ -817,6 +817,19 @@ END; $procedure$ LANGUAGE plpgsql; +-- Returns all VDS for a given pool and having given status +CREATE OR REPLACE FUNCTION getVdsByStoragePoolIdWithStatus(v_storage_pool_id UUID, v_status integer) RETURNS SETOF vds + AS $procedure$ +BEGIN + BEGIN + RETURN QUERY SELECT vds.* + FROM vds + WHERE (status = v_status) AND (storage_pool_id = v_storage_pool_id); + END; + RETURN; +END; $procedure$ +LANGUAGE plpgsql; + Create or replace FUNCTION UpdateVdsDynamicStatus( v_vds_guid UUID, diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java index 385d2b8..df5f2b4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java @@ -1,6 +1,5 @@ package org.ovirt.engine.core.bll.storage; -import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -9,26 +8,22 @@ import java.util.Map; import java.util.Set; -import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.bll.CommandBase; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.StoragePoolParametersBase; -import org.ovirt.engine.core.common.businessentities.IVdcQueryable; import org.ovirt.engine.core.common.businessentities.StorageDomainSharedStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.StorageFormatType; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.storage_domains; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; -import org.ovirt.engine.core.common.interfaces.SearchType; -import org.ovirt.engine.core.common.queries.SearchParameters; -import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.TransactionScopeOption; import org.ovirt.engine.core.compat.Version; @@ -59,36 +54,15 @@ super(commandId); } - public static final String UpVdssInStoragePoolQuery = "HOST: status = UP and DATACENTER = {0}"; public static final String UpVdssInCluster = "HOST: status = UP and CLUSTER = {0}"; public static final String DesktopsInStoragePoolQuery = "VMS: DATACENTER = {0}"; public static List<VDS> GetAllRunningVdssInPool(storage_pool pool) { - return GetAllRunningVdssInPool(Backend.getInstance(), pool); - } - - private static List<VDS> GetAllRunningVdssInPool(BackendInternal backend, storage_pool pool) { - List<VDS> returnValue = new ArrayList<VDS>(); - - SearchParameters p = new SearchParameters(MessageFormat.format(UpVdssInStoragePoolQuery, pool.getname()), - SearchType.VDS); - p.setMaxCount(Integer.MAX_VALUE); - - @SuppressWarnings("unchecked") - Iterable<IVdcQueryable> fromVds = - (Iterable<IVdcQueryable>) (backend.runInternalQuery(VdcQueryType.Search, p).getReturnValue()); - if (fromVds != null) { - for (IVdcQueryable vds : fromVds) { - if (vds instanceof VDS) { - returnValue.add((VDS) vds); - } - } - } - return returnValue; + return DbFacade.getInstance().getVdsDao().getAllForStoragePoolAndStatus(pool.getId(), VDSStatus.Up); } protected List<VDS> getAllRunningVdssInPool() { - return GetAllRunningVdssInPool(getBackend(), getStoragePool()); + return getVdsDAO().getAllForStoragePoolAndStatus(getStoragePool().getId(), VDSStatus.Up); } protected void updateStoragePoolInDiffTransaction() { @@ -116,12 +90,11 @@ protected boolean InitializeVds() { boolean returnValue = true; if (getVds() == null) { - SearchParameters p = new SearchParameters(MessageFormat.format(UpVdssInStoragePoolQuery, getStoragePool() - .getname()), SearchType.VDS); - p.setMaxCount(Integer.MAX_VALUE); - Object tempVar = LinqUtils.firstOrNull((List) Backend.getInstance() - .runInternalQuery(VdcQueryType.Search, p).getReturnValue(), new All()); - setVds(((VDS) ((tempVar instanceof VDS) ? tempVar : null))); + VDS tempVar = + LinqUtils.firstOrNull(getVdsDAO().getAllForStoragePoolAndStatus(getStoragePool().getId(), + VDSStatus.Up), + new All()); + setVds(tempVar); if (getVds() == null) { returnValue = false; addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NO_VDS_IN_POOL); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java index d5c16db..90515d2 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java @@ -9,6 +9,8 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import java.util.ArrayList; + import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -27,14 +29,12 @@ import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.storage_domains; import org.ovirt.engine.core.common.businessentities.storage_domain_static; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.businessentities.storage_pool_iso_map; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; -import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; -import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; -import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSParametersBase; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; @@ -91,8 +91,7 @@ when(storageDomainStaticDAO.get(any(Guid.class))).thenReturn(new storage_domain_static()); doReturn(backendInternal).when(cmd).getBackend(); - when(backendInternal.runInternalQuery(any(VdcQueryType.class), any(VdcQueryParametersBase.class))).thenReturn - (new VdcQueryReturnValue()); + when(vdsDAO.getAllForStoragePoolAndStatus(any(Guid.class), any(VDSStatus.class))).thenReturn(new ArrayList<VDS>()); when(backendInternal.getResourceManager()).thenReturn(vdsBrokerFrontend); VDSReturnValue returnValue = new VDSReturnValue(); returnValue.setSucceeded(true); diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java index c1ef510..472839e 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java @@ -8,6 +8,8 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import java.util.ArrayList; + import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -19,13 +21,11 @@ import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.VDS; +import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.storage_domains; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.businessentities.storage_pool_iso_map; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; -import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; -import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; -import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSParametersBase; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; @@ -81,8 +81,7 @@ any(TransactionMethod.class)); doReturn(backendInternal).when(cmd).getBackend(); - when(backendInternal.runInternalQuery(any(VdcQueryType.class), any(VdcQueryParametersBase.class))) - .thenReturn(new VdcQueryReturnValue()); + when(vdsDAO.getAllForStoragePoolAndStatus(any(Guid.class), any(VDSStatus.class))).thenReturn(new ArrayList<VDS>()); when(backendInternal.getResourceManager()).thenReturn(vdsBrokerFrontend); VDSReturnValue returnValue = new VDSReturnValue(); returnValue.setSucceeded(true); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java index 724b173..7ea904c 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java @@ -158,6 +158,15 @@ List<VDS> getAllForStoragePool(Guid storagePool, Guid userID, boolean isFiltered); /** + * Retrieves all VDS instances by storage pool ID and status. + * + * @param storagePool The storage pool's ID + * @param status The status of vds + * @return the list of VDS instances + */ + List<VDS> getAllForStoragePoolAndStatus(Guid storagePool, VDSStatus status); + + /** * Retrieves all VDS instances in the given Storage Pool, that are in status "UP" * ordered by their vds_spm_priority, not including -1 * @return the list of VDS instances diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java index acc701b..d356e4a 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAODbFacadeImpl.java @@ -156,6 +156,15 @@ } @Override + public List<VDS> getAllForStoragePoolAndStatus(Guid storagePool, VDSStatus status) { + return getCallsHandler().executeReadList("getVdsByStoragePoolIdWithStatus", + new VdsRowMapper(), + getCustomMapSqlParameterSource() + .addValue("storage_pool_id", storagePool) + .addValue("status", status.getValue())); + } + + @Override public List<VDS> getListForSpmSelection(Guid storagePoolId) { return getCallsHandler().executeReadList("GetUpAndPrioritizedVds", VdsRowMapper.instance, diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAOWrapperImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAOWrapperImpl.java deleted file mode 100644 index 3245e21..0000000 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAOWrapperImpl.java +++ /dev/null @@ -1,192 +0,0 @@ -package org.ovirt.engine.core.dao; - -import java.util.ArrayList; -import java.util.List; - -import org.hibernate.Query; -import org.hibernate.Session; -import org.hibernate.criterion.Restrictions; -import org.ovirt.engine.core.common.businessentities.VDS; -import org.ovirt.engine.core.common.businessentities.VDSStatus; -import org.ovirt.engine.core.common.businessentities.VDSType; -import org.ovirt.engine.core.common.businessentities.VdsDynamic; -import org.ovirt.engine.core.common.businessentities.VdsStatic; -import org.ovirt.engine.core.common.businessentities.VdsStatistics; -import org.ovirt.engine.core.compat.Guid; -import org.ovirt.engine.core.compat.NGuid; -import org.ovirt.engine.core.compat.NotImplementedException; -import org.ovirt.engine.core.dao.vds.VdsDynamicDAOHibernateImpl; -import org.ovirt.engine.core.dao.vds.VdsStaticDAOHibernateImpl; -import org.ovirt.engine.core.dao.vds.VdsStatisticsDAOHibernateImpl; - -public class VdsDAOWrapperImpl extends BaseDAOWrapperImpl implements VdsDAO { - - private final VdsStaticDAOHibernateImpl vdsStaticDAO = new VdsStaticDAOHibernateImpl(); - private final VdsDynamicDAOHibernateImpl vdsDynamicDAO = new VdsDynamicDAOHibernateImpl(); - private final VdsStatisticsDAOHibernateImpl vdsStatisticsDAO = new VdsStatisticsDAOHibernateImpl(); - - @Override - public void setSession(Session session) { - super.setSession(session); - - vdsStaticDAO.setSession(session); - vdsDynamicDAO.setSession(session); - vdsStatisticsDAO.setSession(session); - } - - @Override - public VDS get(NGuid id) { - Guid guid = new Guid(id.getUuid()); - - VdsStatic staticPart = vdsStaticDAO.get(guid); - VdsDynamic dynamicPart = vdsDynamicDAO.get(guid); - VdsStatistics statisticsPart = vdsStatisticsDAO.get(guid); - - // if we didn't find one, then return null - if (staticPart == null || dynamicPart == null || statisticsPart == null) { - return null; - } - - return new VDS(staticPart, dynamicPart, statisticsPart); - } - - @Override - public VDS get(NGuid id, Guid userID, boolean isFiltered) { - throw new NotImplementedException(); - } - - private List<VDS> convertToVdsList(List<VdsStatic> found) { - List<VDS> result = new ArrayList<VDS>(); - - for (VdsStatic vdsStatic : found) { - VdsDynamic vdsDynamic = vdsDynamicDAO.get(vdsStatic.getId()); - VdsStatistics vdsStatistics = vdsStatisticsDAO.get(vdsStatic.getId()); - - result.add(new VDS(vdsStatic, vdsDynamic, vdsStatistics)); - } - - return result; - } - - @Override - public List<VDS> getAllWithName(String name) { - List<VdsStatic> found = vdsStaticDAO.findByCriteria(Restrictions.eq("name", name)); - - return convertToVdsList(found); - } - - @Override - public List<VDS> getAllForHostname(String hostname) { - List<VdsStatic> found = vdsStaticDAO.findByCriteria(Restrictions.eq("hostname", hostname)); - - return convertToVdsList(found); - } - - @Override - public List<VDS> getAllWithUniqueId(String id) { - List<VdsStatic> found = vdsStaticDAO.findByCriteria(Restrictions.eq("uniqueId", id)); - - return convertToVdsList(found); - } - - @Override - public List<VDS> getAllOfType(VDSType vds) { - List<VdsStatic> found = vdsStaticDAO.findByCriteria(Restrictions.eq("vdsType", vds.getValue())); - - return convertToVdsList(found); - } - - @Override - public List<VDS> getAllOfTypes(VDSType[] types) { - Integer[] intTypes = new Integer[types.length]; - - for (int index = 0; index < types.length; index++) { - intTypes[index] = types[index].getValue(); - } - - List<VdsStatic> found = vdsStaticDAO.findByCriteria(Restrictions.in("vdsType", intTypes)); - - return convertToVdsList(found); - } - - @SuppressWarnings("unchecked") - @Override - public List<VDS> getAllForVdsGroupWithoutMigrating(final Guid vdsGroup) { - Session session = getSession(); - Query query = session.getNamedQuery("all_vds_static_for_vds_group_without_migration"); - - query.setParameter("vds_group_id", vdsGroup); - - return convertToVdsList(query.list()); - } - - @SuppressWarnings("unchecked") - @Override - public List<VDS> getAllWithQuery(String sql) { - Session session = getSession(); - Query query = session.createSQLQuery(sql); - - return query.list(); - } - - @Override - public List<VDS> getAll(Guid userID, boolean isFiltered) { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> getAll() { - List<VdsStatic> found = vdsStaticDAO.getAll(); - - return convertToVdsList(found); - } - - @Override - public List<VDS> getAllWithIpAddress(String address) { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> getAllForVdsGroup(Guid vdsGroup) { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> getAllForStoragePool(Guid storagePool) { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> getAllForStoragePool(Guid storagePool, Guid userID, boolean isFiltered) { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> getListForSpmSelection(Guid storagePoolId) { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> listFailedAutorecoverables() { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> getAllForVdsGroupWithStatus(Guid vdsGroupId, VDSStatus status) { - // TODO Auto-generated method stub - return null; - } - - @Override - public List<VDS> getAllForVdsGroup(Guid vdsGroup, Guid userID, boolean isFiltered) { - // TODO Auto-generated method stub - return null; - } -} diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java index 500d57a..4499766 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VdsDAOTest.java @@ -318,6 +318,12 @@ assertGetAllForVdsGroupCorrectResult(result); } + @Test + public void testGetAllForStoragePoolAndStatus() { + List<VDS> result = dao.getAllForStoragePoolAndStatus(existingVds.getstorage_pool_id(), existingVds.getstatus()); + assertCorrectGetAllResult(result); + } + private void assertGetAllForVdsGroupCorrectResult(List<VDS> result) { assertNotNull(result); assertFalse(result.isEmpty()); -- To view, visit http://gerrit.ovirt.org/8457 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1c1adda55bd27beef2907bf0ed8a0f8bc274e495 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Kublin <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
