Damans227 commented on code in PR #13915:
URL: https://github.com/apache/cloudstack/pull/13915#discussion_r4284236148
##########
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java:
##########
@@ -236,6 +236,149 @@ public void
doInTransactionWithoutResult(TransactionStatus status) throws CloudR
});
}
+ /**
+ * Batched increment for one ResourceType across multiple tags.
+ *
+ * <p>Why: a per-(type, tag) call chain (incrementResourceCountWithTag →
... → updateCountByDeltaForIds)
+ * issues one UPDATE per tag, each acquiring X-locks on resource_count
rows that are then held for the
+ * remainder of the surrounding transaction. With tagged storage limits
enabled, a single
+ * incrementVolumeResourceCount runs four sequential UPDATEs — concurrent
callers serialize on the
+ * shared rows and exceed innodb_lock_wait_timeout (50 s default). This
helper aggregates the row IDs
+ * for all (accountId, type, tag) entries and issues a single UPDATE,
shortening the in-transaction
+ * lock-acquire chain.
+ *
+ * <p>Behavior:
+ * <ul>
+ * <li>System accounts ({@link Account#ACCOUNT_ID_SYSTEM}) are skipped
silently.</li>
+ * <li>Empty tag list is a no-op.</li>
+ * <li>Non-positive {@code numToIncrement} is logged at WARN and the
call returns without updating.</li>
+ * <li>If row ID resolution returns no rows (no count records exist for
any tag), the call logs a
+ * WARN and returns without updating — preserving fail-loud
visibility for an unexpected state.</li>
+ * <li>Reservation entries (if any) for {@code type} on the current
{@link CallContext} are removed
+ * once per invocation, inside the same nested transaction as the
UPDATE.</li>
+ * <li>If the batched UPDATE fails, throws {@link CloudRuntimeException}
to roll back the
+ * surrounding orchestration transaction.</li>
+ * </ul>
+ *
+ * @param accountId the account whose count is being incremented;
system accounts are skipped
+ * @param type the {@link ResourceType} whose count is being
incremented
+ * @param tags the list of resource-limit tags to update;
{@code null} denotes the untagged row,
+ * non-empty entries denote tagged rows
+ * @param numToIncrement positive delta to add; non-positive values are
logged and ignored
+ */
+ @SuppressWarnings("unchecked")
+ protected void
removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ final long accountId, final ResourceType type, final List<String>
tags, final long numToIncrement) {
+ if (accountId == Account.ACCOUNT_ID_SYSTEM) {
+ logger.trace("Not incrementing resource count for system accounts,
returning");
+ return;
+ }
+ if (CollectionUtils.isEmpty(tags)) {
+ return;
+ }
+ if (numToIncrement <= 0) {
+ logger.warn(String.format("Skipping increment of resource count:
non-positive delta = %d for Account = %d Type = %s",
+ numToIncrement, accountId, type));
+ return;
+ }
+ Object obj =
CallContext.current().getContextParameter(CheckedReservation.getResourceReservationContextParameterKey(type));
+ final List<Long> reservationIds = (List<Long>) obj;
+ Transaction.execute(new
TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
+ @Override
+ public void doInTransactionWithoutResult(TransactionStatus status)
throws CloudRuntimeException {
+ reservationDao.removeByIds(reservationIds);
+ Set<Long> rowIds = collectRowIdsForTags(accountId, type, tags,
numToIncrement, true);
+ if (rowIds.isEmpty()) {
+ logger.warn("No resource_count rows resolved to increment
for Account = " + accountId
Review Comment:
any reason not to throw here like before? when no count rows are found the
create used to fail, now it only logs and carries on
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
