From 50e9ec6a64bab11973d29bc4aa610b8926210bcc Mon Sep 17 00:00:00 2001 From: Ganesh Maharaj Mahalingam <5544378+ganeshmaharaj@users.noreply.github.com> Date: Fri, 5 Jun 2026 19:50:26 -0700 Subject: [PATCH 1/3] Batch resource_count UPDATEs across tags to reduce lock contention (#835) Concurrent restoreVirtualMachine on KVM clusters that have tagged storage limits configured (resource.limit.storage.tags) was failing at scale with MySQL "Lock wait timeout exceeded" (errcode 1205). On the worst-affected clusters the failure rate for reimage-vm reached ~43%, and >98% of failures during storm windows traced to RestoreVMCmdByAdmin bottoming out at ResourceCountDaoImpl.updateCountByDeltaForIds. Root cause: the volume and primary_storage resource-count entry points iterated the configured tag list (the untagged sentinel plus each storage tag) and issued one UPDATE cloud.resource_count per (type, tag) pair. Each UPDATE acquired X-locks on the account row and every parent domain row for that pair, all held until the outer restoreVirtualMachine transaction committed. With multiple sequential UPDATEs per restore and concurrent callers serializing on the shared rows, the in-transaction lock-acquire chain exceeded innodb_lock_wait_timeout (50s default). Fix: add removeResourceReservationIfNeededAndIncrementResourceCountForTags and decrementResourceCountForTags helpers that resolve the union of resource_count row IDs across the full tag list and issue a single batched UPDATE per ResourceType. Migrate all four affected entry points onto these helpers: - incrementVolumeResourceCount - decrementVolumeResourceCount - incrementVolumePrimaryStorageResourceCount - decrementVolumePrimaryStorageResourceCount For an N-tag configuration this collapses 2N sequential UPDATEs to 2 Signed-off-by: Ganesh Maharaj Mahalingam Co-authored-by: Ganesh Maharaj Mahalingam (cherry picked from commit a8bcb52cee166e362e4ec2744a322ae2588cfe5b) --- .../ResourceLimitManagerImpl.java | 173 ++++++++++++++++-- .../ResourceLimitManagerImplTest.java | 93 +++++++--- 2 files changed, 226 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index fad2da89cf28..03a563bd85a7 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -236,6 +236,150 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws CloudR }); } + /** + * Batched increment for one ResourceType across multiple tags. + * + *

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. + * + *

Behavior: + *

+ * + * @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; the empty string sentinel + * 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 tags, final long numToIncrement) { + if (accountId == Account.ACCOUNT_ID_SYSTEM) { + s_logger.trace("Not incrementing resource count for system accounts, returning"); + return; + } + if (CollectionUtils.isEmpty(tags)) { + return; + } + if (numToIncrement <= 0) { + s_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 reservationIds = (List) obj; + Transaction.execute(new TransactionCallbackWithExceptionNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) throws CloudRuntimeException { + reservationDao.removeByIds(reservationIds); + Set rowIds = collectRowIdsForTags(accountId, type, tags, numToIncrement, true); + if (rowIds.isEmpty()) { + s_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; the empty string sentinel + * 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) { + s_logger.trace("Not decrementing resource count for system accounts, returning"); + return; + } + if (CollectionUtils.isEmpty(tags)) { + return; + } + if (numToDecrement <= 0) { + s_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()) { + s_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. + * + *

The empty string 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 ""} 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 (s_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(); + s_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 +1954,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 +1975,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 +2134,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 +2148,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..ea490ca89f5e 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 = List.of("", "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 = List.of("", "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 From 0eaa46ad42977a42a825b86625e52c88efebebd8 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 4 Aug 2026 13:21:45 -0300 Subject: [PATCH 2/3] Address review comments --- .../cloud/resourcelimit/ResourceLimitManagerImpl.java | 11 +++++------ .../resourcelimit/ResourceLimitManagerImplTest.java | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 03a563bd85a7..82c56fa0f87a 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -262,8 +262,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws CloudR * * @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; the empty string sentinel - * denotes the untagged row, non-empty entries denote tagged rows + * @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") @@ -315,8 +315,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws CloudR * * @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; the empty string sentinel - * denotes the untagged row + * @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, @@ -352,13 +351,13 @@ protected void decrementResourceCountForTags(final long accountId, final Resourc * 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. * - *

The empty string in {@code tags} is a sentinel for the untagged row. Each tag is resolved + *

{@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 ""} sentinel for untagged, plus any tagged entries) + * @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 diff --git a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java index ea490ca89f5e..e1fe3dddf982 100644 --- a/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java +++ b/server/src/test/java/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java @@ -993,7 +993,7 @@ public void testIncrementVolumeResourceCount() { public void testIncrementVolumeResourceCountBatchesAcrossAllTags() { long accountId = 1L; long delta = 32L * 1024 * 1024 * 1024; - List tags = List.of("", "ed1", "ed2"); + List tags = Arrays.asList("", "ed1", "ed2"); Mockito.doReturn(tags).when(resourceLimitManager) .getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class)); Mockito.doNothing().when(resourceLimitManager).removeResourceReservationIfNeededAndIncrementResourceCountForTags( @@ -1059,7 +1059,7 @@ public void testIncrementVolumePrimaryStorageResourceCount() { public void testIncrementVolumePrimaryStorageResourceCountBatchesAcrossAllTags() { long accountId = 1L; long delta = 16L * 1024 * 1024 * 1024; - List tags = List.of("", "ed1", "ed2"); + List tags = Arrays.asList("", "ed1", "ed2"); Mockito.doReturn(tags).when(resourceLimitManager) .getResourceLimitStorageTagsForResourceCountOperation(Mockito.anyBoolean(), Mockito.any(DiskOffering.class)); Mockito.doNothing().when(resourceLimitManager).removeResourceReservationIfNeededAndIncrementResourceCountForTags( From eb97e4b9e0142ca4cc94849b679a1a741051f286 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 18 Aug 2026 21:53:36 -0300 Subject: [PATCH 3/3] Fix build --- .../resourcelimit/ResourceLimitManagerImpl.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 82c56fa0f87a..1863b5a36983 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -270,14 +270,14 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws CloudR protected void removeResourceReservationIfNeededAndIncrementResourceCountForTags( final long accountId, final ResourceType type, final List tags, final long numToIncrement) { if (accountId == Account.ACCOUNT_ID_SYSTEM) { - s_logger.trace("Not incrementing resource count for system accounts, returning"); + logger.trace("Not incrementing resource count for system accounts, returning"); return; } if (CollectionUtils.isEmpty(tags)) { return; } if (numToIncrement <= 0) { - s_logger.warn(String.format("Skipping increment of resource count: non-positive delta = %d for Account = %d Type = %s", + logger.warn(String.format("Skipping increment of resource count: non-positive delta = %d for Account = %d Type = %s", numToIncrement, accountId, type)); return; } @@ -289,7 +289,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws CloudR reservationDao.removeByIds(reservationIds); Set rowIds = collectRowIdsForTags(accountId, type, tags, numToIncrement, true); if (rowIds.isEmpty()) { - s_logger.warn("No resource_count rows resolved to increment for Account = " + accountId + logger.warn("No resource_count rows resolved to increment for Account = " + accountId + " Type = " + type + " tags = " + tags + "; skipping update"); return; } @@ -321,20 +321,20 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws CloudR protected void decrementResourceCountForTags(final long accountId, final ResourceType type, final List tags, final long numToDecrement) { if (accountId == Account.ACCOUNT_ID_SYSTEM) { - s_logger.trace("Not decrementing resource count for system accounts, returning"); + logger.trace("Not decrementing resource count for system accounts, returning"); return; } if (CollectionUtils.isEmpty(tags)) { return; } if (numToDecrement <= 0) { - s_logger.warn(String.format("Skipping decrement of resource count: non-positive delta = %d for Account = %d Type = %s", + 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()) { - s_logger.warn("No resource_count rows resolved to decrement for Account = " + accountId + logger.warn("No resource_count rows resolved to decrement for Account = " + accountId + " Type = " + type + " tags = " + tags + "; skipping update"); return; } @@ -366,12 +366,12 @@ protected void decrementResourceCountForTags(final long accountId, final Resourc private Set collectRowIdsForTags(long accountId, ResourceType type, List tags, long delta, boolean increment) { Set rowIds = new HashSet<>(); for (String tag : tags) { - if (s_logger.isDebugEnabled()) { + 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(); - s_logger.debug("Updating resource Type = " + typeStr + " count for Account = " + accountId + 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));