Merge pull request #1866 from HubSpot/executor_cleanup

Clean up logrotate files sooner
HubSpot · Oct 22, 2018 · f53906d · f53906d
2 parents 11a533e + b7f24d0
commit f53906d
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 10 deletions.
diff --git a/...r/src/main/java/com/hubspot/singularity/executor/task/SingularityExecutorTaskCleanup.java b/...r/src/main/java/com/hubspot/singularity/executor/task/SingularityExecutorTaskCleanup.java
@@ -82,6 +82,10 @@ public TaskCleanupResult cleanup(boolean cleanupTaskAppDirectory, boolean isDock
     }
   }
 
+  public void cleanUpLogs() {
+    taskLogManager.teardown();
+  }
+
   private TaskCleanupResult finishTaskCleanup(boolean dockerCleanSuccess) {
     boolean cleanTaskDefinitionFile = cleanTaskDefinitionFile();
 

diff --git a/...up/src/main/java/com/hubspot/singularity/executor/cleanup/SingularityExecutorCleanup.java b/...up/src/main/java/com/hubspot/singularity/executor/cleanup/SingularityExecutorCleanup.java
@@ -13,6 +13,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -270,24 +271,30 @@ private TaskCleanupResult cleanTask(SingularityExecutorTaskDefinition taskDefini
 
     boolean cleanupTaskAppDirectory = !taskDefinition.getExecutorData().getPreserveTaskSandboxAfterFinish().or(Boolean.FALSE);
 
+    if (taskDefinition.shouldLogrotateLogFile()) {
+      checkForUncompressedLogrotatedFile(taskDefinition);
+    }
+
     if (taskHistory.isPresent()) {
       final Optional<SingularityTaskHistoryUpdate> lastUpdate = JavaUtils.getLast(taskHistory.get().getTaskUpdates());
 
-      if (lastUpdate.isPresent() && lastUpdate.get().getTaskState().isFailed()) {
-        final long delta = System.currentTimeMillis() - lastUpdate.get().getTimestamp();
+      if (lastUpdate.isPresent()) {
+        if (lastUpdate.get().getTaskState().isDone() && System.currentTimeMillis() - lastUpdate.get().getTimestamp() > TimeUnit.MINUTES.toMillis(15)) {
+          LOG.info("Task {} is done for > 15 minutes, removing logrotate files");
+          taskCleanup.cleanUpLogs();
+        }
+        if (lastUpdate.get().getTaskState().isFailed()) {
+          final long delta = System.currentTimeMillis() - lastUpdate.get().getTimestamp();
 
-        if (delta < cleanupConfiguration.getCleanupAppDirectoryOfFailedTasksAfterMillis()) {
-          LOG.info("Not cleaning up task app directory of {} because only {} has elapsed since it failed (will cleanup after {})", taskDefinition.getTaskId(),
-              JavaUtils.durationFromMillis(delta), JavaUtils.durationFromMillis(cleanupConfiguration.getCleanupAppDirectoryOfFailedTasksAfterMillis()));
-          cleanupTaskAppDirectory = false;
+          if (delta < cleanupConfiguration.getCleanupAppDirectoryOfFailedTasksAfterMillis()) {
+            LOG.info("Not cleaning up task app directory of {} because only {} has elapsed since it failed (will cleanup after {})", taskDefinition.getTaskId(),
+                JavaUtils.durationFromMillis(delta), JavaUtils.durationFromMillis(cleanupConfiguration.getCleanupAppDirectoryOfFailedTasksAfterMillis()));
+            cleanupTaskAppDirectory = false;
+          }
         }
       }
     }
 
-    if (taskDefinition.shouldLogrotateLogFile()) {
-      checkForUncompressedLogrotatedFile(taskDefinition);
-    }
-
     boolean isDocker = (taskHistory.isPresent()
         && taskHistory.get().getTask().getTaskRequest().getDeploy().getContainerInfo().isPresent()
         && taskHistory.get().getTask().getTaskRequest().getDeploy().getContainerInfo().get().getType() == SingularityContainerType.DOCKER);