Additional job check #37
Conversation
kannon/master.py
Outdated
| if task.make_unique_id() in launched_task_ids: | ||
| if task.make_unique_id() in self.task_id_to_job_name: | ||
| # check if task is still running on child job | ||
| assert self._check_child_task_status(task), f"Child task {self._gen_task_info(task)} failed." |
There was a problem hiding this comment.
It would be better to keep the master node running while the child nodes are running. If we add ownerReference to child node, the running child nodes will be killed by Kubernetes. #38
What do you think?
There was a problem hiding this comment.
@kitagry
Sorry for late reply!
You mean we should not kill the master node while one or more child nodes are running, right?
In the current implementation, the master node fails right after finding a child node failed, even when other child nodes are still living.
And, in my opinion, when it finds out that a child node fails, it means a failure of the whole pipeline, so I would prefer the current one.
However, as you pointed out, there might be a need for the strategy where we do not stop the master node and all living child nodes keep running even other child nodes fail.
So, I'll create an issue about it, and prefer keeping the current implementation for now.
There was a problem hiding this comment.
kannon/master.py
Outdated
| if task.make_unique_id() in launched_task_ids: | ||
| if task.make_unique_id() in self.task_id_to_job_name: | ||
| # check if task is still running on child job | ||
| assert self._check_child_task_status(task), f"Child task {self._gen_task_info(task)} failed." |
There was a problem hiding this comment.
This assert will not be called, because this method raise error if job is not succeed.
There was a problem hiding this comment.
Thanks, now fixed!
Add status checking of submitted child job, in order to return earlier when the job failed.
Also updated unit tests.