jihaozh commented on a change in pull request #4554: [TE] Add timeout for all
tasks and clean up RUNNING states if timeout
URL: https://github.com/apache/incubator-pinot/pull/4554#discussion_r317266945
##########
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/task/TaskDriver.java
##########
@@ -98,21 +105,33 @@ public void start() throws Exception {
ThirdeyeMetricsUtil.taskCounter.inc();
try {
- LOG.info("Executing task: {} {}", anomalyTaskSpec.getJobName(),
- anomalyTaskSpec.getTaskInfo());
+ LOG.info("Executing task: {} {}",
anomalyTaskSpec.getJobName(), anomalyTaskSpec.getTaskInfo());
// execute the selected task
TaskType taskType = anomalyTaskSpec.getTaskType();
TaskRunner taskRunner =
TaskRunnerFactory.getTaskRunnerFromTaskType(taskType);
TaskInfo taskInfo =
TaskInfoFactory.getTaskInfoFromTaskType(taskType,
anomalyTaskSpec.getTaskInfo());
-
updateTaskStartTime(anomalyTaskSpec.getId());
- List<TaskResult> taskResults = taskRunner.execute(taskInfo,
taskContext);
+ Future<List<TaskResult>> future =
taskExecutorService.submit(new Callable<List<TaskResult>>() {
+ @Override
+ public List<TaskResult> call() throws Exception {
+ return taskRunner.execute(taskInfo, taskContext);
+ }
+ });
+ try {
+ List<TaskResult> taskResults =
future.get(driverConfiguration.getMaxTaskRunTimeMillis(),
TimeUnit.MILLISECONDS);
+ } catch (TimeoutException e) {
Review comment:
Should we merge the try-catch with the outter level try-catch block? That
looks cleaner.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
