From 1cb950b5a40b5162d8546a5dce15ec5bcc464e97 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 30 Jul 2026 08:46:33 +0200 Subject: [PATCH] Fix consecutive-failure counting bugs flagged in PR #13743 review - Use a distinct event type (SNAPSHOT.RECURRING.FAILURE.LIMIT.REACHED) for the "gave up" WARN notification instead of EVENT_SNAPSHOT_CREATE, since that event became the latest EVENT_SNAPSHOT_CREATE entry and silently reset countConsecutiveFailedAttempts on the next scan. - Filter out archived events in EventDao#listLatestEventsByResource so stale archived events can't be counted as the latest failure history, matching the archived=false filtering already used by sibling searches in EventDaoImpl. Co-Authored-By: Claude Sonnet 5 --- api/src/main/java/com/cloud/event/EventTypes.java | 2 ++ .../src/main/java/com/cloud/event/dao/EventDaoImpl.java | 2 ++ .../com/cloud/storage/snapshot/SnapshotSchedulerImpl.java | 2 +- .../cloud/storage/snapshot/SnapshotSchedulerImplTest.java | 7 +++++-- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/com/cloud/event/EventTypes.java b/api/src/main/java/com/cloud/event/EventTypes.java index 5323f71c52b..acc14e7df35 100644 --- a/api/src/main/java/com/cloud/event/EventTypes.java +++ b/api/src/main/java/com/cloud/event/EventTypes.java @@ -371,6 +371,7 @@ public class EventTypes { public static final String EVENT_SNAPSHOT_REVERT = "SNAPSHOT.REVERT"; public static final String EVENT_SNAPSHOT_EXTRACT = "SNAPSHOT.EXTRACT"; public static final String EVENT_SNAPSHOT_SKIPPED = "SNAPSHOT.SKIPPED"; + public static final String EVENT_SNAPSHOT_RECURRING_FAILURE_LIMIT_REACHED = "SNAPSHOT.RECURRING.FAILURE.LIMIT.REACHED"; public static final String EVENT_SNAPSHOT_POLICY_CREATE = "SNAPSHOTPOLICY.CREATE"; public static final String EVENT_SNAPSHOT_POLICY_UPDATE = "SNAPSHOTPOLICY.UPDATE"; public static final String EVENT_SNAPSHOT_POLICY_DELETE = "SNAPSHOTPOLICY.DELETE"; @@ -1076,6 +1077,7 @@ public class EventTypes { // Snapshots entityEventDetails.put(EVENT_SNAPSHOT_CREATE, Snapshot.class); entityEventDetails.put(EVENT_SNAPSHOT_SKIPPED, Snapshot.class); + entityEventDetails.put(EVENT_SNAPSHOT_RECURRING_FAILURE_LIMIT_REACHED, Snapshot.class); entityEventDetails.put(EVENT_SNAPSHOT_DELETE, Snapshot.class); entityEventDetails.put(EVENT_SNAPSHOT_EXTRACT, Snapshot.class); entityEventDetails.put(EVENT_SNAPSHOT_ON_PRIMARY, Snapshot.class); 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 da03ca1482d..bbb815251c7 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 @@ -51,6 +51,7 @@ public class EventDaoImpl extends GenericDaoBase implements Event LatestEventsByResourceSearch.and("resourceId", LatestEventsByResourceSearch.entity().getResourceId(), Op.EQ); LatestEventsByResourceSearch.and("resourceType", LatestEventsByResourceSearch.entity().getResourceType(), Op.EQ); LatestEventsByResourceSearch.and("type", LatestEventsByResourceSearch.entity().getType(), Op.EQ); + LatestEventsByResourceSearch.and("archived", LatestEventsByResourceSearch.entity().getArchived(), Op.EQ); LatestEventsByResourceSearch.done(); ToArchiveOrDeleteEventSearch = createSearchBuilder(); @@ -118,6 +119,7 @@ public class EventDaoImpl extends GenericDaoBase implements Event sc.setParameters("resourceId", resourceId); sc.setParameters("resourceType", resourceType); sc.setParameters("type", type); + sc.setParameters("archived", false); Filter filter = new Filter(EventVO.class, "createDate", false, 0L, (long) limit); return listBy(sc, filter); } diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java index 7fcfe76f1c7..afe29b87efe 100644 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java @@ -384,7 +384,7 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu lockedSchedule.setScheduledTimestamp(nextRegularRun); logger.warn("Snapshot schedule [{}] for volume [{}] has failed [{}] consecutive times; it will not be retried until its next regularly scheduled run at [{}].", snapshotToBeExecuted, volume, totalFailures, nextRegularRun); - ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_WARN, EventTypes.EVENT_SNAPSHOT_CREATE, true, + ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_WARN, EventTypes.EVENT_SNAPSHOT_RECURRING_FAILURE_LIMIT_REACHED, true, String.format("Recurring snapshot for volume [%s] has failed %d consecutive times and will not be retried until its next regularly scheduled run.", volume, totalFailures), volumeId, ApiCommandResourceType.Volume.toString()); } else { diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java index 4177a23553e..6d8222842ac 100644 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java @@ -17,6 +17,7 @@ package com.cloud.storage.snapshot; import com.cloud.event.ActionEventUtils; +import com.cloud.event.EventTypes; import com.cloud.event.EventVO; import com.cloud.event.dao.EventDao; import com.cloud.storage.Snapshot; @@ -390,9 +391,11 @@ public class SnapshotSchedulerImplTest { snapshotSchedulerImplSpy.handleFailedSnapshotDispatch(snapshotScheduleVoMock, volumeVoMock, snapshotScheduleVoMock, null, new Exception("boom")); actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent( - Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_ERROR), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString())); + Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_ERROR), Mockito.eq(EventTypes.EVENT_SNAPSHOT_CREATE), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString())); + // Must use a distinct event type from EVENT_SNAPSHOT_CREATE, otherwise it becomes the "latest event" scanned + // by countConsecutiveFailedAttempts and silently resets the consecutive-failure count on the next attempt. actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent( - Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_WARN), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString())); + Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_WARN), Mockito.eq(EventTypes.EVENT_SNAPSHOT_RECURRING_FAILURE_LIMIT_REACHED), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString())); } Mockito.verify(snapshotScheduleDaoMock).update(Mockito.anyLong(), Mockito.eq(snapshotScheduleVoMock));