Skip to content

Commit 52d87fa

Browse files
committed
Allow a few seconds of slack when checking the resource alert cooldown
1 parent 33b195b commit 52d87fa

2 files changed

Lines changed: 28 additions & 3 deletions

File tree

‎plugins/resource-alerts/src/main/java/org/apache/cloudstack/resourcealert/ResourceAlertManagerImpl.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ public class ResourceAlertManagerImpl extends ManagerBase implements ResourceAle
129129

130130
private ScheduledExecutorService executor;
131131
static final int EMAIL_QUEUE_SIZE = 100;
132+
static final long COOLDOWN_TOLERANCE_MILLIS = 5000L;
132133

133134
// One sender and a bounded queue, so a slow mail server or an alert storm can't pile up threads.
134135
ExecutorService emailExecutor = new ThreadPoolExecutor(1, 1, 0L, TimeUnit.MILLISECONDS,
@@ -525,11 +526,13 @@ private VolumeStats getVolumeStats(long volumeId) {
525526
return locator != null ? statsCollector.getVolumeStats(locator) : null;
526527
}
527528

528-
private boolean canFire(long ruleId, Long resourceId, int resetInterval) {
529+
// Alert times are stored without milliseconds and runs drift a little, so a cooldown equal to
530+
// the check interval would otherwise skip every other check.
531+
boolean canFire(long ruleId, Long resourceId, int resetInterval) {
529532
ResourceAlertVO last = alertDao.findLastFiredForRule(ruleId, resourceId);
530533
if (last == null) return true;
531-
long secondsSinceLast = (System.currentTimeMillis() - last.getAlertTimestamp().getTime()) / 1000;
532-
return secondsSinceLast >= resetInterval;
534+
long millisSinceLast = System.currentTimeMillis() - last.getAlertTimestamp().getTime();
535+
return millisSinceLast >= TimeUnit.SECONDS.toMillis(resetInterval) - COOLDOWN_TOLERANCE_MILLIS;
533536
}
534537

535538
private void fireAlert(ResourceAlertRuleVO rule, Long resourceId, double value) {

‎plugins/resource-alerts/src/test/java/org/apache/cloudstack/resourcealert/ResourceAlertManagerImplTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,4 +1069,26 @@ public void testBrokenRuleDoesNotStopOtherRules() {
10691069

10701070
verify(alertDao, times(1)).persist(any());
10711071
}
1072+
1073+
private ResourceAlertVO firedSecondsAgo(long seconds) {
1074+
ResourceAlertVO last = mock(ResourceAlertVO.class);
1075+
when(last.getAlertTimestamp()).thenReturn(new Date(System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(seconds)));
1076+
return last;
1077+
}
1078+
1079+
@Test
1080+
public void testCooldownEqualToCheckIntervalFiresOnNextCheck() {
1081+
ResourceAlertVO last = firedSecondsAgo(59);
1082+
when(alertDao.findLastFiredForRule(1L, VM_ID)).thenReturn(last);
1083+
1084+
assertTrue(manager.canFire(1L, VM_ID, 60));
1085+
}
1086+
1087+
@Test
1088+
public void testCooldownStillBlocksWellBeforeItEnds() {
1089+
ResourceAlertVO last = firedSecondsAgo(50);
1090+
when(alertDao.findLastFiredForRule(1L, VM_ID)).thenReturn(last);
1091+
1092+
assertFalse(manager.canFire(1L, VM_ID, 60));
1093+
}
10721094
}

0 commit comments

Comments
 (0)