Skip to content

Commit 546569b

Browse files
Pearl1594aniishadasanishadassliceofapplepiemprokopchuk
authored
Improve performance of listTemplates API (#13566)
* Bypass logic to create template_pair from 6 tables, with fallback * added bypass logic to create template_pair from 6 tables, fallback for hard filters * Removed batching, checking against per pair * follow try-with-resources design * unit tests for the bypass logic * Fix cross-zone template lookup in listTemplates Phase 2 When temp_zone_pair has dcId=0 (cross-zone template with no data_center), use IS NULL predicate instead of broken EQ/IN with literal 0 which never matches NULL rows. * Add filter for non-root domain-admin users --------- Co-authored-by: anishadas <adas33@apple.com> Co-authored-by: Aaron Chung <aaron_chung@apple.com> * Add @ignore, remove logs/comments (#824) Co-authored-by: anishadas <adas33@apple.com> (cherry picked from commit f6c9fd1081f0e1d9386db08e86485b482bf064ab) * Improve BypassTemplateView config description and rename helper method * remove logger * Fix issue with listing templates when filtered on name / keyword - count is correct, but lists all templates * listTemplates bypass: honor templatetype/isvnf/forcks, EXTERNAL ready format and view-consistent showunique paging Add templateType, isVnf and forCks to TemplateListFilter and the bypass WHERE clause, treat EXTERNAL like BAREMETAL for onlyReady, and tie-break showunique pages on CONCAT(vt.id, '_') so page boundaries match the view path. Update stale domainPath fallback comments and add unit tests. * address comments * honor arch, osCategory, extension, imageStore and domain/isrecursive scoping; fall back for storagePoolId --------- Co-authored-by: aniishadas <32420385+aniishadas@users.noreply.github.com> Co-authored-by: anishadas <adas33@apple.com> Co-authored-by: Aaron Chung <aaron_chung@apple.com> Co-authored-by: mprokopchuk <mprokopchuk@apple.com> Co-authored-by: Pearl Dsilva <pearl1954@gmail.com>
1 parent dfff32e commit 546569b

9 files changed

Lines changed: 1235 additions & 46 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/query/QueryService.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,12 @@ public interface QueryService {
140140
ConfigKey<Boolean> ReturnVmStatsOnVmList = new ConfigKey<>("Advanced", Boolean.class, "list.vm.default.details.stats", "true",
141141
"Determines whether VM stats should be returned when details are not explicitly specified in listVirtualMachines API request. When false, details default to [group, nics, secgrp, tmpl, servoff, diskoff, backoff, iso, volume, min, affgrp]. When true, all details are returned including 'stats'.", true, ConfigKey.Scope.Global);
142142

143+
ConfigKey<Boolean> BypassTemplateView = new ConfigKey<>("Advanced", Boolean.class, "template.list.bypass.view",
144+
"false",
145+
"If true, uses an optimized query path for listing templates and ISOs, which can improve performance on " +
146+
"deployments with large numbers of templates. Automatically falls back to the standard query when " +
147+
"filtering by tags, using the shared, featured, or community template filters, or when including " +
148+
"removed templates.", true, ConfigKey.Scope.Global);
143149

144150
ListResponse<UserResponse> searchForUsers(ResponseObject.ResponseView responseView, ListUsersCmd cmd) throws PermissionDeniedException;
145151

‎server/src/main/java/com/cloud/api/query/QueryManagerImpl.java‎

Lines changed: 156 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@
207207
import com.cloud.api.query.dao.SnapshotJoinDao;
208208
import com.cloud.api.query.dao.StoragePoolJoinDao;
209209
import com.cloud.api.query.dao.TemplateJoinDao;
210+
import com.cloud.api.query.dao.TemplateListFilter;
210211
import com.cloud.api.query.dao.UserAccountJoinDao;
211212
import com.cloud.api.query.dao.UserVmJoinDao;
212213
import com.cloud.api.query.dao.VolumeJoinDao;
@@ -328,6 +329,7 @@
328329
import com.cloud.storage.dao.VolumeDao;
329330
import com.cloud.tags.ResourceTagVO;
330331
import com.cloud.tags.dao.ResourceTagDao;
332+
import com.cloud.template.VirtualMachineTemplate;
331333
import com.cloud.template.VirtualMachineTemplate.State;
332334
import com.cloud.template.VirtualMachineTemplate.TemplateFilter;
333335
import com.cloud.user.Account;
@@ -4989,8 +4991,9 @@ private Pair<List<TemplateJoinVO>, Integer> searchForTemplatesInternal(Long temp
49894991
sc.setJoinParameters("storagePool", "pool_id", storagePoolId);
49904992
}
49914993

4994+
List<Long> guestOsIds = null;
49924995
if (osCategoryId != null) {
4993-
List<Long> guestOsIds = guestOSDao.listIdsByCategoryId(osCategoryId);
4996+
guestOsIds = guestOSDao.listIdsByCategoryId(osCategoryId);
49944997
if (CollectionUtils.isNotEmpty(guestOsIds)) {
49954998
sc.setParameters("guestOsIdIN", guestOsIds.toArray());
49964999
} else {
@@ -5146,7 +5149,12 @@ else if (!template.isPublicTemplate() && caller.getType() != Account.Type.ADMIN)
51465149
applyPublicTemplateSharingRestrictions(sc, caller);
51475150

51485151
return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller,
5149-
showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc);
5152+
showRemovedTmpl, parentTemplateId, showUnique, templateType, isVnf, forCks, searchFilter, sc,
5153+
buildTemplateListFilter(templateId, ids, name, keyword, templateFilter, isIso, bootable,
5154+
pageSize, startIndex, zoneId, hyperType, hypers, showDomr, onlyReady,
5155+
permittedAccounts, caller, listProjectResourcesCriteria, tags, showRemovedTmpl,
5156+
parentTemplateId, showUnique, templateType, isVnf, forCks, arch, guestOsIds, extensionId,
5157+
imageStoreId, storagePoolId, domainId, isRecursive));
51505158
}
51515159

51525160
/**
@@ -5201,7 +5209,8 @@ protected boolean checkIfDomainSharesTemplates(Long domainId) {
52015209
private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<HypervisorType> hypers, Map<String, String> tags, String name, String keyword,
52025210
HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, Account caller,
52035211
boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique, String templateType, Boolean isVnf, Boolean forCks,
5204-
Filter searchFilter, SearchCriteria<TemplateJoinVO> sc) {
5212+
Filter searchFilter, SearchCriteria<TemplateJoinVO> sc,
5213+
TemplateListFilter listFilter) {
52055214
if (!isIso) {
52065215
// add hypervisor criteria for template case
52075216
if (hypers != null && !hypers.isEmpty()) {
@@ -5308,6 +5317,14 @@ private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<H
53085317

53095318
// search unique templates and find details by Ids
53105319
Pair<List<TemplateJoinVO>, Integer> uniqueTmplPair;
5320+
5321+
// Only attempted when the bypass DAO supports the filter set; falls
5322+
// back to the SearchBuilder/template_view path below otherwise.
5323+
if (!showRemovedTmpl && BypassTemplateView.value() && listFilter != null && listFilter.canBypass()) {
5324+
uniqueTmplPair = _templateJoinDao.findDistinctTempZonePairs(listFilter);
5325+
return findTemplatesByIdOrTempZonePair(uniqueTmplPair, showRemovedTmpl, showUnique != null && showUnique, caller);
5326+
}
5327+
53115328
if (showRemovedTmpl) {
53125329
uniqueTmplPair = _templateJoinDao.searchIncludingRemovedAndCount(sc, searchFilter);
53135330
} else {
@@ -5329,6 +5346,140 @@ private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<H
53295346
// about what the code was doing.
53305347
}
53315348

5349+
/**
5350+
* Build the immutable filter the bypass-the-view DAO consumes. Mirrors the
5351+
* SearchCriteria construction in {@link #searchForTemplatesInternal}; the
5352+
* SearchBuilder path is unchanged. The "hard" markers (sharedAccountIds,
5353+
* domainIdsForFeaturedCommunity, tags, requiresViewFallback) drive
5354+
* {@link TemplateListFilter#canBypass()} — when any are populated,
5355+
* {@code templateChecks} routes to the legacy view-based path instead.
5356+
*/
5357+
private TemplateListFilter buildTemplateListFilter(Long templateId, List<Long> ids, String name, String keyword,
5358+
TemplateFilter templateFilter, boolean isIso, Boolean bootable,
5359+
Long pageSize, Long startIndex, Long zoneId, HypervisorType hyperType,
5360+
List<HypervisorType> hypers, boolean showDomr, boolean onlyReady,
5361+
List<Account> permittedAccounts, Account caller,
5362+
ListProjectResourcesCriteria listProjectResourcesCriteria,
5363+
Map<String, String> tags, boolean showRemovedTmpl, Long parentTemplateId,
5364+
Boolean showUnique, String templateType, Boolean isVnf, Boolean forCks,
5365+
CPU.CPUArch arch, List<Long> guestOsIds, Long extensionId, Long imageStoreId,
5366+
Long storagePoolId, Long domainId, boolean isRecursive) {
5367+
TemplateListFilter.Builder b = TemplateListFilter.builder()
5368+
.templateId(templateId)
5369+
.ids(ids == null ? null : new ArrayList<>(ids))
5370+
.name(name)
5371+
.keyword(keyword)
5372+
.hypervisorType(hyperType != null && !HypervisorType.None.equals(hyperType) ? hyperType : null)
5373+
.availableHypervisors(hypers)
5374+
.format(ImageFormat.ISO) // bypass DAO uses isIso flag to flip between EQ and NEQ
5375+
.isIso(isIso)
5376+
.bootable(bootable)
5377+
.parentTemplateId(parentTemplateId)
5378+
.zoneId(zoneId)
5379+
.onlyReady(onlyReady)
5380+
.excludeSystemTemplates(!showDomr)
5381+
.templateType(templateType)
5382+
.isVnf(isVnf)
5383+
.forCks(forCks)
5384+
.arch(arch)
5385+
.guestOsIds(guestOsIds)
5386+
.extensionId(extensionId)
5387+
.imageStoreId(imageStoreId)
5388+
.showUnique(showUnique != null && showUnique)
5389+
.startIndex(startIndex)
5390+
.pageSize(pageSize)
5391+
.sortAscending(SortKeyAscending.value())
5392+
.showRemoved(showRemovedTmpl)
5393+
.tags(tags);
5394+
5395+
if (!showRemovedTmpl) {
5396+
b.templateStates(Arrays.asList(VirtualMachineTemplate.State.Active,
5397+
VirtualMachineTemplate.State.UploadAbandoned,
5398+
VirtualMachineTemplate.State.UploadError,
5399+
VirtualMachineTemplate.State.NotUploaded,
5400+
VirtualMachineTemplate.State.UploadInProgress));
5401+
}
5402+
5403+
// accountType filter (Project vs non-Project), applied only when no specific templateId.
5404+
if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
5405+
b.accountTypeNeq(Account.Type.PROJECT);
5406+
} else if (templateId == null && listProjectResourcesCriteria == ListProjectResourcesCriteria.ListProjectResourcesOnly) {
5407+
b.accountTypeEq(Account.Type.PROJECT);
5408+
}
5409+
5410+
// ACL handling — mirrors the templateFilter switch in searchForTemplatesInternal.
5411+
// For "hard" templateFilter values, set requiresViewFallback so canBypass() returns
5412+
// false and the dispatcher falls back to the legacy view-based path.
5413+
if (templateId != null) {
5414+
return b.build();
5415+
}
5416+
5417+
// template_spool_ref join is not modeled in bypass SQL.
5418+
if (storagePoolId != null) {
5419+
b.requiresViewFallback(true);
5420+
}
5421+
5422+
List<Long> permittedAccountIds = new ArrayList<>();
5423+
for (Account account : permittedAccounts) {
5424+
permittedAccountIds.add(account.getId());
5425+
}
5426+
5427+
if (templateFilter == TemplateFilter.featured || templateFilter == TemplateFilter.community) {
5428+
// Walks the domain hierarchy and ORs domainId IS NULL — not yet modeled in bypass SQL.
5429+
b.publicTemplate(Boolean.TRUE);
5430+
b.featured(templateFilter == TemplateFilter.featured ? Boolean.TRUE : Boolean.FALSE);
5431+
b.requiresViewFallback(true);
5432+
} else if (templateFilter == TemplateFilter.self || templateFilter == TemplateFilter.selfexecutable) {
5433+
if (accountMgr.isAdmin(caller.getAccountId())) {
5434+
applySelfDomainScope(b, permittedAccounts, caller, domainId, isRecursive);
5435+
}
5436+
if (!permittedAccountIds.isEmpty()) {
5437+
b.accountIds(permittedAccountIds);
5438+
b.accountIdRequiredForFilter(true);
5439+
}
5440+
} else if (templateFilter == TemplateFilter.sharedexecutable || templateFilter == TemplateFilter.shared) {
5441+
// launch_permission join — not modeled in bypass SQL.
5442+
b.requiresViewFallback(true);
5443+
} else if (templateFilter == TemplateFilter.executable) {
5444+
b.publicOrAccountIdComposite(true);
5445+
if (!permittedAccountIds.isEmpty()) {
5446+
b.accountIds(permittedAccountIds);
5447+
}
5448+
} else if (templateFilter == TemplateFilter.all && caller.getType() != Account.Type.ADMIN) {
5449+
// Non-admin "all" composite splits on listProjectResourcesCriteria:
5450+
// SkipProjectResources → (public OR domainPath LIKE) — bypass-eligible
5451+
// else → (public OR account_id IN OR sharedAccountId IN) — needs launch_permission, falls back
5452+
if (listProjectResourcesCriteria == ListProjectResourcesCriteria.SkipProjectResources) {
5453+
DomainVO callerDomain = _domainDao.findById(caller.getDomainId());
5454+
if (callerDomain != null) {
5455+
b.publicOrDomainPathComposite(true);
5456+
b.domainPathLike(callerDomain.getPath() + "%");
5457+
} else {
5458+
b.requiresViewFallback(true);
5459+
}
5460+
} else {
5461+
b.requiresViewFallback(true);
5462+
}
5463+
}
5464+
5465+
return b.build();
5466+
}
5467+
5468+
// Mirrors the admin domain-path scoping of the view path for self/selfexecutable.
5469+
private void applySelfDomainScope(TemplateListFilter.Builder b, List<Account> permittedAccounts, Account caller,
5470+
Long domainId, boolean isRecursive) {
5471+
Long domainIdForScope = !permittedAccounts.isEmpty()
5472+
? permittedAccounts.get(0).getDomainId()
5473+
: Objects.requireNonNullElse(domainId, caller.getDomainId());
5474+
DomainVO scopeDomain = _domainDao.findById(domainIdForScope);
5475+
// A recursive scope from the root domain matches every row, so skip the domain join.
5476+
if (scopeDomain == null || (isRecursive && scopeDomain.getId() == Domain.ROOT_DOMAIN)) {
5477+
return;
5478+
}
5479+
b.domainPathLike(isRecursive ? scopeDomain.getPath() + "%" : scopeDomain.getPath());
5480+
b.domainPathExact(!isRecursive);
5481+
}
5482+
53325483
// findTemplatesByIdOrTempZonePair returns the templates with the given ids if showUnique is true, or else by the TempZonePair
53335484
private Pair<List<TemplateJoinVO>, Integer> findTemplatesByIdOrTempZonePair(Pair<List<TemplateJoinVO>, Integer> templateDataPair,
53345485
boolean showRemoved, boolean showUnique, Account caller) {
@@ -6342,7 +6493,8 @@ public String getConfigComponentName() {
63426493
@Override
63436494
public ConfigKey<?>[] getConfigKeys() {
63446495
return new ConfigKey<?>[] {AllowUserViewDestroyedVM, UserVMDeniedDetails, UserVMReadOnlyDetails, SortKeyAscending,
6345-
AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains, ReturnVmStatsOnVmList};
6496+
AllowUserViewAllDomainAccounts, AllowUserViewAllDataCenters, SharePublicTemplatesWithOtherDomains,
6497+
ReturnVmStatsOnVmList, BypassTemplateView };
63466498
}
63476499

63486500
@Override

‎server/src/main/java/com/cloud/api/query/dao/TemplateJoinDao.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,20 @@ public interface TemplateJoinDao extends GenericDao<TemplateJoinVO, Long> {
5252

5353
Pair<List<TemplateJoinVO>, Integer> searchIncludingRemovedAndCount(final SearchCriteria<TemplateJoinVO> sc, final Filter filter);
5454

55+
/**
56+
* Bypass-the-view Phase 1 implementation. Issues a hand-tuned SQL query
57+
* directly against the underlying tables (vm_template, account,
58+
* template_store_ref, image_store, template_zone_ref, data_center) instead
59+
* of going through {@code template_view}. Caller must check
60+
* {@link TemplateListFilter#canBypass()} first; this method does not handle
61+
* tags, sharedAccountIds, or featured/community-style domainId
62+
* filters and will throw IllegalArgumentException if those are populated.
63+
*
64+
* Returns TemplateJoinVO objects with only {@code id} (when
65+
* {@code filter.showUnique}) or {@code tempZonePair} populated, plus the
66+
* total distinct count for pagination.
67+
*/
68+
Pair<List<TemplateJoinVO>, Integer> findDistinctTempZonePairs(TemplateListFilter filter);
69+
5570
List<TemplateJoinVO> findByDistinctIds(Long... ids);
5671
}

0 commit comments

Comments
 (0)