Merge pull request #1973 from HubSpot/cooldown_tweaks_2

tweak cooldown thresholds and evaluation logic
HubSpot · Jul 11, 2019 · 25115be · 25115be
2 parents 4c0be03 + 96e42ec
commit 25115be
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 9 deletions.
diff --git a/...ularityService/src/main/java/com/hubspot/singularity/config/SingularityConfiguration.java b/...ularityService/src/main/java/com/hubspot/singularity/config/SingularityConfiguration.java
@@ -120,15 +120,15 @@ public class SingularityConfiguration extends Configuration {
 
   private int fastFailureCooldownCount = 3;
 
-  private long fastFailureCooldownMs = 60000;
+  private long fastFailureCooldownMs = 30000;
 
   private long fastCooldownExpiresMinutesWithoutFailure = 5;
 
   private int slowFailureCooldownCount = 5;
 
   private long slowFailureCooldownMs = 600000;
 
-  private long slowCooldownExpiresMinutesWithoutFailure = 8;
+  private long slowCooldownExpiresMinutesWithoutFailure = 5;
 
   private long cooldownMinScheduleSeconds = 120;
 

diff --git a/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityCooldown.java b/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityCooldown.java
@@ -67,8 +67,9 @@ private boolean hasFailureLoop(SingularityDeployStatistics deployStatistics, Opt
         .count();
     java.util.Optional<Long> mostRecentFailure = failureTimestamps.stream().max(Comparator.comparingLong(Long::valueOf));
 
-    return failureCount >= cooldownCount
-        && (!mostRecentFailure.isPresent() || mostRecentFailure.get() > System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(expiresAfterMins));
+    boolean mostRecentFailureOutsideWindow = !mostRecentFailure.isPresent() || mostRecentFailure.get() < System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(expiresAfterMins);
+
+    return failureCount >= cooldownCount && !mostRecentFailureOutsideWindow;
   }
 
   boolean hasCooldownExpired(SingularityDeployStatistics deployStatistics, Optional<Long> recentFailureTimestamp) {

diff --git a/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityScheduler.java b/SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityScheduler.java
@@ -591,7 +591,10 @@ private Optional<PendingType> handleCompletedTaskWithStatistics(Optional<Singula
     }
 
     if (!status.hasReason() || !status.getReason().equals(Reason.REASON_INVALID_OFFERS)) {
-      if (!state.isSuccess() && taskHistoryUpdateCreateResult == SingularityCreateResult.CREATED && cooldown.shouldEnterCooldown(request, requestState, deployStatistics, timestamp)) {
+      if (state != ExtendedTaskState.TASK_KILLED
+          && !state.isSuccess()
+          && taskHistoryUpdateCreateResult == SingularityCreateResult.CREATED
+          && cooldown.shouldEnterCooldown(request, requestState, deployStatistics, timestamp)) {
         LOG.info("Request {} is entering cooldown due to task {}", request.getId(), taskId);
         requestState = RequestState.SYSTEM_COOLDOWN;
         requestManager.cooldown(request, System.currentTimeMillis());
@@ -715,10 +718,6 @@ private void updateDeployStatistics(SingularityDeployStatistics deployStatistics
       } else {
         bldr.setAverageSchedulingDelayMillis(Optional.of(startedAt - dueTime));
       }
-
-      final SingularityDeployStatistics newStatistics = bldr.build();
-
-      deployManager.saveDeployStatistics(newStatistics);
     }
 
     bldr.setNumTasks(bldr.getNumTasks() + 1);