Skip to content
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

Cleaner shutdown #41

Merged
merged 8 commits into from
Jun 9, 2020
Merged

Cleaner shutdown #41

merged 8 commits into from
Jun 9, 2020

Conversation

jankatins
Copy link
Member

@jankatins jankatins commented May 26, 2020

This makes some attempts to clean up during shutdown.

  • The main thing is the atexit function in the run_pipeline function which simply closes a run/node_run when shutting down.
  • The atexit in run() will shutdown any still running processes.
  • It also attempts to kill the run_process when the run_pipeline process is shut down via strg+c. This should trigger the atexit function in run() so at least the children are killed. This does not do any attempt to do the same for any other signal yet

Also included is a fix to actually check all ancestors of a task when checking if any of them is already failed -> the effect is that we do not schedule any tasks from an already queued sub pipelines (like a parallel task) in case the parent of that subpipeline is failed.

With this in place I could successfully add some signal handler (not included yet, needs some more testing) which kills the running processes and closes the runs. It also handles strg+c in flask better, at least I didn't see leftover processes anymore.

partly covers: #40

jankatins added 4 commits May 26, 2020 12:20
Also kill the the statistics process in that case, so that one is not left arround
Before, we could have the effect that a pipeline would fail but a already queued subpipeline (e.g. a parallel task) would still start. Now we check all ancestors before starting a task. As we propagate failure state to all parent pipelines this should stop any new pipelines to get scheduled.
@jankatins jankatins mentioned this pull request May 26, 2020
@martin-loetzsch martin-loetzsch merged commit b72e6cb into master Jun 9, 2020
@martin-loetzsch martin-loetzsch deleted the cleaner_shutdown branch June 9, 2020 14:30
martin-loetzsch added a commit that referenced this pull request Jun 10, 2020
- Fix duplicated system stats if you run multiple ETLs in parallel (#38)
- Add config default_task_max_retries (#39)
- Cleaner shutdown (#41)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants