rowIds = collectRowIdsForTags(accountId, type, tags, numToIncrement, true);
+ if (rowIds.isEmpty()) {
+ logger.warn("No resource_count rows resolved to increment for Account = " + accountId
+ + " Type = " + type + " tags = " + tags + "; skipping update");
+ return;
+ }
+ if (!_resourceCountDao.updateCountByDeltaForIds(new ArrayList<>(rowIds), true, numToIncrement)) {
+ throw new CloudRuntimeException("Failed to increment resource count of type " + type
+ + " for account id=" + accountId);
+ }
+ }
+ });
+ }
+
+ /**
+ * Batched decrement counterpart of {@link #removeResourceReservationIfNeededAndIncrementResourceCountForTags}.
+ *
+ * Behavior mirrors the increment helper, with one difference: a failed UPDATE raises an
+ * {@link AlertManager.AlertType#ALERT_TYPE_UPDATE_RESOURCE_COUNT} alert rather than throwing.
+ * This matches the historical per-tag {@code decrementResourceCountWithTag} semantics — a stale
+ * count is recoverable via the {@code updateResourceCount} API, so the orchestration transaction
+ * is not aborted on a decrement failure.
+ *
+ *
No nested transaction is opened — callers that need atomicity with surrounding work should
+ * wrap the call in their own {@link Transaction#execute}.
+ *
+ * @param accountId the account whose count is being decremented; system accounts are skipped
+ * @param type the {@link ResourceType} whose count is being decremented
+ * @param tags the list of resource-limit tags to update; {@code null} denotes the untagged row
+ * @param numToDecrement positive delta to subtract; non-positive values are logged and ignored
+ */
+ protected void decrementResourceCountForTags(final long accountId, final ResourceType type,
+ final List tags, final long numToDecrement) {
+ if (accountId == Account.ACCOUNT_ID_SYSTEM) {
+ logger.trace("Not decrementing resource count for system accounts, returning");
+ return;
+ }
+ if (CollectionUtils.isEmpty(tags)) {
+ return;
+ }
+ if (numToDecrement <= 0) {
+ logger.warn(String.format("Skipping decrement of resource count: non-positive delta = %d for Account = %d Type = %s",
+ numToDecrement, accountId, type));
+ return;
+ }
+ Set rowIds = collectRowIdsForTags(accountId, type, tags, numToDecrement, false);
+ if (rowIds.isEmpty()) {
+ logger.warn("No resource_count rows resolved to decrement for Account = " + accountId
+ + " Type = " + type + " tags = " + tags + "; skipping update");
+ return;
+ }
+ if (!_resourceCountDao.updateCountByDeltaForIds(new ArrayList<>(rowIds), false, numToDecrement)) {
+ _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L,
+ "Failed to decrement resource count of type " + type + " for account id=" + accountId,
+ "Failed to decrement resource count of type " + type + " for account id=" + accountId
+ + "; use updateResourceCount API to recalculate/fix the problem");
+ }
+ }
+
+ /**
+ * Resolve the union of {@code resource_count} row IDs that need to be updated for an account
+ * across a list of tags, deduplicated. Logs the per-tag debug line that the legacy per-tag
+ * path emitted from {@link #updateResourceCountForAccount}, preserving log parity for operators.
+ *
+ * {@code null} in {@code tags} is a sentinel for the untagged row. Each tag is resolved
+ * via {@link ResourceCountDao#listAllRowsToUpdate} which returns the account's own row plus the
+ * row for every parent domain in the chain.
+ *
+ * @param accountId account owner of the resource counts
+ * @param type resource type
+ * @param tags tag list ({@code null} sentinel for untagged, plus any tagged entries)
+ * @param delta delta value, used only for the human-readable debug log
+ * @param increment {@code true} for an upcoming increment, {@code false} for decrement; affects
+ * the debug log wording only
+ * @return deduplicated set of row IDs across all tags; may be empty if no count records exist
+ */
+ private Set collectRowIdsForTags(long accountId, ResourceType type, List tags, long delta, boolean increment) {
+ Set rowIds = new HashSet<>();
+ for (String tag : tags) {
+ if (logger.isDebugEnabled()) {
+ String convertedDelta = (type == ResourceType.secondary_storage || type == ResourceType.primary_storage)
+ ? toHumanReadableSize(delta) : String.valueOf(delta);
+ String typeStr = StringUtils.isNotEmpty(tag)
+ ? String.format("%s (tag: %s)", type, tag) : type.getName();
+ logger.debug("Updating resource Type = " + typeStr + " count for Account = " + accountId
+ + " Operation = " + (increment ? "increasing" : "decreasing") + " Amount = " + convertedDelta);
+ }
+ rowIds.addAll(_resourceCountDao.listAllRowsToUpdate(accountId, ResourceOwnerType.Account, type, tag));
+ }
+ return rowIds;
+ }
+
private void cleanupResourceReservationsForMs() {
int reservationsRemoved = reservationDao.removeByMsId(ManagementServerNode.getManagementServerId());
if (reservationsRemoved > 0) {
@@ -1810,11 +1953,12 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
if (CollectionUtils.isEmpty(tags)) {
return;
}
- for (String tag : tags) {
- incrementResourceCountWithTag(accountId, ResourceType.volume, tag);
- if (size != null) {
- incrementResourceCountWithTag(accountId, ResourceType.primary_storage, tag, size);
- }
+ // Single batched UPDATE per ResourceType across the full (untagged + tagged) tag list,
+ // instead of one UPDATE per tag. Cuts the in-transaction row-lock acquire chain in half
+ // and avoids cross-tag waits exceeding innodb_lock_wait_timeout under concurrent restores.
+ removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, ResourceType.volume, tags, 1L);
+ if (size != null) {
+ removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, ResourceType.primary_storage, tags, size);
}
}
});
@@ -1830,11 +1974,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
if (CollectionUtils.isEmpty(tags)) {
return;
}
- for (String tag : tags) {
- decrementResourceCountWithTag(accountId, ResourceType.volume, tag);
- if (size != null) {
- decrementResourceCountWithTag(accountId, ResourceType.primary_storage, tag, size);
- }
+ decrementResourceCountForTags(accountId, ResourceType.volume, tags, 1L);
+ if (size != null) {
+ decrementResourceCountForTags(accountId, ResourceType.primary_storage, tags, size);
}
}
});
@@ -1991,9 +2133,9 @@ public void incrementVolumePrimaryStorageResourceCount(long accountId, Boolean d
if (CollectionUtils.isEmpty(tags)) {
return;
}
- for (String tag : tags) {
- incrementResourceCountWithTag(accountId, ResourceType.primary_storage, tag, size);
- }
+ // Batched: one UPDATE across all (untagged + tagged) rows for primary_storage. Same rationale
+ // as incrementVolumeResourceCount — reduces in-transaction lock-acquire chain on resize paths.
+ removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, ResourceType.primary_storage, tags, size);
}
@Override
@@ -2005,9 +2147,7 @@ public void decrementVolumePrimaryStorageResourceCount(long accountId, Boolean d
if (CollectionUtils.isEmpty(tags)) {
return;
}
- for (String tag : tags) {
- decrementResourceCountWithTag(accountId, ResourceType.primary_storage, tag, size);
- }
+ decrementResourceCountForTags(accountId, ResourceType.primary_storage, tags, size);
}
protected List getResourceLimitHostTagsForResourceCountOperation(Boolean display, ServiceOffering serviceOffering, VirtualMachineTemplate template) {
diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
index 83619c92e8f7..e1fe3dddf982 100644
--- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
+++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
@@ -975,19 +975,42 @@ public void testIncrementVolumeResourceCount() {
Mockito.doReturn(new ArrayList<>()).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
resourceLimitManager.incrementVolumeResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
- Mockito.verify(resourceLimitManager, Mockito.never()).incrementResourceCountWithTag(Mockito.anyLong(),
- Mockito.eq(Resource.ResourceType.volume), Mockito.anyString());
- Mockito.verify(resourceLimitManager, Mockito.never()).incrementResourceCountWithTag(Mockito.anyLong(),
- Mockito.eq(Resource.ResourceType.primary_storage), Mockito.anyString(), Mockito.anyLong());
+ Mockito.verify(resourceLimitManager, Mockito.never()).removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
Mockito.doReturn(List.of(tag)).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
- mockIncrementResourceCountWithTag();
+ Mockito.doNothing().when(resourceLimitManager).removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
resourceLimitManager.incrementVolumeResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
- Mockito.verify(resourceLimitManager, Mockito.times(1)).incrementResourceCountWithTag(
- 1L, Resource.ResourceType.volume, tag);
+ Mockito.verify(resourceLimitManager, Mockito.times(1)).removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ accountId, Resource.ResourceType.volume, List.of(tag), 1L);
Mockito.verify(resourceLimitManager, Mockito.times(1))
- .incrementResourceCountWithTag(accountId, Resource.ResourceType.primary_storage, tag, delta);
+ .removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, Resource.ResourceType.primary_storage, List.of(tag), delta);
+ }
+
+ @Test
+ public void testIncrementVolumeResourceCountBatchesAcrossAllTags() {
+ long accountId = 1L;
+ long delta = 32L * 1024 * 1024 * 1024;
+ List tags = Arrays.asList("", "ed1", "ed2");
+ Mockito.doReturn(tags).when(resourceLimitManager)
+ .getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
+ Mockito.doNothing().when(resourceLimitManager).removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
+
+ resourceLimitManager.incrementVolumeResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
+
+ // Exactly two batched calls — one per ResourceType — regardless of how many tags are configured.
+ Mockito.verify(resourceLimitManager, Mockito.times(1))
+ .removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, Resource.ResourceType.volume, tags, 1L);
+ Mockito.verify(resourceLimitManager, Mockito.times(1))
+ .removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, Resource.ResourceType.primary_storage, tags, delta);
+ // Per-tag pathway must not be invoked — this is the regression guard for the lock-contention fix.
+ Mockito.verify(resourceLimitManager, Mockito.never()).incrementResourceCountWithTag(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString());
+ Mockito.verify(resourceLimitManager, Mockito.never()).incrementResourceCountWithTag(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString(), Mockito.anyLong());
}
@Test
@@ -998,19 +1021,18 @@ public void testDecrementVolumeResourceCount() {
Mockito.doReturn(new ArrayList<>()).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
resourceLimitManager.decrementVolumeResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
- Mockito.verify(resourceLimitManager, Mockito.never()).decrementResourceCountWithTag(Mockito.anyLong(),
- Mockito.eq(Resource.ResourceType.volume), Mockito.anyString());
- Mockito.verify(resourceLimitManager, Mockito.never()).decrementResourceCountWithTag(Mockito.anyLong(),
- Mockito.eq(Resource.ResourceType.primary_storage), Mockito.anyString(), Mockito.anyLong());
+ Mockito.verify(resourceLimitManager, Mockito.never()).decrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
Mockito.doReturn(List.of(tag)).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
- mockDecrementResourceCountWithTag();
+ Mockito.doNothing().when(resourceLimitManager).decrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
resourceLimitManager.decrementVolumeResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
- Mockito.verify(resourceLimitManager, Mockito.times(1)).decrementResourceCountWithTag(
- 1L, Resource.ResourceType.volume, tag);
+ Mockito.verify(resourceLimitManager, Mockito.times(1)).decrementResourceCountForTags(
+ accountId, Resource.ResourceType.volume, List.of(tag), 1L);
Mockito.verify(resourceLimitManager, Mockito.times(1))
- .decrementResourceCountWithTag(accountId, Resource.ResourceType.primary_storage, tag, delta);
+ .decrementResourceCountForTags(accountId, Resource.ResourceType.primary_storage, List.of(tag), delta);
}
@Test
@@ -1021,15 +1043,37 @@ public void testIncrementVolumePrimaryStorageResourceCount() {
Mockito.doReturn(new ArrayList<>()).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
resourceLimitManager.incrementVolumePrimaryStorageResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
- Mockito.verify(resourceLimitManager, Mockito.never()).incrementResourceCountWithTag(Mockito.anyLong(),
- Mockito.eq(Resource.ResourceType.primary_storage), Mockito.anyString(), Mockito.anyLong());
+ Mockito.verify(resourceLimitManager, Mockito.never()).removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
Mockito.doReturn(List.of(tag)).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
- mockIncrementResourceCountWithTag();
+ Mockito.doNothing().when(resourceLimitManager).removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
+ resourceLimitManager.incrementVolumePrimaryStorageResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
+ Mockito.verify(resourceLimitManager, Mockito.times(1))
+ .removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, Resource.ResourceType.primary_storage, List.of(tag), delta);
+ }
+
+ @Test
+ public void testIncrementVolumePrimaryStorageResourceCountBatchesAcrossAllTags() {
+ long accountId = 1L;
+ long delta = 16L * 1024 * 1024 * 1024;
+ List tags = Arrays.asList("", "ed1", "ed2");
+ Mockito.doReturn(tags).when(resourceLimitManager)
+ .getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
+ Mockito.doNothing().when(resourceLimitManager).removeResourceReservationIfNeededAndIncrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
+
resourceLimitManager.incrementVolumePrimaryStorageResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
+
+ // Single batched call regardless of tag count, and the per-tag pathway must not run.
Mockito.verify(resourceLimitManager, Mockito.times(1))
- .incrementResourceCountWithTag(accountId, Resource.ResourceType.primary_storage, tag, delta);
+ .removeResourceReservationIfNeededAndIncrementResourceCountForTags(accountId, Resource.ResourceType.primary_storage, tags, delta);
+ Mockito.verify(resourceLimitManager, Mockito.never()).incrementResourceCountWithTag(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString());
+ Mockito.verify(resourceLimitManager, Mockito.never()).incrementResourceCountWithTag(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyString(), Mockito.anyLong());
}
@Test
@@ -1040,15 +1084,16 @@ public void testDecrementVolumePrimaryStorageResourceCount() {
Mockito.doReturn(new ArrayList<>()).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
resourceLimitManager.decrementVolumePrimaryStorageResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
- Mockito.verify(resourceLimitManager, Mockito.never()).decrementResourceCountWithTag(Mockito.anyLong(),
- Mockito.eq(Resource.ResourceType.primary_storage), Mockito.anyString(), Mockito.anyLong());
+ Mockito.verify(resourceLimitManager, Mockito.never()).decrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
Mockito.doReturn(List.of(tag)).when(resourceLimitManager)
.getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class));
- mockDecrementResourceCountWithTag();
+ Mockito.doNothing().when(resourceLimitManager).decrementResourceCountForTags(
+ Mockito.anyLong(), Mockito.any(Resource.ResourceType.class), Mockito.anyList(), Mockito.anyLong());
resourceLimitManager.decrementVolumePrimaryStorageResourceCount(accountId, false, delta, Mockito.mock(DiskOffering.class));
Mockito.verify(resourceLimitManager, Mockito.times(1))
- .decrementResourceCountWithTag(accountId, Resource.ResourceType.primary_storage, tag, delta);
+ .decrementResourceCountForTags(accountId, Resource.ResourceType.primary_storage, List.of(tag), delta);
}
@Test