Skip to content

Commit 183b829

Browse files
DaanHooglandclaude
andcommitted
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 <noreply@anthropic.com>
1 parent b729f08 commit 183b829

4 files changed

Lines changed: 10 additions & 3 deletions

File tree

‎api/src/main/java/com/cloud/event/EventTypes.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ public class EventTypes {
376376
public static final String EVENT_SNAPSHOT_REVERT = "SNAPSHOT.REVERT";
377377
public static final String EVENT_SNAPSHOT_EXTRACT = "SNAPSHOT.EXTRACT";
378378
public static final String EVENT_SNAPSHOT_SKIPPED = "SNAPSHOT.SKIPPED";
379+
public static final String EVENT_SNAPSHOT_RECURRING_FAILURE_LIMIT_REACHED = "SNAPSHOT.RECURRING.FAILURE.LIMIT.REACHED";
379380
public static final String EVENT_SNAPSHOT_POLICY_CREATE = "SNAPSHOTPOLICY.CREATE";
380381
public static final String EVENT_SNAPSHOT_POLICY_UPDATE = "SNAPSHOTPOLICY.UPDATE";
381382
public static final String EVENT_SNAPSHOT_POLICY_DELETE = "SNAPSHOTPOLICY.DELETE";
@@ -1096,6 +1097,7 @@ public class EventTypes {
10961097
// Snapshots
10971098
entityEventDetails.put(EVENT_SNAPSHOT_CREATE, Snapshot.class);
10981099
entityEventDetails.put(EVENT_SNAPSHOT_SKIPPED, Snapshot.class);
1100+
entityEventDetails.put(EVENT_SNAPSHOT_RECURRING_FAILURE_LIMIT_REACHED, Snapshot.class);
10991101
entityEventDetails.put(EVENT_SNAPSHOT_DELETE, Snapshot.class);
11001102
entityEventDetails.put(EVENT_SNAPSHOT_EXTRACT, Snapshot.class);
11011103
entityEventDetails.put(EVENT_SNAPSHOT_ON_PRIMARY, Snapshot.class);

‎engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ public EventDaoImpl() {
5252
LatestEventsByResourceSearch.and("resourceId", LatestEventsByResourceSearch.entity().getResourceId(), Op.EQ);
5353
LatestEventsByResourceSearch.and("resourceType", LatestEventsByResourceSearch.entity().getResourceType(), Op.EQ);
5454
LatestEventsByResourceSearch.and("type", LatestEventsByResourceSearch.entity().getType(), Op.EQ);
55+
LatestEventsByResourceSearch.and("archived", LatestEventsByResourceSearch.entity().getArchived(), Op.EQ);
5556
LatestEventsByResourceSearch.done();
5657

5758
ToArchiveOrDeleteEventSearch = createSearchBuilder();
@@ -138,6 +139,7 @@ public List<EventVO> listLatestEventsByResource(long resourceId, String resource
138139
sc.setParameters("resourceId", resourceId);
139140
sc.setParameters("resourceType", resourceType);
140141
sc.setParameters("type", type);
142+
sc.setParameters("archived", false);
141143
Filter filter = new Filter(EventVO.class, "createDate", false, 0L, (long) limit);
142144
return listBy(sc, filter);
143145
}

‎server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ protected void handleFailedSnapshotDispatch(final SnapshotScheduleVO snapshotToB
384384
lockedSchedule.setScheduledTimestamp(nextRegularRun);
385385
logger.warn("Snapshot schedule [{}] for volume [{}] has failed [{}] consecutive times; it will not be retried until its next regularly scheduled run at [{}].",
386386
snapshotToBeExecuted, volume, totalFailures, nextRegularRun);
387-
ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_WARN, EventTypes.EVENT_SNAPSHOT_CREATE, true,
387+
ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_WARN, EventTypes.EVENT_SNAPSHOT_RECURRING_FAILURE_LIMIT_REACHED, true,
388388
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),
389389
volumeId, ApiCommandResourceType.Volume.toString());
390390
} else {

‎server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.cloud.storage.snapshot;
1818

1919
import com.cloud.event.ActionEventUtils;
20+
import com.cloud.event.EventTypes;
2021
import com.cloud.event.EventVO;
2122
import com.cloud.event.dao.EventDao;
2223
import com.cloud.storage.Snapshot;
@@ -390,9 +391,11 @@ public void handleFailedSnapshotDispatchTestAtMaxGivesUpAndNotifies() {
390391
snapshotSchedulerImplSpy.handleFailedSnapshotDispatch(snapshotScheduleVoMock, volumeVoMock, snapshotScheduleVoMock, null, new Exception("boom"));
391392

392393
actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent(
393-
Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_ERROR), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString()));
394+
Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_ERROR), Mockito.eq(EventTypes.EVENT_SNAPSHOT_CREATE), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString()));
395+
// Must use a distinct event type from EVENT_SNAPSHOT_CREATE, otherwise it becomes the "latest event" scanned
396+
// by countConsecutiveFailedAttempts and silently resets the consecutive-failure count on the next attempt.
394397
actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent(
395-
Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_WARN), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString()));
398+
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()));
396399
}
397400

398401
Mockito.verify(snapshotScheduleDaoMock).update(Mockito.anyLong(), Mockito.eq(snapshotScheduleVoMock));

0 commit comments

Comments
 (0)