-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Improve performance of listTemplates API #13566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.22
Are you sure you want to change the base?
Changes from all commits
3fd789b
cf6beb4
54d4392
222cf25
21955d9
42fe302
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,14 @@ | |
| import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; | ||
| import com.cloud.cluster.ManagementServerHostPeerJoinVO; | ||
|
|
||
| import com.cloud.template.VirtualMachineTemplate; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: these new imports break alphabetical order here, and the same classes ( |
||
| import com.cloud.user.AccountVO; | ||
| import com.cloud.user.SSHKeyPairVO; | ||
| import com.cloud.user.dao.SSHKeyPairDao; | ||
| import com.cloud.vm.InstanceGroupVMMapVO; | ||
| import com.cloud.vm.NicVO; | ||
| import com.cloud.vm.dao.InstanceGroupVMMapDao; | ||
| import com.cloud.vm.dao.NicDao; | ||
| import com.cloud.vm.UserVmManager; | ||
| import org.apache.cloudstack.acl.ControlledEntity; | ||
| import org.apache.cloudstack.acl.ControlledEntity.ACLType; | ||
|
|
@@ -207,6 +215,7 @@ | |
| import com.cloud.api.query.dao.SnapshotJoinDao; | ||
| import com.cloud.api.query.dao.StoragePoolJoinDao; | ||
| import com.cloud.api.query.dao.TemplateJoinDao; | ||
| import com.cloud.api.query.dao.TemplateListFilter; | ||
| import com.cloud.api.query.dao.UserAccountJoinDao; | ||
| import com.cloud.api.query.dao.UserVmJoinDao; | ||
| import com.cloud.api.query.dao.VolumeJoinDao; | ||
|
|
@@ -332,12 +341,9 @@ | |
| import com.cloud.template.VirtualMachineTemplate.TemplateFilter; | ||
| import com.cloud.user.Account; | ||
| import com.cloud.user.AccountManager; | ||
| import com.cloud.user.AccountVO; | ||
| import com.cloud.user.DomainManager; | ||
| import com.cloud.user.SSHKeyPairVO; | ||
| import com.cloud.user.User; | ||
| import com.cloud.user.dao.AccountDao; | ||
| import com.cloud.user.dao.SSHKeyPairDao; | ||
| import com.cloud.user.dao.UserDao; | ||
| import com.cloud.utils.DateUtil; | ||
| import com.cloud.utils.NumbersUtil; | ||
|
|
@@ -353,17 +359,13 @@ | |
| import com.cloud.utils.db.SearchCriteria.Op; | ||
| import com.cloud.utils.exception.CloudRuntimeException; | ||
| import com.cloud.vm.DomainRouterVO; | ||
| import com.cloud.vm.InstanceGroupVMMapVO; | ||
| import com.cloud.vm.NicVO; | ||
| import com.cloud.vm.UserVmVO; | ||
| import com.cloud.vm.VMInstanceDetailVO; | ||
| import com.cloud.vm.VMInstanceVO; | ||
| import com.cloud.vm.VirtualMachine; | ||
| import com.cloud.vm.VirtualMachineManager; | ||
| import com.cloud.vm.VmDetailConstants; | ||
| import com.cloud.vm.dao.DomainRouterDao; | ||
| import com.cloud.vm.dao.InstanceGroupVMMapDao; | ||
| import com.cloud.vm.dao.NicDao; | ||
| import com.cloud.vm.dao.UserVmDao; | ||
| import com.cloud.vm.dao.VMInstanceDao; | ||
| import com.cloud.vm.dao.VMInstanceDetailsDao; | ||
|
|
@@ -5089,7 +5091,11 @@ | |
| applyPublicTemplateSharingRestrictions(sc, caller); | ||
|
|
||
| return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller, | ||
| showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc); | ||
| showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc, | ||
| buildTemplateListFilter(templateId, ids, name, keyword, templateFilter, isIso, bootable, | ||
| pageSize, startIndex, zoneId, hyperType, hypers, showDomr, onlyReady, | ||
| permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedTmpl, | ||
| parentTemplateId, showUnique)); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -5144,7 +5150,8 @@ | |
| private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<HypervisorType> hypers, Map<String, String> tags, String name, String keyword, | ||
| HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, Account caller, | ||
| boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique, String templateType, Boolean isVnf, Boolean forCks, | ||
| Filter searchFilter, SearchCriteria<TemplateJoinVO> sc) { | ||
| Filter searchFilter, SearchCriteria<TemplateJoinVO> sc, | ||
| TemplateListFilter listFilter) { | ||
| if (!isIso) { | ||
| // add hypervisor criteria for template case | ||
| if (hypers != null && !hypers.isEmpty()) { | ||
|
|
@@ -5251,6 +5258,14 @@ | |
|
|
||
| // search unique templates and find details by Ids | ||
| Pair<List<TemplateJoinVO>, Integer> uniqueTmplPair; | ||
|
|
||
| // Only attempted when the bypass DAO supports the filter set; falls | ||
| // back to the SearchBuilder/template_view path below otherwise. | ||
| if (!showRemovedTmpl && BypassTemplateView.value() && listFilter != null && listFilter.canBypass()) { | ||
|
Check warning on line 5264 in server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
|
||
| uniqueTmplPair = _templateJoinDao.findDistinctTempZonePairs(listFilter); | ||
| return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique != null && showUnique, caller); | ||
| } | ||
|
|
||
| if (showRemovedTmpl) { | ||
| uniqueTmplPair = _templateJoinDao.searchIncludingRemovedAndCount(sc, searchFilter); | ||
| } else { | ||
|
|
@@ -5272,6 +5287,119 @@ | |
| // about what the code was doing. | ||
| } | ||
|
|
||
| /** | ||
| * Build the immutable filter the bypass-the-view DAO consumes. Mirrors the | ||
| * SearchCriteria construction in {@link #searchForTemplatesInternal}; the | ||
| * SearchBuilder path is unchanged. The "hard" markers (sharedAccountIds, | ||
| * domainPathLike, domainIdsForFeaturedCommunity, tags) drive | ||
| * {@link TemplateListFilter#canBypass()} — when any are populated, | ||
| * {@code templateChecks} routes to the legacy view-based path instead. | ||
| */ | ||
| private TemplateListFilter buildTemplateListFilter(Long templateId, List<Long> ids, String name, String keyword, | ||
|
Check failure on line 5298 in server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
|
||
| TemplateFilter templateFilter, boolean isIso, Boolean bootable, | ||
| Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType, | ||
| List<HypervisorType> hypers, boolean showDomr, boolean onlyReady, | ||
| List<Account> permittedAccounts, Account caller, | ||
| ListProjectResourcesCriteria listProjectResourcesCriteria, | ||
| Map<String, String> tags, boolean showRemovedTmpl, Long parentTemplateId, | ||
| Boolean showUnique) { | ||
| TemplateListFilter.Builder b = TemplateListFilter.builder() | ||
| .templateId(templateId) | ||
| .ids(ids == null ? null : new ArrayList<>(ids)) | ||
| .name(name) | ||
| .keyword(keyword) | ||
| .hypervisorType(hyperType != null && !HypervisorType.None.equals(hyperType) ? hyperType : null) | ||
| .availableHypervisors(hypers) | ||
| .format(ImageFormat.ISO) // bypass DAO uses isIso flag to flip between EQ and NEQ | ||
| .isIso(isIso) | ||
| .bootable(bootable) | ||
| .parentTemplateId(parentTemplateId) | ||
| .zoneId(zoneId) | ||
| .onlyReady(onlyReady) | ||
| .excludeSystemTemplates(!showDomr) | ||
| .showUnique(showUnique != null && showUnique) | ||
| .startIndex(startIndex) | ||
| .pageSize(pageSize) | ||
| .sortAscending(SortKeyAscending.value()) | ||
| .showRemoved(showRemovedTmpl) | ||
| .tags(tags); | ||
|
|
||
| if (!showRemovedTmpl) { | ||
| b.templateStates(Arrays.asList(VirtualMachineTemplate.State.Active, | ||
| VirtualMachineTemplate.State.UploadAbandoned, | ||
| VirtualMachineTemplate.State.UploadError, | ||
| VirtualMachineTemplate.State.NotUploaded, | ||
| VirtualMachineTemplate.State.UploadInProgress)); | ||
| } | ||
|
|
||
| // accountType filter (Project vs non-Project), applied only when no specific templateId. | ||
| if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) { | ||
| b.accountTypeNeq(Account.Type.PROJECT); | ||
| } else if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.ListProjectResourcesOnly) { | ||
| b.accountTypeEq(Account.Type.PROJECT); | ||
| } | ||
|
|
||
| // ACL handling — mirrors the templateFilter switch in searchForTemplatesInternal. | ||
| // For "hard" templateFilter values, set requiresViewFallback so canBypass() returns | ||
| // false and the dispatcher falls back to the legacy view-based path. | ||
| if (templateId != null) { | ||
| return b.build(); | ||
| } | ||
|
|
||
| List<Long> permittedAccountIds = new ArrayList<>(); | ||
| for (Account account : permittedAccounts) { | ||
| permittedAccountIds.add(account.getId()); | ||
| } | ||
|
|
||
| if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) { | ||
| // Walks the domain hierarchy and ORs domainId IS NULL — not yet modeled in bypass SQL. | ||
| b.publicTemplate(Boolean.TRUE); | ||
| b.featured(templateFilter == TemplateFilter.featured ? Boolean.TRUE : Boolean.FALSE); | ||
| b.requiresViewFallback(true); | ||
| } else if (templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable) { | ||
| if (caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { | ||
| // Match the existing path's domain resolution (see searchForTemplatesInternal:4498-4502): | ||
| // scope to the queried account's domain when one was specified, else the caller's. | ||
| Long domainIdForScope = !permittedAccounts.isEmpty() | ||
| ? permittedAccounts.get(0).getDomainId() | ||
| : caller.getDomainId(); | ||
| DomainVO scopeDomain = _domainDao.findById(domainIdForScope); | ||
| if (scopeDomain != null) { | ||
| b.domainPathLike(scopeDomain.getPath() + "%"); | ||
| } | ||
| } | ||
| if (!permittedAccountIds.isEmpty()) { | ||
| b.accountIds(permittedAccountIds); | ||
| b.accountIdRequiredForFilter(true); | ||
| } | ||
| } else if (templateFilter == TemplateFilter.sharedexecutable || templateFilter == TemplateFilter.shared) { | ||
| // launch_permission join — not modeled in bypass SQL. | ||
| b.requiresViewFallback(true); | ||
| } else if (templateFilter == TemplateFilter.executable) { | ||
| b.publicOrAccountIdComposite(true); | ||
| if (!permittedAccountIds.isEmpty()) { | ||
| b.accountIds(permittedAccountIds); | ||
| } | ||
| } else if (templateFilter == TemplateFilter.all && caller.getType() != Account.Type.ADMIN) { | ||
| // Non-admin "all" composite splits on listProjectResourcesCriteria: | ||
| // SkipProjectResources → (public OR domainPath LIKE) — bypass-eligible | ||
| // else → (public OR account_id IN OR sharedAccountId IN) — needs launch_permission, falls back | ||
| if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) { | ||
| DomainVO callerDomain = _domainDao.findById(caller.getDomainId()); | ||
| if (callerDomain != null) { | ||
| b.publicOrDomainPathComposite(true); | ||
| b.domainPathLike(callerDomain.getPath() + "%"); | ||
| } else { | ||
| b.requiresViewFallback(true); | ||
| } | ||
| } else { | ||
| b.requiresViewFallback(true); | ||
| } | ||
| } | ||
|
|
||
| return b.build(); | ||
| } | ||
|
|
||
| // findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair | ||
| private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair, | ||
| boolean showRemoved, boolean showUnique, Account caller) { | ||
|
|
@@ -6285,7 +6413,8 @@ | |
| @Override | ||
| public ConfigKey<?>[] getConfigKeys() { | ||
| return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending, | ||
| AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains, ReturnVmStatsOnVmList}; | ||
| AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains, | ||
| ReturnVmStatsOnVmList, BypassTemplateView }; | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: the concatenated description string has inconsistent leading/trailing space placement across the lines, worth double-checking the rendered text doesn't end up with a missing or doubled space.