Skip to content
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.

Sign up for GitHub

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

Jump to bottom

listNetworks optimization #9096

Closed
wants to merge 1 commit into from
Closed

listNetworks optimization #9096

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 51 additions & 39 deletions server/src/main/java/com/cloud/network/NetworkServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2267,7 +2267,7 @@ public Pair<List<? extends Network>, Integer> searchForNetworks(ListNetworksCmd
isRecursive = true;
}

Filter searchFilter = new Filter(NetworkVO.class, "id", false, null, null);
Filter searchFilter = new Filter(NetworkVO.class, "id", false, cmd.getStartIndex(), cmd.getPageSizeVal());
Copy link
Member

@weizhouapache weizhouapache May 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if networkfilter is Account, Domain or Shared, it should work

However, if the networkfilter is All or AccountDomain, it might not work if the searchFilter is the same
for example, there are 25 networks for account, 25 networks for domain. if networkfilter is All or AccountDomain, and the page size is 20,

  • on first page, it returns 20 networks for account, it is good
  • on second page, it should return the last 5 networks for account, and the first 15 networks for domain. But it will return the last 5 networks for account, and the last 5 networks for domain. right ?
    I think the searchFilter needs to be updated after the each search.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very good point.
The issue here is that we are concatenating results from multiple queries ( lest's say Q1, Q2, Q3). Things kind of work for the first page. We can adjust pageSize of a query depending on how many results are already returned for other queries.

But if we want to display the second page, it is hard to say which startIndex should be used for Q2. As it depends on how many results were displayed for Q2 on page 1. Which in turn depends on how many Q1 results were there on Page 1. For example, if page 1 had 15 results for Q1 and 5 results for Q2, then page2 Q2 startIndex should be 6.

So, to fix this I think the filter should always have startIndex as 0 and pageSize should be cmd.getPageSizeVal() * (cmd.getStartIndex() + 1)

Filter searchFilter = new Filter(NetworkVO.class, "id", false, 0, (cmd.getStartIndex() + 1) * cmd.getPageSizeVal());

This way we have info on all previous page results and when pagination is applied at the end of the method, the correct result will be returned.

We are losing some optimization if startIndex is higher but it is still better than reading all rows.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there not any other API similar to this where pagination is applied where the pattern can be repeated, maybe templates, volumes, offerings?

SearchBuilder<NetworkVO> sb = _networksDao.createSearchBuilder();

if (forVpc != null) {
Expand Down Expand Up @@ -2322,41 +2322,52 @@ public Pair<List<? extends Network>, Integer> searchForNetworks(ListNetworksCmd
sb.join("associatedNetworkSearch", associatedNetworkSearch, sb.entity().getId(), associatedNetworkSearch.entity().getResourceId(), JoinBuilder.JoinType.INNER);
}

List<NetworkVO> networksToReturn = new ArrayList<NetworkVO>();
Pair<List<NetworkVO>, Integer> networks;
Pair<List<NetworkVO>, Integer> networksToReturn = new Pair<>(new ArrayList<NetworkVO>(), 0);

if (isSystem == null || !isSystem) {
if (!permittedAccounts.isEmpty()) {
if (Arrays.asList(Network.NetworkFilter.Account, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) {
//get account level networks
networksToReturn.addAll(listAccountSpecificNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, skipProjectNetworks, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, permittedAccounts));
networks = listAccountSpecificNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, skipProjectNetworks, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, permittedAccounts);
networksToReturn.first().addAll(networks.first());
networksToReturn.second(networksToReturn.second() + networks.second());
}
if (domainId != null && Arrays.asList(Network.NetworkFilter.Domain, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) {
//get domain level networks
networksToReturn.addAll(listDomainLevelNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, domainId, false));
networks = listDomainLevelNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, domainId, false);
networksToReturn.first().addAll(networks.first());
networksToReturn.second(networksToReturn.second() + networks.second());
}
if (Arrays.asList(Network.NetworkFilter.Shared, Network.NetworkFilter.All).contains(networkFilter)) {
// get shared networks
List<NetworkVO> sharedNetworks = listSharedNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
Pair<List<NetworkVO>, Integer> sharedNetworks = listSharedNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, permittedAccounts);
addNetworksToReturnIfNotExist(networksToReturn, sharedNetworks);
addNetworksToReturnIfNotExist(networksToReturn, sharedNetworks.first());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The total count of result won't be super accurate now.
As before we would have gone through all the account specific and domain level networks and gotten the union with sharedNetworks. That would have returned the exact total networks count.

But now we will be working with at max 20 account and domain level networks in the list. So there is no way to apply union with all the shared networks.

For example,
Domain level netowrks = 100
Shared networks = 100
Intersection(Domain, Shared) among all of the domain and shared networks = 50

Before this change :
Total network count = Domain n/w + shared n/w - intersection = 150

After this change:
Intersection(Domain, shared) among the first 20 domain and shared networks returned by the query = 10
Total network count = Domain n/w + shared n/w - intersection = 190

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be accurate too. Perhaps once the query returns the page, we can call a dao::getCount() passing only the list of IDs?


}
} else {
if (Arrays.asList(Network.NetworkFilter.Account, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) {
//add account specific networks
networksToReturn.addAll(listAccountSpecificNetworksByDomainPath(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, skipProjectNetworks, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, path, isRecursive));
networks = listAccountSpecificNetworksByDomainPath(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, skipProjectNetworks, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, path, isRecursive);
networksToReturn.first().addAll(networks.first());
networksToReturn.second(networksToReturn.second() + networks.second());
}
if (Arrays.asList(Network.NetworkFilter.Domain, Network.NetworkFilter.AccountDomain, Network.NetworkFilter.All).contains(networkFilter)) {
//add domain specific networks of domain + parent domains
networksToReturn.addAll(listDomainSpecificNetworksByDomainPath(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, path, isRecursive));
networks = listDomainSpecificNetworksByDomainPath(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, path, isRecursive);
networksToReturn.first().addAll(networks.first());
networksToReturn.second(networksToReturn.second() + networks.second());
//add networks of subdomains
if (domainId == null) {
networksToReturn.addAll(listDomainLevelNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, caller.getDomainId(), true));
networks = listDomainLevelNetworks(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
aclType, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter, caller.getDomainId(), true);
networksToReturn.first().addAll(networks.first());
networksToReturn.second(networksToReturn.second() + networks.second());
}
}
if (Arrays.asList(Network.NetworkFilter.Shared, Network.NetworkFilter.All).contains(networkFilter)) {
Expand All @@ -2367,11 +2378,11 @@ public Pair<List<? extends Network>, Integer> searchForNetworks(ListNetworksCmd
}
}
} else {
networksToReturn = _networksDao.search(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
networksToReturn = _networksDao.searchAndCount(buildNetworkSearchCriteria(sb, keyword, id, isSystem, zoneId, guestIpType, trafficType, physicalNetworkId, networkOfferingId,
null, true, restartRequired, specifyIpRanges, vpcId, tags, display, vlanId, associatedNetworkId), searchFilter);
}

if (supportedServicesStr != null && !supportedServicesStr.isEmpty() && !networksToReturn.isEmpty()) {
if (supportedServicesStr != null && !supportedServicesStr.isEmpty() && !networksToReturn.first().isEmpty()) {
List<NetworkVO> supportedNetworks = new ArrayList<NetworkVO>();
Service[] suppportedServices = new Service[supportedServicesStr.size()];
int i = 0;
Expand All @@ -2385,44 +2396,45 @@ public Pair<List<? extends Network>, Integer> searchForNetworks(ListNetworksCmd
i++;
}

for (NetworkVO network : networksToReturn) {
for (NetworkVO network : networksToReturn.first()) {
if (areServicesSupportedInNetwork(network.getId(), suppportedServices)) {
supportedNetworks.add(network);
}
}

networksToReturn = supportedNetworks;
networksToReturn.first(supportedNetworks);
}

if (canUseForDeploy != null) {
List<NetworkVO> networksForDeploy = new ArrayList<NetworkVO>();
for (NetworkVO network : networksToReturn) {
for (NetworkVO network : networksToReturn.first()) {
if (_networkModel.canUseForDeploy(network) == canUseForDeploy) {
networksForDeploy.add(network);
}
}

networksToReturn = networksForDeploy;
networksToReturn.first(networksForDeploy);
}

//Now apply pagination
List<? extends Network> wPagination = com.cloud.utils.StringUtils.applyPagination(networksToReturn, cmd.getStartIndex(), cmd.getPageSizeVal());
List<? extends Network> wPagination = com.cloud.utils.StringUtils.applyPagination(networksToReturn.first(), cmd.getStartIndex(), cmd.getPageSizeVal());
if (wPagination != null) {
Pair<List<? extends Network>, Integer> listWPagination = new Pair<List<? extends Network>, Integer>(wPagination, networksToReturn.size());
Pair<List<? extends Network>, Integer> listWPagination = new Pair<List<? extends Network>, Integer>(wPagination, networksToReturn.second());
return listWPagination;
}

return new Pair<List<? extends Network>, Integer>(networksToReturn, networksToReturn.size());
return new Pair<List<? extends Network>, Integer>(networksToReturn.first(), networksToReturn.second());
}

private void addNetworksToReturnIfNotExist(final List<NetworkVO> networksToReturn, final List<NetworkVO> sharedNetworks) {
Set<Long> networkIds = networksToReturn.stream()
private void addNetworksToReturnIfNotExist(final Pair<List<NetworkVO>, Integer> networksToReturn, final List<NetworkVO> sharedNetworks) {
Set<Long> networkIds = networksToReturn.first().stream()
.map(NetworkVO::getId)
.collect(Collectors.toSet());
List<NetworkVO> sharedNetworksToReturn = sharedNetworks.stream()
.filter(network -> ! networkIds.contains(network.getId()))
.collect(Collectors.toList());
networksToReturn.addAll(sharedNetworksToReturn);
networksToReturn.first().addAll(sharedNetworksToReturn);
networksToReturn.second(networksToReturn.second() + sharedNetworksToReturn.size());
}

private SearchCriteria<NetworkVO> buildNetworkSearchCriteria(SearchBuilder<NetworkVO> sb, String keyword, Long id,
Expand Down Expand Up @@ -2516,7 +2528,7 @@ private SearchCriteria<NetworkVO> buildNetworkSearchCriteria(SearchBuilder<Netwo
return sc;
}

private List<NetworkVO> listDomainLevelNetworks(SearchCriteria<NetworkVO> sc, Filter searchFilter, long domainId, boolean parentDomainsOnly) {
private Pair<List<NetworkVO>, Integer> listDomainLevelNetworks(SearchCriteria<NetworkVO> sc, Filter searchFilter, long domainId, boolean parentDomainsOnly) {
List<Long> networkIds = new ArrayList<Long>();
Set<Long> allowedDomains = _domainMgr.getDomainParentIds(domainId);
List<NetworkDomainVO> maps = _networkDomainDao.listDomainNetworkMapByDomain(allowedDomains.toArray());
Expand All @@ -2537,13 +2549,13 @@ private List<NetworkVO> listDomainLevelNetworks(SearchCriteria<NetworkVO> sc, Fi
domainSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Domain.toString());

sc.addAnd("id", SearchCriteria.Op.SC, domainSC);
return _networksDao.search(sc, searchFilter);
return _networksDao.searchAndCount(sc, searchFilter);
} else {
return new ArrayList<NetworkVO>();
return new Pair<>(new ArrayList<NetworkVO>(), 0);
}
}

private List<NetworkVO> listAccountSpecificNetworks(SearchCriteria<NetworkVO> sc, Filter searchFilter, List<Long> permittedAccounts) {
private Pair<List<NetworkVO>, Integer> listAccountSpecificNetworks(SearchCriteria<NetworkVO> sc, Filter searchFilter, List<Long> permittedAccounts) {
SearchCriteria<NetworkVO> accountSC = _networksDao.createSearchCriteria();
if (!permittedAccounts.isEmpty()) {
accountSC.addAnd("accountId", SearchCriteria.Op.IN, permittedAccounts.toArray());
Expand All @@ -2552,10 +2564,10 @@ private List<NetworkVO> listAccountSpecificNetworks(SearchCriteria<NetworkVO> sc
accountSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Account.toString());

sc.addAnd("id", SearchCriteria.Op.SC, accountSC);
return _networksDao.search(sc, searchFilter);
return _networksDao.searchAndCount(sc, searchFilter);
}

private List<NetworkVO> listAccountSpecificNetworksByDomainPath(SearchCriteria<NetworkVO> sc, Filter searchFilter, String path, boolean isRecursive) {
private Pair<List<NetworkVO>, Integer> listAccountSpecificNetworksByDomainPath(SearchCriteria<NetworkVO> sc, Filter searchFilter, String path, boolean isRecursive) {
SearchCriteria<NetworkVO> accountSC = _networksDao.createSearchCriteria();
accountSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Account.toString());

Expand All @@ -2568,10 +2580,10 @@ private List<NetworkVO> listAccountSpecificNetworksByDomainPath(SearchCriteria<N
}

sc.addAnd("id", SearchCriteria.Op.SC, accountSC);
return _networksDao.search(sc, searchFilter);
return _networksDao.searchAndCount(sc, searchFilter);
}

private List<NetworkVO> listDomainSpecificNetworksByDomainPath(SearchCriteria<NetworkVO> sc, Filter searchFilter, String path, boolean isRecursive) {
private Pair<List<NetworkVO>, Integer> listDomainSpecificNetworksByDomainPath(SearchCriteria<NetworkVO> sc, Filter searchFilter, String path, boolean isRecursive) {

Set<Long> allowedDomains = new HashSet<Long>();
if (path != null) {
Expand All @@ -2597,21 +2609,21 @@ private List<NetworkVO> listDomainSpecificNetworksByDomainPath(SearchCriteria<Ne
domainSC.addAnd("aclType", SearchCriteria.Op.EQ, ACLType.Domain.toString());

sc.addAnd("id", SearchCriteria.Op.SC, domainSC);
return _networksDao.search(sc, searchFilter);
return _networksDao.searchAndCount(sc, searchFilter);
} else {
return new ArrayList<NetworkVO>();
return new Pair<>(new ArrayList<NetworkVO>(), 0);
}
}

private List<NetworkVO> listSharedNetworks(SearchCriteria<NetworkVO> sc, Filter searchFilter, List<Long> permittedAccounts) {
private Pair<List<NetworkVO>, Integer> listSharedNetworks(SearchCriteria<NetworkVO> sc, Filter searchFilter, List<Long> permittedAccounts) {
List<Long> sharedNetworkIds = _networkPermissionDao.listPermittedNetworkIdsByAccounts(permittedAccounts);
if (!sharedNetworkIds.isEmpty()) {
SearchCriteria<NetworkVO> ssc = _networksDao.createSearchCriteria();
ssc.addAnd("id", SearchCriteria.Op.IN, sharedNetworkIds.toArray());
sc.addAnd("id", SearchCriteria.Op.SC, ssc);
return _networksDao.search(sc, searchFilter);
return _networksDao.searchAndCount(sc, searchFilter);
}
return new ArrayList<NetworkVO>();
return new Pair<>(new ArrayList<NetworkVO>(), 0);
}

private List<NetworkVO> listSharedNetworksByDomainPath(SearchCriteria<NetworkVO> sc, Filter searchFilter, String path, boolean isRecursive) {
Expand Down
Loading