Skip to content

Better handling of errors in scheduler poller #2233

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.

Sign up for GitHub

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

Merged
WH77 merged 6 commits into from
Oct 20, 2021
Merged

Better handling of errors in scheduler poller #2233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
084b413
debug cron error in scheduler loop
Sep 22, 2021
6b34104
do not abort singularityschedulerpoller on error
Sep 22, 2021
f10d6f8
allow some scheduled pending requests to fail without killing all of …
Sep 22, 2021
88d8307
remove pointless cron expression test
Sep 28, 2021
34de1d8
add metric for unscheduled tasks
Oct 1, 2021
169ca43
allow scheduler poller to abort on unhandled errors
Oct 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions SingularityService/src/main/java/com/hubspot/singularity/SingularityMainModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ public class SingularityMainModule implements Module {
public static final String CURRENT_HTTP_REQUEST = "_singularity_current_http_request";

public static final String LOST_TASKS_METER = "singularity.lost.tasks.meter";
public static final String UNSCHEDULED_TASKS_METER =
"singularity.unscheduled.tasks.meter";

public static final String STATUS_UPDATE_DELTAS = "singularity.status.update.deltas";
public static final String LAST_MESOS_MASTER_HEARTBEAT_TIME =
Expand Down Expand Up @@ -507,6 +509,13 @@ public Meter providesLostTasksMeter(MetricRegistry registry) {
return registry.meter("com.hubspot.singularity.lostTasks");
}

@Provides
@Singleton
@Named(UNSCHEDULED_TASKS_METER)
public Meter providesUnscheduledTasksMeter(MetricRegistry registry) {
return registry.meter("com.hubspot.singularity.unscheduledTasks");
}

@Provides
@Singleton
@Named(STATUS_UPDATE_DELTAS)
Expand Down
31 changes: 29 additions & 2 deletions SingularityService/src/main/java/com/hubspot/singularity/scheduler/SingularityScheduler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.hubspot.singularity.scheduler;

import com.codahale.metrics.Meter;
import com.codahale.metrics.annotation.Timed;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.inject.Inject;
import com.google.inject.name.Named;
import com.hubspot.mesos.JavaUtils;
import com.hubspot.mesos.Resources;
import com.hubspot.singularity.DeployState;
Expand All @@ -22,6 +24,7 @@
import com.hubspot.singularity.SingularityDeployStatisticsBuilder;
import com.hubspot.singularity.SingularityKilledTaskIdRecord;
import com.hubspot.singularity.SingularityMachineAbstraction;
import com.hubspot.singularity.SingularityMainModule;
import com.hubspot.singularity.SingularityManagedThreadPoolFactory;
import com.hubspot.singularity.SingularityPendingDeploy;
import com.hubspot.singularity.SingularityPendingRequest;
Expand Down Expand Up @@ -104,6 +107,7 @@ public class SingularityScheduler {
private final SingularitySchedulerLock lock;
private final ExecutorService schedulerExecutorService;
private final SingularityMesosSchedulerClient mesosSchedulerClient;
private final Meter unscheduledTasksMetric;

private final Map<SingularityTaskId, Long> requestedReconciles;

Expand All @@ -122,7 +126,8 @@ public SingularityScheduler(
SingularityLeaderCache leaderCache,
SingularitySchedulerLock lock,
SingularityManagedThreadPoolFactory threadPoolFactory,
SingularityMesosSchedulerClient mesosSchedulerClient
SingularityMesosSchedulerClient mesosSchedulerClient,
@Named(SingularityMainModule.UNSCHEDULED_TASKS_METER) Meter unscheduledTasksMetric
) {
this.taskRequestManager = taskRequestManager;
this.configuration = configuration;
Expand All @@ -140,6 +145,7 @@ public SingularityScheduler(
threadPoolFactory.get("scheduler", configuration.getCoreThreadpoolSize());
this.mesosSchedulerClient = mesosSchedulerClient;
this.requestedReconciles = new HashMap<>();
this.unscheduledTasksMetric = unscheduledTasksMetric;
}

private void cleanupTaskDueToDecomission(
Expand Down Expand Up @@ -359,7 +365,7 @@ public void drainPendingQueue() {
() ->
lock.runWithRequestLock(
() ->
handlePendingRequestsForDeployKey(
handlePendingRequestsForDeployKeySafe(
obsoleteRequests,
heldForScheduledActiveTask,
totalNewScheduledTasks,
Expand All @@ -384,6 +390,27 @@ public void drainPendingQueue() {
);
}

private void handlePendingRequestsForDeployKeySafe(
AtomicInteger obsoleteRequests,
AtomicInteger heldForScheduledActiveTask,
AtomicInteger totalNewScheduledTasks,
SingularityDeployKey deployKey,
List<SingularityPendingRequest> pendingRequestsForDeploy
) {
try {
handlePendingRequestsForDeployKey(
obsoleteRequests,
heldForScheduledActiveTask,
totalNewScheduledTasks,
deployKey,
pendingRequestsForDeploy
);
} catch (Exception e) {
LOG.error("Error handling pending requests for {}, skipping", deployKey, e);
unscheduledTasksMetric.mark();
Copy link
Contributor Author

@WH77 WH77 Oct 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debated just reusing the lost tasks meter here, could be misleading (same task would get "lost" repeatedly) but would be fairly correct in spirit - @pschoenfelder you have any preference here?

}
}

private void handlePendingRequestsForDeployKey(
AtomicInteger obsoleteRequests,
AtomicInteger heldForScheduledActiveTask,
Expand Down