Liran Zelkha has uploaded a new change for review. Change subject: userportal : Fix DiskForVmGuid high CPU ......................................................................
userportal : Fix DiskForVmGuid high CPU In this patch we remove the need to load all disk data and just load the required information for the user portal. Bug URL: https://bugzilla.redhat.com/show_bug.cgi?id=971237 Change-Id: Iceb6e518936f65403561703e6b70920fbcc1f07c Signed-off-by: [email protected] <[email protected]> --- A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java A backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImageBasicView.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java M frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java M packaging/dbscripts/all_disks_sp.sql 8 files changed, 406 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/57/16657/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java new file mode 100644 index 0000000..fc3501d --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java @@ -0,0 +1,58 @@ +package org.ovirt.engine.core.bll; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.ovirt.engine.core.common.businessentities.Disk; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.VmDevice; +import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; +import org.ovirt.engine.core.common.queries.IdQueryParameters; +import org.ovirt.engine.core.common.utils.VmDeviceType; +import org.ovirt.engine.core.compat.Guid; + +public class GetAllDisksPartialDataByVmIdQuery<P extends IdQueryParameters> extends QueriesCommandBase<P> { + public GetAllDisksPartialDataByVmIdQuery(P parameters) { + super(parameters); + } + + @Override + protected void executeQueryCommand() { + List<Disk> allDisks = + getDbFacade().getDiskDao().getAllForVmPartialData + (getParameters().getId(), false, getUserID(), getParameters().isFiltered()); + // Set<Guid> pluggedDiskIds = getPluggedDiskIds(); + // List<Disk> disks = new ArrayList<Disk>(allDisks); + // for (Disk disk : allDisks) { + // disk.setPlugged(pluggedDiskIds.contains(disk.getId())); + // if (disk.getDiskStorageType() == DiskStorageType.IMAGE) { + // DiskImage diskImage = (DiskImage) disk; + // diskImage.getSnapshots().addAll(getAllImageSnapshots(diskImage)); + // } + // } + // getQueryReturnValue().setReturnValue(disks); + getQueryReturnValue().setReturnValue(allDisks); + } + + protected List<DiskImage> getAllImageSnapshots(DiskImage diskImage) { + return ImagesHandler.getAllImageSnapshots(diskImage.getImageId(), diskImage.getImageTemplateId()); + } + + private Set<Guid> getPluggedDiskIds() { + List<VmDevice> disksVmDevices = + getDbFacade().getVmDeviceDao() + .getVmDeviceByVmIdTypeAndDevice(getParameters().getId(), + VmDeviceGeneralType.DISK, + VmDeviceType.DISK.getName(), + getUserID(), + getParameters().isFiltered()); + Set<Guid> pluggedDiskIds = new HashSet<Guid>(); + for (VmDevice diskVmDevice : disksVmDevices) { + if (diskVmDevice.getIsPlugged()) { + pluggedDiskIds.add(diskVmDevice.getDeviceId()); + } + } + return pluggedDiskIds; + } +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImageBasicView.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImageBasicView.java new file mode 100644 index 0000000..18a00e9 --- /dev/null +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImageBasicView.java @@ -0,0 +1,268 @@ +package org.ovirt.engine.core.common.businessentities; + +import java.util.ArrayList; +import java.util.Date; +import javax.validation.constraints.NotNull; +import org.ovirt.engine.core.common.validation.group.CreateEntity; +import org.ovirt.engine.core.common.validation.group.UpdateEntity; +import org.ovirt.engine.core.compat.Guid; + +public class DiskImageBasicView extends DiskImage { + private static final long serialVersionUID = -1662959000228929684L; + + public DiskImageBasicView() { + super(); + } + + @Override + public long getSize() { + return super.getSize(); + } + + @Override + public void setSize(long size) { + super.setSize(size); + } + + @Override + public long getSizeInGigabytes() { + return super.getSizeInGigabytes(); + } + + @Override + public void setSizeInGigabytes(long value) { + super.setSizeInGigabytes(value); + } + + @Override + public String getDiskAlias() { + return super.getDiskAlias(); + } + + @Override + public void setDiskAlias(String diskAlias) { + super.setDiskAlias(diskAlias); + } + + @Override + public Image getImage() { + return super.getImage(); + } + + @Override + public void setImage(Image image) { + super.setImage(image); + } + + @Override + public VolumeType getVolumeType() { + return null; + } + + @Override + public void setVolumeType(VolumeType volumeType) { + } + + @Override + public VolumeFormat getVolumeFormat() { + return null; + } + + @Override + public void setvolumeFormat(VolumeFormat volumeFormat) { + } + + @Override + public Guid getQuotaId() { + return null; + } + + @Override + public void setQuotaId(Guid quotaId) { + } + + @Override + public String getQuotaName() { + return null; + } + + @Override + public void setQuotaName(String quotaName) { + } + + @Override + public boolean isQuotaDefault() { + return false; + } + + @Override + public void setIsQuotaDefault(boolean isQuotaDefault) { + } + + @Override + public QuotaEnforcementTypeEnum getQuotaEnforcementType() { + return null; + } + + @Override + public void setQuotaEnforcementType(QuotaEnforcementTypeEnum quotaEnforcementType) { + } + + @Override + public boolean isAllowSnapshot() { + return false; + } + + @Override + public DiskStorageType getDiskStorageType() { + return null; + } + + @Override + public void setId(Guid id) { + super.setId(id); + } + + @Override + public VmEntityType getVmEntityType() { + return null; + } + + @Override + public void setVmEntityType(VmEntityType vmEntityType) { + } + + @Override + public Boolean getPlugged() { + return null; + } + + @Override + public void setPlugged(Boolean plugged) { + } + + @Override + public int hashCode() { + return super.hashCode(); + } + + @Override + public boolean equals(Object obj) { + return super.equals(obj); + } + + @Override + public int getNumberOfVms() { + return 0; + } + + @Override + public void setNumberOfVms(int numberOfVms) { + } + + @Override + public ArrayList<String> getVmNames() { + return null; + } + + @Override + public void setVmNames(ArrayList<String> vmNames) { + } + + @Override + public Object getQueryableId() { + return null; + } + + @Override + public Guid getId() { + return null; + } + + @Override + @NotNull(message = "VALIDATION.DISK_INTERFACE.NOT_NULL", groups = { CreateEntity.class, UpdateEntity.class }) + public DiskInterface getDiskInterface() { + return null; + } + + @Override + public void setDiskInterface(DiskInterface diskInterface) { + } + + @Override + public boolean isWipeAfterDelete() { + return false; + } + + @Override + public boolean isWipeAfterDeleteSet() { + return false; + } + + @Override + public void setWipeAfterDelete(boolean wipeAfterDelete) { + } + + @Override + public PropagateErrors getPropagateErrors() { + return null; + } + + @Override + public void setPropagateErrors(PropagateErrors propagateErrors) { + } + + @Override + public String getDiskDescription() { + return null; + } + + @Override + public void setDiskDescription(String diskDescription) { + } + + @Override + public boolean isShareable() { + return false; + } + + @Override + public void setShareable(boolean shareable) { + } + + @Override + public boolean isBoot() { + return false; + } + + @Override + public void setBoot(boolean value) { + } + + @Override + public ScsiGenericIO getSgio() { + return null; + } + + @Override + public void setSgio(ScsiGenericIO sgio) { + } + + @Override + public DiskAlignment getAlignment() { + return null; + } + + @Override + public void setAlignment(DiskAlignment value) { + } + + @Override + public Date getLastAlignmentScan() { + return null; + } + + @Override + public void setLastAlignmentScan(Date value) { + } +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java index b218614..e3e3b77 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java @@ -257,8 +257,10 @@ GetDeviceCustomProperties(VdcQueryAuthType.User), + GetAllDisksPartialDataByVmId(VdcQueryAuthType.User), // Default type instead of having to null check Unknown(VdcQueryAuthType.User); + ; /** * What kind of authorization the query requires. Although this is essentially a <code>boolean</code>, it's diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java index a250f97..679399e 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java @@ -102,4 +102,19 @@ * @return the Disk */ Disk get(Guid id, Guid userID, boolean isFiltered); + + /** + * Retrieves all disks for the specified virtual machine id, with optional filtering. Only data for the UI basic + * screen is returned + * @param id + * the VM id + * @param onlyPluggedDisks + * whether to returned only the disks plugged to the VM or not + * @param userID + * the ID of the user requesting the information + * @param isFiltered + * Whether the results should be filtered according to the user's permissions + * @return the list of disks + */ + List<Disk> getAllForVmPartialData(Guid id, boolean onlyPluggedDisks, Guid userID, boolean isFiltered); } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java index 01d150b..73e3a36 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java @@ -3,9 +3,11 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.List; +import java.util.UUID; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType; +import org.ovirt.engine.core.common.businessentities.DiskImageBasicView; import org.ovirt.engine.core.common.businessentities.LunDisk; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.DiskImageDAODbFacadeImpl.DiskImageRowMapper; @@ -48,6 +50,21 @@ .addValue("user_id", userID) .addValue("is_filtered", isFiltered); return getCallsHandler().executeReadList("GetDisksVmGuid", DiskRowMapper.instance, parameterSource); + } + + @Override + public List<Disk> getAllForVmPartialData(Guid id, + boolean onlyPluggedDisks, + Guid userID, + boolean isFiltered) { + MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() + .addValue("vm_guid", id) + .addValue("only_plugged", onlyPluggedDisks) + .addValue("user_id", userID) + .addValue("is_filtered", isFiltered); + return getCallsHandler().executeReadList("GetDisks_VmGuid_BasicView", + DiskBasicViewRowMapper.instance, + parameterSource); } @Override @@ -113,6 +130,24 @@ } } + private static class DiskBasicViewRowMapper implements RowMapper<Disk> { + + public static DiskBasicViewRowMapper instance = new DiskBasicViewRowMapper(); + + private DiskBasicViewRowMapper() { + } + + @Override + public Disk mapRow(ResultSet rs, int rowNum) throws SQLException { + DiskImageBasicView disk = new DiskImageBasicView(); + disk.setDiskAlias(rs.getString("disk_alias")); + disk.setSize(rs.getLong("size")); + disk.setId(new Guid((UUID) rs.getObject("disk_id"))); + + return disk; + } + } + private static class LunDiskRowMapper extends AbstractDiskRowMapper<LunDisk> { public static LunDiskRowMapper instance = new LunDiskRowMapper(); diff --git a/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml b/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml index e177dbd..22e355e 100644 --- a/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml +++ b/frontend/webadmin/modules/gwt-common/src/main/resources/org/ovirt/engine/core/Common.gwt.xml @@ -22,6 +22,7 @@ <include name="common/businessentities/Disk.java" /> <include name="common/businessentities/LunDisk.java" /> <include name="common/businessentities/Image.java" /> + <include name="common/businessentities/DiskImageBasicView.java" /> <include name="common/businessentities/DiskImage.java" /> <include name="common/businessentities/DiskImageBase.java" /> <include name="common/businessentities/DiskImageDynamic.java" /> diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java index bcf8686..10e3b7f9 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java @@ -4,7 +4,7 @@ import java.util.Collections; import java.util.List; -import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.DiskImageBasicView; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmPool; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -45,9 +45,9 @@ @Override public void onSuccess(Object model, Object ReturnValue) { - List<DiskImage> disks = - (List<DiskImage>) ((VdcQueryReturnValue) ReturnValue).getReturnValue(); - ArrayList<DiskImage> diskList = new ArrayList<DiskImage>(); + List<DiskImageBasicView> disks = + (List<DiskImageBasicView>) ((VdcQueryReturnValue) ReturnValue).getReturnValue(); + ArrayList<DiskImageBasicView> diskList = new ArrayList<DiskImageBasicView>(); diskList.addAll(disks); Collections.sort(diskList, new Linq.DiskByAliasComparer()); @@ -59,7 +59,7 @@ // Since we know we have permissions for the VM, there is no need to MLA-filter the disks IdQueryParameters queryParameters = new IdQueryParameters(vm.getId()); queryParameters.setRefresh(getIsQueryFirstTime()); - Frontend.RunQuery(VdcQueryType.GetAllDisksByVmId, queryParameters, + Frontend.RunQuery(VdcQueryType.GetAllDisksPartialDataByVmId, queryParameters, _asyncQuery, Boolean.FALSE); } else if (getEntity() instanceof VmPool) @@ -84,9 +84,9 @@ @Override public void onSuccess(Object model1, Object ReturnValue) { - List<DiskImage> disks = - (List<DiskImage>) ((VdcQueryReturnValue) ReturnValue).getReturnValue(); - ArrayList<DiskImage> diskList = new ArrayList<DiskImage>(); + List<DiskImageBasicView> disks = + (List<DiskImageBasicView>) ((VdcQueryReturnValue) ReturnValue).getReturnValue(); + ArrayList<DiskImageBasicView> diskList = new ArrayList<DiskImageBasicView>(); diskList.addAll(disks); Collections.sort(diskList, new Linq.DiskByAliasComparer()); @@ -98,7 +98,7 @@ // Since we know we have permissions for the VM, there is no need to MLA-filter the disks IdQueryParameters queryParameters = new IdQueryParameters(vm.getId()); queryParameters.setRefresh(getIsQueryFirstTime()); - Frontend.RunQuery(VdcQueryType.GetAllDisksByVmId, queryParameters, + Frontend.RunQuery(VdcQueryType.GetAllDisksPartialDataByVmId, queryParameters, _asyncQuery1, Boolean.FALSE); } } diff --git a/packaging/dbscripts/all_disks_sp.sql b/packaging/dbscripts/all_disks_sp.sql index fbbd1ae..fb9c853 100644 --- a/packaging/dbscripts/all_disks_sp.sql +++ b/packaging/dbscripts/all_disks_sp.sql @@ -52,6 +52,24 @@ END; $procedure$ LANGUAGE plpgsql; +CREATE TYPE GetDisks_VmGuid_BasicView_rs AS (disk_id UUID,disk_alias varchar(255),size BIGINT); + +Create or replace FUNCTION GetDisks_VmGuid_BasicView(v_vm_guid UUID, v_only_plugged BOOLEAN, v_user_id UUID, v_is_filtered BOOLEAN) +RETURNS SETOF GetDisks_VmGuid_BasicView_rs + AS $procedure$ +BEGIN + RETURN QUERY + select disk_id,disk_alias, size + from images + LEFT OUTER JOIN base_disks ON images.image_group_id = base_disks.disk_id + LEFT JOIN vm_device ON vm_device.device_id = image_group_id AND (NOT v_only_plugged OR is_plugged) + WHERE vm_device.vm_id = v_vm_guid + AND (NOT v_is_filtered OR EXISTS (SELECT 1 + FROM user_disk_permissions_view + WHERE user_id = v_user_id AND entity_id = disk_id)); + +END; $procedure$ +LANGUAGE plpgsql; Create or replace FUNCTION GetVmBootDisk(v_vm_guid UUID) RETURNS SETOF all_disks AS $procedure$ BEGIN -- To view, visit http://gerrit.ovirt.org/16657 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iceb6e518936f65403561703e6b70920fbcc1f07c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
