From 86e5993bd5fb46f849576a6d9af58fd102c9acc8 Mon Sep 17 00:00:00 2001 From: Aaron Chung Date: Mon, 4 May 2026 22:18:26 -0700 Subject: [PATCH 1/5] Optimize HostDaoImpl.resetHosts to use bulk UPDATE instead of row-level locks Replace lockRows + per-row update loop with a single bulk UPDATE using the existing createForUpdate/UpdateBuilder/update(ub, sc, null) pattern (same as markHostsAsDisconnected in the same file). Eliminates N SELECT FOR UPDATE + N individual UPDATE round-trips during MS startup/reconnect. The lock was safe to remove because resetHosts only modifies rows where management_server_id = ourMS (no other MS targets these rows), and setting to NULL is idempotent with no read-dependent computation. A non-locking SELECT is issued before the bulk UPDATE when TRACE logging is enabled to preserve per-host-ID logging from the original code. --- .../java/com/cloud/host/dao/HostDaoImpl.java | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java index 99c9a979c3bf..f32d1cb8e21a 100644 --- a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java @@ -644,16 +644,22 @@ private void resetHosts(long managementServerId, long lastPingSecondsAfter) { sc.setParameters("lastPinged", lastPingSecondsAfter); sc.setParameters("status", Status.Disconnected, Status.Down, Status.Alert); - StringBuilder sb = new StringBuilder(); - List hosts = lockRows(sc, null, true); // exclusive lock - for (HostVO host : hosts) { - host.setManagementServerId(null); - update(host.getId(), host); - sb.append(host.getId()); - sb.append(" "); + // SELECT before bulk UPDATE to preserve per-host-ID trace logging — the bulk UPDATE + // cannot return which rows it matched since the WHERE column is being set to NULL + if (logger.isTraceEnabled()) { + List hosts = listBy(sc); + StringBuilder sb = new StringBuilder(); + for (HostVO host : hosts) { + sb.append(host.getId()); + sb.append(" "); + } + logger.trace("Following hosts will be reset: {}", sb); } - logger.trace("Following hosts got reset: {}", sb); + HostVO host = createForUpdate(); + host.setManagementServerId(null); + UpdateBuilder ub = getUpdateBuilder(host); + update(ub, sc, null); } /* From af6ec4ed42ac671c30f31b32378d3b1921f212c6 Mon Sep 17 00:00:00 2001 From: Aaron Chung Date: Mon, 4 May 2026 22:23:21 -0700 Subject: [PATCH 2/5] Optimize alert and event archival to use bulk UPDATE instead of row-level locks Replace per-row lockRow + update + commit loop with a single bulk UPDATE using createForUpdate/UpdateBuilder/update(ub, sc, null) pattern. AlertDaoImpl.archiveAlert: reuses the existing SearchCriteria to issue one UPDATE alert SET archived=1 WHERE ... instead of N individual lock-update-commit cycles. EventDaoImpl.archiveEvents: builds an ID-based SearchCriteria from the pre-fetched event list and issues a single bulk UPDATE. Also fixes a latent NPE in both methods where lockRow returning null (row deleted concurrently) would cause the next line to throw. --- .../com/cloud/alert/dao/AlertDaoImpl.java | 17 +++++-------- .../com/cloud/event/dao/EventDaoImpl.java | 24 +++++++++++-------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java b/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java index 94d01f472ba5..a7dd51bcecc1 100644 --- a/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java @@ -28,7 +28,7 @@ import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.SearchCriteria.Op; -import com.cloud.utils.db.TransactionLegacy; +import com.cloud.utils.db.UpdateBuilder; @Component public class AlertDaoImpl extends GenericDaoBase implements AlertDao { @@ -108,22 +108,17 @@ public boolean archiveAlert(List ids, String type, Date startDate, Date en sc.setParameters("archived", false); boolean result = true; - ; + List alerts = listBy(sc); if (ids != null && alerts.size() < ids.size()) { result = false; return result; } if (alerts != null && !alerts.isEmpty()) { - TransactionLegacy txn = TransactionLegacy.currentTxn(); - txn.start(); - for (AlertVO alert : alerts) { - alert = lockRow(alert.getId(), true); - alert.setArchived(true); - update(alert.getId(), alert); - txn.commit(); - } - txn.close(); + AlertVO alertForUpdate = createForUpdate(); + alertForUpdate.setArchived(true); + UpdateBuilder ub = getUpdateBuilder(alertForUpdate); + update(ub, sc, null); } return result; } diff --git a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java index e748e98900eb..81b7fae415b5 100644 --- a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java @@ -18,6 +18,7 @@ import java.util.Date; import java.util.List; +import java.util.stream.Collectors; import org.springframework.stereotype.Component; @@ -29,12 +30,13 @@ import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.db.SearchCriteria.Op; -import com.cloud.utils.db.TransactionLegacy; +import com.cloud.utils.db.UpdateBuilder; @Component public class EventDaoImpl extends GenericDaoBase implements EventDao { protected final SearchBuilder CompletedEventSearch; protected final SearchBuilder ToArchiveOrDeleteEventSearch; + protected final SearchBuilder ArchiveByIdsSearch; public EventDaoImpl() { CompletedEventSearch = createSearchBuilder(); @@ -51,6 +53,10 @@ public EventDaoImpl() { ToArchiveOrDeleteEventSearch.and("createdDateL", ToArchiveOrDeleteEventSearch.entity().getCreateDate(), Op.LTEQ); ToArchiveOrDeleteEventSearch.and("archived", ToArchiveOrDeleteEventSearch.entity().getArchived(), Op.EQ); ToArchiveOrDeleteEventSearch.done(); + + ArchiveByIdsSearch = createSearchBuilder(); + ArchiveByIdsSearch.and("id", ArchiveByIdsSearch.entity().getId(), Op.IN); + ArchiveByIdsSearch.done(); } @Override @@ -101,15 +107,13 @@ public List listToArchiveOrDeleteEvents(List ids, String type, Da @Override public void archiveEvents(List events) { if (events != null && !events.isEmpty()) { - TransactionLegacy txn = TransactionLegacy.currentTxn(); - txn.start(); - for (EventVO event : events) { - event = lockRow(event.getId(), true); - event.setArchived(true); - update(event.getId(), event); - txn.commit(); - } - txn.close(); + List ids = events.stream().map(EventVO::getId).collect(Collectors.toList()); + SearchCriteria sc = ArchiveByIdsSearch.create(); + sc.setParameters("id", ids.toArray(new Object[ids.size()])); + EventVO eventForUpdate = createForUpdate(); + eventForUpdate.setArchived(true); + UpdateBuilder ub = getUpdateBuilder(eventForUpdate); + update(ub, sc, null); } } } From 0d1211a2d1870ee49edee172e627c966b9b2e9b4 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 5 Jun 2026 00:57:06 +0530 Subject: [PATCH 3/5] code improvements --- .../com/cloud/alert/dao/AlertDaoImpl.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java b/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java index a7dd51bcecc1..97b7c54f0844 100644 --- a/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/alert/dao/AlertDaoImpl.java @@ -20,6 +20,7 @@ import java.util.List; +import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; import com.cloud.alert.AlertVO; @@ -107,20 +108,20 @@ public boolean archiveAlert(List ids, String type, Date startDate, Date en } sc.setParameters("archived", false); - boolean result = true; - List alerts = listBy(sc); if (ids != null && alerts.size() < ids.size()) { - result = false; - return result; + return false; } - if (alerts != null && !alerts.isEmpty()) { - AlertVO alertForUpdate = createForUpdate(); - alertForUpdate.setArchived(true); - UpdateBuilder ub = getUpdateBuilder(alertForUpdate); - update(ub, sc, null); + + if (CollectionUtils.isEmpty(alerts)) { + return true; } - return result; + + AlertVO alertForUpdate = createForUpdate(); + alertForUpdate.setArchived(true); + UpdateBuilder ub = getUpdateBuilder(alertForUpdate); + update(ub, sc, null); + return true; } @Override From 1713907b4798afeb23c51d1017872ebe3345d974 Mon Sep 17 00:00:00 2001 From: Aaron Chung Date: Mon, 4 May 2026 22:25:23 -0700 Subject: [PATCH 4/5] Optimize SecurityGroupWorkDaoImpl.updateStep to use direct UPDATE instead of row-level locks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace lockRow/lockRows + update pattern in both updateStep overloads with createForUpdate + update(entity, sc) — the same pattern already used in findAndCleanupUnfinishedWork in the same file. updateStep(vmId, seqNum, step): replaces lockRows(LIMIT 1) + update with a single UPDATE ... SET step=? WHERE instance_id=? AND seq_no=?. updateStep(workId, step): replaces lockRow + null check + update with a single UPDATE ... SET step=? WHERE id=?. Non-existent row results in 0 rows affected — same no-op as the original null check. The locks were redundant because the work queue model guarantees single-owner access: take() assigns a serverId to each work item, and only the owning server updates its step. --- .../dao/SecurityGroupWorkDaoImpl.java | 35 ++++--------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java index 327d12c759a7..41f47d1b05da 100644 --- a/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java @@ -141,26 +141,14 @@ public SecurityGroupWorkVO take(long serverId) { } @Override - @DB public void updateStep(Long vmId, Long logSequenceNumber, Step step) { - final TransactionLegacy txn = TransactionLegacy.currentTxn(); - txn.start(); SearchCriteria sc = VmIdSeqNumSearch.create(); sc.setParameters("vmId", vmId); sc.setParameters("seqno", logSequenceNumber); - final Filter filter = new Filter(SecurityGroupWorkVO.class, null, true, 0l, 1l); - - final List vos = lockRows(sc, filter, true); - if (vos.size() == 0) { - txn.commit(); - return; - } - SecurityGroupWorkVO work = vos.get(0); - work.setStep(step); - update(work.getId(), work); - - txn.commit(); + SecurityGroupWorkVO workForUpdate = createForUpdate(); + workForUpdate.setStep(step); + update(workForUpdate, sc); } @Override @@ -172,21 +160,10 @@ public SecurityGroupWorkVO findByVmIdStep(long vmId, Step step) { } @Override - @DB public void updateStep(Long workId, Step step) { - final TransactionLegacy txn = TransactionLegacy.currentTxn(); - txn.start(); - - SecurityGroupWorkVO work = lockRow(workId, true); - if (work == null) { - txn.commit(); - return; - } - work.setStep(step); - update(work.getId(), work); - - txn.commit(); - + SecurityGroupWorkVO workForUpdate = createForUpdate(); + workForUpdate.setStep(step); + update(workId, workForUpdate); } @Override From e654122a8416ccda3ebb125200e0fa6c32434b44 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 5 Jun 2026 01:02:14 +0530 Subject: [PATCH 5/5] changes/improvements --- .../com/cloud/event/dao/EventDaoImpl.java | 19 +++++++++++-------- .../dao/SecurityGroupWorkDaoImpl.java | 19 +++++++++++++++---- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java index 81b7fae415b5..9417ddd12595 100644 --- a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java @@ -21,6 +21,7 @@ import java.util.stream.Collectors; +import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; import com.cloud.event.Event.State; @@ -106,14 +107,16 @@ public List listToArchiveOrDeleteEvents(List ids, String type, Da @Override public void archiveEvents(List events) { - if (events != null && !events.isEmpty()) { - List ids = events.stream().map(EventVO::getId).collect(Collectors.toList()); - SearchCriteria sc = ArchiveByIdsSearch.create(); - sc.setParameters("id", ids.toArray(new Object[ids.size()])); - EventVO eventForUpdate = createForUpdate(); - eventForUpdate.setArchived(true); - UpdateBuilder ub = getUpdateBuilder(eventForUpdate); - update(ub, sc, null); + if (CollectionUtils.isEmpty(events)) { + return; } + + List ids = events.stream().map(EventVO::getId).collect(Collectors.toList()); + SearchCriteria sc = ArchiveByIdsSearch.create(); + sc.setParameters("id", ids.toArray(new Object[ids.size()])); + EventVO eventForUpdate = createForUpdate(); + eventForUpdate.setArchived(true); + UpdateBuilder ub = getUpdateBuilder(eventForUpdate); + update(ub, sc, null); } } diff --git a/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java index 41f47d1b05da..6c813ce88cc6 100644 --- a/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/security/dao/SecurityGroupWorkDaoImpl.java @@ -116,7 +116,7 @@ public SecurityGroupWorkVO take(long serverId) { //ensure that there is no job in Processing state for the same VM processing = true; if (logger.isTraceEnabled()) { - logger.trace("Security Group work take: found a job in Scheduled and Processing vmid=" + work.getInstanceId()); + logger.trace("Security Group work take: found a job in Scheduled and Processing vmid={}", work.getInstanceId()); } } work.setServerId(serverId); @@ -142,13 +142,24 @@ public SecurityGroupWorkVO take(long serverId) { @Override public void updateStep(Long vmId, Long logSequenceNumber, Step step) { + final TransactionLegacy txn = TransactionLegacy.currentTxn(); + txn.start(); SearchCriteria sc = VmIdSeqNumSearch.create(); sc.setParameters("vmId", vmId); sc.setParameters("seqno", logSequenceNumber); - SecurityGroupWorkVO workForUpdate = createForUpdate(); - workForUpdate.setStep(step); - update(workForUpdate, sc); + final Filter filter = new Filter(SecurityGroupWorkVO.class, null, true, 0l, 1l); + + final List vos = lockRows(sc, filter, true); + if (vos.size() == 0) { + txn.commit(); + return; + } + SecurityGroupWorkVO work = vos.get(0); + work.setStep(step); + update(work.getId(), work); + + txn.commit(); } @Override