New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deleting state #1386
Deleting state #1386
Changes from 7 commits
91de88b
cac4dba
53b397f
d2da062
fa02949
60c1892
ffb5e8c
3715937
b98095e
8f39f9c
4be8f90
53162de
4797793
394a1e5
c818b74
a9a2dde
07ff1ca
05ee09e
b7f4f2b
590c5c8
b6fff6d
de9a21d
152d7a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,10 @@ | |
|
||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.HashMap; | ||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
|
@@ -23,6 +25,7 @@ | |
import com.hubspot.mesos.JavaUtils; | ||
import com.hubspot.singularity.LoadBalancerRequestType; | ||
import com.hubspot.singularity.LoadBalancerRequestType.LoadBalancerRequestId; | ||
import com.hubspot.singularity.RequestCleanupType; | ||
import com.hubspot.singularity.RequestState; | ||
import com.hubspot.singularity.SingularityDeleteResult; | ||
import com.hubspot.singularity.SingularityDeploy; | ||
|
@@ -37,8 +40,10 @@ | |
import com.hubspot.singularity.SingularityRequestCleanup; | ||
import com.hubspot.singularity.SingularityRequestDeployState; | ||
import com.hubspot.singularity.SingularityRequestHistory; | ||
import com.hubspot.singularity.SingularityRequestHistory.RequestHistoryType; | ||
import com.hubspot.singularity.SingularityRequestLbCleanup; | ||
import com.hubspot.singularity.SingularityRequestWithState; | ||
import com.hubspot.singularity.SingularityShellCommand; | ||
import com.hubspot.singularity.SingularityTask; | ||
import com.hubspot.singularity.SingularityTaskCleanup; | ||
import com.hubspot.singularity.SingularityTaskId; | ||
|
@@ -123,8 +128,7 @@ private boolean shouldKillTask(SingularityTaskCleanup taskCleanup, List<Singular | |
|
||
// If pausing, must be a long-running task to kill here | ||
if (requestWithState.get().getState() == RequestState.PAUSED && | ||
((taskCleanup.getCleanupType() == TaskCleanupType.PAUSING && request.isLongRunning()) | ||
|| !(taskCleanup.getCleanupType() == TaskCleanupType.PAUSING))) { | ||
(!(taskCleanup.getCleanupType() == TaskCleanupType.PAUSING) || request.isLongRunning())) { | ||
LOG.debug("Killing a task {} immediately because the request was paused", taskCleanup); | ||
return true; | ||
} | ||
|
@@ -336,7 +340,7 @@ private void drainRequestCleanupQueue() { | |
continue; | ||
} | ||
} else { | ||
if (pause(requestCleanup, matchingActiveTaskIds, killActiveTasks) == TaskCleanupType.PAUSING) { | ||
if (pause(requestCleanup, matchingActiveTaskIds) == TaskCleanupType.PAUSING) { | ||
killActiveTasks = false; | ||
} | ||
} | ||
|
@@ -350,11 +354,16 @@ private void drainRequestCleanupQueue() { | |
createLbCleanupRequest(requestId, matchingActiveTaskIds); | ||
} | ||
} else { | ||
Optional<SingularityRequestHistory> maybeHistory = requestHistoryHelper.getLastHistory(requestId); | ||
if (maybeHistory.isPresent() && maybeHistory.get().getRequest().isLoadBalanced() && configuration.isDeleteRemovedRequestsFromLoadBalancer()) { | ||
createLbCleanupRequest(requestId, matchingActiveTaskIds); | ||
if (!Iterables.isEmpty(matchingActiveTaskIds)) { | ||
delete(requestCleanup, matchingActiveTaskIds); | ||
} else { | ||
Optional<SingularityRequestHistory> maybeHistory = requestHistoryHelper.getLastHistory(requestId); | ||
if (maybeHistory.isPresent() && maybeHistory.get().getRequest().isLoadBalanced() && configuration.isDeleteRemovedRequestsFromLoadBalancer()) { | ||
createLbCleanupRequest(requestId, matchingActiveTaskIds); | ||
requestManager.markDeleted(maybeHistory.get().getRequest(), RequestHistoryType.DELETED, System.currentTimeMillis(), Optional.<String>absent(), Optional.<String>absent()); | ||
} | ||
cleanupDeployState(requestCleanup); | ||
} | ||
cleanupDeployState(requestCleanup); | ||
} | ||
break; | ||
case BOUNCE: | ||
|
@@ -432,7 +441,7 @@ private void bounce(SingularityRequestCleanup requestCleanup, final List<Singula | |
LOG.info("Added {} tasks for request {} to cleanup bounce queue in {}", matchingTaskIds.size(), requestCleanup.getRequestId(), JavaUtils.duration(start)); | ||
} | ||
|
||
private TaskCleanupType pause(SingularityRequestCleanup requestCleanup, Iterable<SingularityTaskId> activeTaskIds, boolean killActiveTasks) { | ||
private TaskCleanupType pause(SingularityRequestCleanup requestCleanup, Iterable<SingularityTaskId> activeTaskIds) { | ||
final long start = System.currentTimeMillis(); | ||
boolean killTasks = requestCleanup.getKillTasks().or(configuration.isDefaultValueForKillTasksOfPausedRequests()); | ||
if (requestCleanup.getRunShellCommandBeforeKill().isPresent()) { | ||
|
@@ -458,6 +467,24 @@ private TaskCleanupType pause(SingularityRequestCleanup requestCleanup, Iterable | |
return cleanupType; | ||
} | ||
|
||
private void delete(SingularityRequestCleanup requestCleanup, Iterable<SingularityTaskId> activeTaskIds){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ssalinas I kept the name as delete since it followed the pattern set by the other actions (pause, bounce, etc.) |
||
final long start = System.currentTimeMillis(); | ||
|
||
for (SingularityTaskId taskId : activeTaskIds) { | ||
LOG.debug("Adding task {} to cleanup (delete)", taskId.getId()); | ||
|
||
Optional<SingularityTaskShellCommandRequestId> runBeforeKillId = Optional.absent(); | ||
|
||
if (requestCleanup.getRunShellCommandBeforeKill().isPresent()) { | ||
SingularityTaskShellCommandRequest shellRequest = new SingularityTaskShellCommandRequest(taskId, requestCleanup.getUser(), System.currentTimeMillis(), requestCleanup.getRunShellCommandBeforeKill().get()); | ||
taskManager.saveTaskShellCommandRequestToQueue(shellRequest); | ||
runBeforeKillId = Optional.of(shellRequest.getId()); | ||
} | ||
|
||
taskManager.createTaskCleanup(new SingularityTaskCleanup(requestCleanup.getUser(), TaskCleanupType.USER_REQUESTED_DESTROY, start, taskId, requestCleanup.getMessage(), requestCleanup.getActionId(), runBeforeKillId)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missed this one initially, my fault here. This has some unintended consequences. With this in place, any task that is forcibly killed by a user will then cause the request to be deleted as well. We need a new |
||
} | ||
} | ||
|
||
private void cleanupDeployState(SingularityRequestCleanup requestCleanup) { | ||
SingularityDeleteResult deletePendingDeployResult = deployManager.deletePendingDeploy(requestCleanup.getRequestId()); | ||
SingularityDeleteResult deleteRequestDeployStateResult = deployManager.deleteRequestDeployState(requestCleanup.getRequestId()); | ||
|
@@ -536,6 +563,7 @@ private void drainTaskCleanupQueue() { | |
|
||
final Multiset<SingularityDeployKey> incrementalCleaningTasks = HashMultiset.create(cleanupTasks.size()); | ||
final Set<SingularityDeployKey> isBouncing = new HashSet<>(cleanupTasks.size()); | ||
final Map<String, List<String>> deletedRequestIdToTaskIds = new HashMap<>(); | ||
|
||
final List<SingularityTaskId> cleaningTasks = Lists.newArrayListWithCapacity(cleanupTasks.size()); | ||
for (SingularityTaskCleanup cleanupTask : cleanupTasks) { | ||
|
@@ -546,6 +574,9 @@ private void drainTaskCleanupQueue() { | |
if (cleanupTask.getCleanupType() == TaskCleanupType.BOUNCING || cleanupTask.getCleanupType() == TaskCleanupType.INCREMENTAL_BOUNCE) { | ||
isBouncing.add(SingularityDeployKey.fromTaskId(cleanupTask.getTaskId())); | ||
} | ||
if (cleanupTask.getCleanupType() == TaskCleanupType.USER_REQUESTED_DESTROY) { | ||
updateRequestToTaskMap(cleanupTask, deletedRequestIdToTaskIds); | ||
} | ||
} | ||
|
||
LOG.info("Cleaning up {} tasks", cleanupTasks.size()); | ||
|
@@ -555,17 +586,19 @@ private void drainTaskCleanupQueue() { | |
int killedTasks = 0; | ||
|
||
for (SingularityTaskCleanup cleanupTask : cleanupTasks) { | ||
SingularityTaskId taskId = cleanupTask.getTaskId(); | ||
|
||
if (!isValidTask(cleanupTask)) { | ||
LOG.info("Couldn't find a matching active task for cleanup task {}, deleting..", cleanupTask); | ||
taskManager.deleteCleanupTask(cleanupTask.getTaskId().getId()); | ||
taskManager.deleteCleanupTask(taskId.getId()); | ||
} else if (shouldKillTask(cleanupTask, activeTaskIds, cleaningTasks, incrementalCleaningTasks) && checkLBStateAndShouldKillTask(cleanupTask)) { | ||
driverManager.killAndRecord(cleanupTask.getTaskId(), cleanupTask.getCleanupType(), cleanupTask.getUser()); | ||
|
||
taskManager.deleteCleanupTask(cleanupTask.getTaskId().getId()); | ||
driverManager.killAndRecord(taskId, cleanupTask.getCleanupType(), cleanupTask.getUser()); | ||
cleanupRequestIfNoRemainingTasks(taskId, deletedRequestIdToTaskIds); | ||
taskManager.deleteCleanupTask(taskId.getId()); | ||
|
||
killedTasks++; | ||
} else if (cleanupTask.getCleanupType() == TaskCleanupType.BOUNCING || cleanupTask.getCleanupType() == TaskCleanupType.INCREMENTAL_BOUNCE) { | ||
isBouncing.remove(SingularityDeployKey.fromTaskId(cleanupTask.getTaskId())); | ||
isBouncing.remove(SingularityDeployKey.fromTaskId(taskId)); | ||
} | ||
} | ||
|
||
|
@@ -582,6 +615,33 @@ private void drainTaskCleanupQueue() { | |
LOG.info("Killed {} tasks in {}", killedTasks, JavaUtils.duration(start)); | ||
} | ||
|
||
private void updateRequestToTaskMap(SingularityTaskCleanup cleanupTask, Map<String, List<String>> requestIdToTaskIds) { | ||
SingularityTaskId taskId = cleanupTask.getTaskId(); | ||
List<String> requestTasks = requestIdToTaskIds.get(taskId.getRequestId()); | ||
if (requestTasks != null) { | ||
requestTasks.add(taskId.getId()); | ||
} else { | ||
requestTasks = new ArrayList<>(Collections.singletonList(taskId.getId())); | ||
requestIdToTaskIds.put(taskId.getRequestId(), requestTasks); | ||
} | ||
} | ||
|
||
private void cleanupRequestIfNoRemainingTasks(SingularityTaskId taskId, Map<String, List<String>> requestIdToTaskIds) { | ||
String requestId = taskId.getRequestId(); | ||
if (requestIdToTaskIds.get(requestId) == null) { | ||
return; | ||
} | ||
List<String> requestTasks = requestIdToTaskIds.get(requestId); | ||
requestTasks.remove(taskId.getId()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
if (requestTasks.isEmpty()) { | ||
requestManager.createCleanupRequest( | ||
new SingularityRequestCleanup( | ||
Optional.<String> absent(), RequestCleanupType.DELETING, System.currentTimeMillis(), | ||
Optional.of(Boolean.TRUE), requestId, Optional.<String> absent(), | ||
Optional.<Boolean> absent(), Optional.<String> absent(), Optional.<String> absent(), Optional.<SingularityShellCommand>absent())); | ||
} | ||
} | ||
|
||
private boolean checkLBStateAndShouldKillTask(SingularityTaskCleanup cleanupTask) { | ||
final long start = System.currentTimeMillis(); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to still be able to grab request data while it is in
DELETING
state, we shouldn't have this line here. This deletes all of the current request data. We should probably move this step to be triggered by the cleaner after everything is done (i.e. when the active deploy cleanup is happening)