Skip to content

Speedup resource count calculation #8903

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

Merged
yadvr merged 3 commits into from
Apr 17, 2024
Merged

Speedup resource count calculation #8903

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6225684
Speed up resource count calculation
vishesh92 Apr 10, 2024
3bbe7fc
Refactor resource count calculation
vishesh92 Apr 11, 2024
b1797bf
Start transaction for updateCountByDeltaForIds
vishesh92 Apr 12, 2024
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
7 changes: 7 additions & 0 deletions engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,15 @@ public interface ResourceCountDao extends GenericDao<ResourceCountVO, Long> {

ResourceCountVO findByOwnerAndTypeAndTag(long ownerId, ResourceOwnerType ownerType, ResourceType type, String tag);

List<ResourceCountVO> findByOwnersAndTypeAndTag(List<Long> ownerIdList, ResourceOwnerType ownerType,
ResourceType type, String tag);

List<ResourceCountVO> listResourceCountByOwnerType(ResourceOwnerType ownerType);

Set<Long> listAllRowsToUpdate(long ownerId, ResourceOwnerType ownerType, ResourceType type, String tag);

boolean updateCountByDeltaForIds(List<Long> ids, boolean increment, long delta);

Set<Long> listRowsToUpdateForDomain(long domainId, ResourceType type, String tag);

long removeEntriesByOwner(long ownerId, ResourceOwnerType ownerType);
Expand All @@ -72,4 +77,6 @@ public interface ResourceCountDao extends GenericDao<ResourceCountVO, Long> {
long countMemoryAllocatedToAccount(long accountId);

void removeResourceCountsForNonMatchingTags(Long ownerId, ResourceOwnerType ownerType, List<ResourceType> types, List<String> tags);

List<ResourceCountVO> lockRows(Set<Long> ids);
}
73 changes: 63 additions & 10 deletions engine/schema/src/main/java/com/cloud/configuration/dao/ResourceCountDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import javax.annotation.PostConstruct;
import javax.inject.Inject;
Expand Down Expand Up @@ -56,27 +57,30 @@ public class ResourceCountDaoImpl extends GenericDaoBase<ResourceCountVO, Long>
private final SearchBuilder<ResourceCountVO> TypeSearch;
private final SearchBuilder<ResourceCountVO> TypeNullTagSearch;
private final SearchBuilder<ResourceCountVO> NonMatchingTagsSearch;

private final SearchBuilder<ResourceCountVO> AccountSearch;
private final SearchBuilder<ResourceCountVO> DomainSearch;
private final SearchBuilder<ResourceCountVO> IdsSearch;

@Inject
private DomainDao _domainDao;
@Inject
private AccountDao _accountDao;

protected static final String INCREMENT_COUNT_BY_IDS_SQL = "UPDATE `cloud`.`resource_count` SET `count` = `count` + ? WHERE `id` IN (?)";
protected static final String DECREMENT_COUNT_BY_IDS_SQL = "UPDATE `cloud`.`resource_count` SET `count` = `count` - ? WHERE `id` IN (?)";

public ResourceCountDaoImpl() {
TypeSearch = createSearchBuilder();
TypeSearch.and("type", TypeSearch.entity().getType(), SearchCriteria.Op.EQ);
TypeSearch.and("accountId", TypeSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
TypeSearch.and("domainId", TypeSearch.entity().getDomainId(), SearchCriteria.Op.EQ);
TypeSearch.and("accountId", TypeSearch.entity().getAccountId(), SearchCriteria.Op.IN);
TypeSearch.and("domainId", TypeSearch.entity().getDomainId(), SearchCriteria.Op.IN);
TypeSearch.and("tag", TypeSearch.entity().getTag(), SearchCriteria.Op.EQ);
TypeSearch.done();

TypeNullTagSearch = createSearchBuilder();
TypeNullTagSearch.and("type", TypeNullTagSearch.entity().getType(), SearchCriteria.Op.EQ);
TypeNullTagSearch.and("accountId", TypeNullTagSearch.entity().getAccountId(), SearchCriteria.Op.EQ);
TypeNullTagSearch.and("domainId", TypeNullTagSearch.entity().getDomainId(), SearchCriteria.Op.EQ);
TypeNullTagSearch.and("accountId", TypeNullTagSearch.entity().getAccountId(), SearchCriteria.Op.IN);
TypeNullTagSearch.and("domainId", TypeNullTagSearch.entity().getDomainId(), SearchCriteria.Op.IN);
TypeNullTagSearch.and("tag", TypeNullTagSearch.entity().getTag(), SearchCriteria.Op.NULL);
TypeNullTagSearch.done();

Expand All @@ -90,6 +94,10 @@ public ResourceCountDaoImpl() {

AccountSearch = createSearchBuilder();
DomainSearch = createSearchBuilder();

IdsSearch = createSearchBuilder();
IdsSearch.and("id", IdsSearch.entity().getId(), SearchCriteria.Op.IN);
IdsSearch.done();
}

@PostConstruct
Expand All @@ -109,20 +117,33 @@ protected void configure() {

@Override
public ResourceCountVO findByOwnerAndTypeAndTag(long ownerId, ResourceOwnerType ownerType, ResourceType type, String tag) {
List<ResourceCountVO> resourceCounts = findByOwnersAndTypeAndTag(List.of(ownerId), ownerType, type, tag);
if (CollectionUtils.isNotEmpty(resourceCounts)) {
return resourceCounts.get(0);
} else {
return null;
}
}

@Override
public List<ResourceCountVO> findByOwnersAndTypeAndTag(List<Long> ownerIdList, ResourceOwnerType ownerType, ResourceType type, String tag) {
if (CollectionUtils.isEmpty(ownerIdList)) {
return new ArrayList<>();
}
SearchCriteria<ResourceCountVO> sc = tag != null ? TypeSearch.create() : TypeNullTagSearch.create();
sc.setParameters("type", type);
if (tag != null) {
sc.setParameters("tag", tag);
}

if (ownerType == ResourceOwnerType.Account) {
sc.setParameters("accountId", ownerId);
return findOneIncludingRemovedBy(sc);
sc.setParameters("accountId", ownerIdList.toArray());
return listIncludingRemovedBy(sc);
} else if (ownerType == ResourceOwnerType.Domain) {
sc.setParameters("domainId", ownerId);
return findOneIncludingRemovedBy(sc);
sc.setParameters("domainId", ownerIdList.toArray());
return listIncludingRemovedBy(sc);
} else {
return null;
return new ArrayList<>();
}
}

Expand Down Expand Up @@ -154,6 +175,28 @@ public boolean updateById(long id, boolean increment, long delta) {
return update(resourceCountVO.getId(), resourceCountVO);
}

@Override
public boolean updateCountByDeltaForIds(List<Long> ids, boolean increment, long delta) {
Copy link
Contributor

@DaanHoogland DaanHoogland Apr 12, 2024

Choose a reason for hiding this comment

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

would it make sense to remove increment and check for positive/negative numbers?

the higher level call to updateResourceCountForAccount is only done in two places so it wouldn't be a big refactor.

if (CollectionUtils.isEmpty(ids)) {
return false;
}
String updateSql = increment ? INCREMENT_COUNT_BY_IDS_SQL : DECREMENT_COUNT_BY_IDS_SQL;

String poolIdsInStr = ids.stream().map(String::valueOf).collect(Collectors.joining(",", "(", ")"));
String sql = updateSql.replace("(?)", poolIdsInStr);

try (TransactionLegacy txn = TransactionLegacy.currentTxn();
PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql)
) {
pstmt.setLong(1, delta);
pstmt.executeUpdate();
txn.commit();
return true;
} catch (SQLException e) {
throw new CloudRuntimeException(e);
}
}

@Override
public Set<Long> listRowsToUpdateForDomain(long domainId, ResourceType type, String tag) {
Set<Long> rowIds = new HashSet<Long>();
Expand Down Expand Up @@ -345,4 +388,14 @@ public void removeResourceCountsForNonMatchingTags(Long ownerId, ResourceOwnerTy
}
remove(sc);
}

@Override
public List<ResourceCountVO> lockRows(Set<Long> ids) {
if (CollectionUtils.isEmpty(ids)) {
return new ArrayList<>();
}
SearchCriteria<ResourceCountVO> sc = IdsSearch.create();
sc.setParameters("id", ids.toArray());
return lockRows(sc, null, true);
}
}
1 change: 1 addition & 0 deletions engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ public interface ReservationDao extends GenericDao<ReservationVO, Long> {
void setResourceId(Resource.ResourceType type, Long resourceId);
List<Long> getResourceIds(long accountId, Resource.ResourceType type);
List<ReservationVO> getReservationsForAccount(long accountId, Resource.ResourceType type, String tag);
void removeByIds(List<Long> reservationIds);
}
16 changes: 16 additions & 0 deletions engine/schema/src/main/java/org/apache/cloudstack/reservation/dao/ReservationDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
import org.apache.cloudstack.user.ResourceReservation;
import org.apache.commons.collections.CollectionUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand All @@ -40,13 +41,15 @@ public class ReservationDaoImpl extends GenericDaoBase<ReservationVO, Long> impl
private static final String RESOURCE_ID = "resourceId";
private static final String ACCOUNT_ID = "accountId";
private static final String DOMAIN_ID = "domainId";
private static final String IDS = "ids";
private final SearchBuilder<ReservationVO> listResourceByAccountAndTypeSearch;
private final SearchBuilder<ReservationVO> listAccountAndTypeSearch;
private final SearchBuilder<ReservationVO> listAccountAndTypeAndNoTagSearch;

private final SearchBuilder<ReservationVO> listDomainAndTypeSearch;
private final SearchBuilder<ReservationVO> listDomainAndTypeAndNoTagSearch;
private final SearchBuilder<ReservationVO> listResourceByAccountAndTypeAndNoTagSearch;
private final SearchBuilder<ReservationVO> listIdsSearch;

public ReservationDaoImpl() {

Expand Down Expand Up @@ -87,6 +90,10 @@ public ReservationDaoImpl() {
listDomainAndTypeAndNoTagSearch.and(RESOURCE_TYPE, listDomainAndTypeAndNoTagSearch.entity().getResourceType(), SearchCriteria.Op.EQ);
listDomainAndTypeAndNoTagSearch.and(RESOURCE_TAG, listDomainAndTypeAndNoTagSearch.entity().getTag(), SearchCriteria.Op.NULL);
listDomainAndTypeAndNoTagSearch.done();

listIdsSearch = createSearchBuilder();
listIdsSearch.and(IDS, listIdsSearch.entity().getId(), SearchCriteria.Op.IN);
listIdsSearch.done();
}

@Override
Expand Down Expand Up @@ -161,4 +168,13 @@ public List<ReservationVO> getReservationsForAccount(long accountId, Resource.Re
}
return listBy(sc);
}

@Override
public void removeByIds(List<Long> reservationIds) {
if (CollectionUtils.isNotEmpty(reservationIds)) {
SearchCriteria<ReservationVO> sc = listIdsSearch.create();
sc.setParameters(IDS, reservationIds.toArray());
remove(sc);
}
}
}
Loading