Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

runservice: global and runconfig runs and tasks timeout #6

Closed
sgotti opened this issue May 24, 2019 · 3 comments · Fixed by #331
Closed

runservice: global and runconfig runs and tasks timeout #6

sgotti opened this issue May 24, 2019 · 3 comments · Fixed by #331
Labels
enhancement New feature or request

Comments

@sgotti
Copy link
Member

sgotti commented May 24, 2019

No description provided.

@alessandro-sorint
Copy link
Contributor

I are working to this issue, so I write some ideas:

To add a field "task_timeout" to the Executor config globally and another "task_timeout" in runconfig in the Task struct.
If task_timeout value in the runconfig is greater than the globally field, we can return an error in the CheckRunConfigTasks method.
For the run timeout, I would like to add a run_timeout fileld in the Runservice and in the runconfig Run struct, with a check of the value in the Runservice newRun method.

To check the timeouts of the runs the idea is to add a new routine in the Runservice or modify the executorTasksCleaner.
If a run exced the timeout the routine will call the StopRun action.

To check the task timeout I think the better way is to modify the taskUpdater method in the Executor.

@sgotti
Copy link
Member Author

sgotti commented Mar 7, 2022

To add a field "task_timeout" to the Executor config globally and another "task_timeout" in runconfig in the Task struct.
If task_timeout value in the runconfig is greater than the globally field, we can return an error in the CheckRunConfigTasks method.

You should probably also consider a task_timeout at the run level and at the global config level that will be inherited by all runs and tasks if no task_timeout exists at their level.

For the run timeout, I would like to add a run_timeout fileld in the Runservice and in the runconfig Run struct, with a check of the value in the Runservice newRun method.

I'll leave a task_timeout defined in the executor config outside this issue and for anothert issue if really needed.
Anyway it's wrong to return an error if the executor level task timeout is lower then the configured task timeout since users don't know how the executor timeout is configured.

For the run timeout, I would like to add a run_timeout fileld in the Runservice and in the runconfig Run struct, with a check of the value in the Runservice newRun method.

Now that tasks in agola may require approval probably a run_timeout as is doesn't makes much sense. Or it should be implemented to consider also tasks waiting for approval. I'll leave this in another PR after careful reasoning.

To check the task timeout I think the better way is to modify the taskUpdater method in the Executor.

It should be handled both at the executor level that at the runservice scheduler level since an executor may not be responding.

We should also consider how to report the timeout state to the runservice and how to store in the run status.

@alessandro-sorint
Copy link
Contributor

To add a field "task_timeout" to the Executor config globally and another "task_timeout" in runconfig in the Task struct.
If task_timeout value in the runconfig is greater than the globally field, we can return an error in the CheckRunConfigTasks method.

You should probably also consider a task_timeout at the run level and at the global config level that will be inherited by all runs and tasks if no task_timeout exists at their level.

For the run timeout, I would like to add a run_timeout fileld in the Runservice and in the runconfig Run struct, with a check of the value in the Runservice newRun method.

I'll leave a task_timeout defined in the executor config outside this issue and for anothert issue if really needed. Anyway it's wrong to return an error if the executor level task timeout is lower then the configured task timeout since users don't know how the executor timeout is configured.

Thanks @sgotti. So we'll have task_timeout in run config with 3 levels, with inheritance, it is similar to docker_registries_auth.

To check the task timeout I think the better way is to modify the taskUpdater method in the Executor.

It should be handled both at the executor level that at the runservice scheduler level since an executor may not be responding.

To handle in the executor level I confirm to use the taskUpdater method; for the runservice scheduler level I think to modify the executorTaskCleaner method

We should also consider how to report the timeout state to the runservice and how to store in the run status.
@sgotti I think to report the timeout state to the runservice we can use the api /executor/{executorid}/tasks/{taskid} with the phase value "timeout" and store the task status(RunTaskStatus) "timeout", the run result will be "failed".

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants