-
Notifications
You must be signed in to change notification settings - Fork 94
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
Kill tasks during job prep #6535
base: 8.4.x
Are you sure you want to change the base?
Changes from all commits
d3be5ec
21be5b0
da76cf4
02a025e
9146357
475962b
adf59f3
378e65b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Ensure tasks can be killed while in the preparing state. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,6 +123,11 @@ | |
|
||
|
||
if TYPE_CHECKING: | ||
# BACK COMPAT: typing_extensions.Literal | ||
# FROM: Python 3.7 | ||
# TO: Python 3.8 | ||
from typing_extensions import Literal | ||
|
||
from cylc.flow.task_proxy import TaskProxy | ||
from cylc.flow.workflow_db_mgr import WorkflowDatabaseManager | ||
|
||
|
@@ -156,10 +161,10 @@ class TaskJobManager: | |
|
||
def __init__(self, workflow, proc_pool, workflow_db_mgr, | ||
task_events_mgr, data_store_mgr, bad_hosts): | ||
self.workflow = workflow | ||
self.workflow: str = workflow | ||
self.proc_pool = proc_pool | ||
self.workflow_db_mgr: WorkflowDatabaseManager = workflow_db_mgr | ||
self.task_events_mgr = task_events_mgr | ||
self.task_events_mgr: TaskEventsManager = task_events_mgr | ||
self.data_store_mgr = data_store_mgr | ||
self.job_file_writer = JobFileWriter() | ||
self.job_runner_mgr = self.job_file_writer.job_runner_mgr | ||
|
@@ -196,6 +201,15 @@ def kill_task_jobs( | |
self._kill_task_jobs_callback_255 | ||
) | ||
|
||
def kill_prep_task(self, itask: 'TaskProxy') -> None: | ||
"""Kill a preparing task.""" | ||
itask.waiting_on_job_prep = False | ||
itask.local_job_file_path = None # reset for retry | ||
self._set_retry_timers(itask) | ||
self._prep_submit_task_job_error( | ||
self.workflow, itask, '(killed in job prep)', '' | ||
) | ||
Comment on lines
+204
to
+211
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking around at other _prep_submit_task_job_error calls, we might want to consider adding: itask.summary['platforms_used'][itask.submit_num] = '' |
||
|
||
def poll_task_jobs(self, workflow, itasks, msg=None): | ||
"""Poll jobs of specified tasks. | ||
|
||
|
@@ -220,14 +234,19 @@ def poll_task_jobs(self, workflow, itasks, msg=None): | |
self._poll_task_jobs_callback_255 | ||
) | ||
|
||
def prep_submit_task_jobs(self, workflow, itasks, check_syntax=True): | ||
def prep_submit_task_jobs( | ||
self, | ||
workflow: str, | ||
itasks: 'Iterable[TaskProxy]', | ||
check_syntax: bool = True, | ||
) -> 'Tuple[List[TaskProxy], List[TaskProxy]]': | ||
"""Prepare task jobs for submit. | ||
|
||
Prepare tasks where possible. Ignore tasks that are waiting for host | ||
select command to complete. Bad host select command or error writing to | ||
a job file will cause a bad task - leading to submission failure. | ||
|
||
Return [list, list]: list of good tasks, list of bad tasks | ||
Return (good_tasks, bad_tasks) | ||
""" | ||
prepared_tasks = [] | ||
bad_tasks = [] | ||
|
@@ -244,17 +263,12 @@ def prep_submit_task_jobs(self, workflow, itasks, check_syntax=True): | |
prepared_tasks.append(itask) | ||
elif prep_task is False: | ||
bad_tasks.append(itask) | ||
return [prepared_tasks, bad_tasks] | ||
return (prepared_tasks, bad_tasks) | ||
|
||
def submit_task_jobs( | ||
self, | ||
workflow, | ||
itasks, | ||
curve_auth, | ||
client_pub_key_dir, | ||
run_mode: RunMode = RunMode.LIVE, | ||
): | ||
"""Prepare for job submission and submit task jobs. | ||
def submit_livelike_task_jobs( | ||
self, workflow, itasks, curve_auth, client_pub_key_dir | ||
) -> 'List[TaskProxy]': | ||
"""Submission for live tasks and dummy tasks. | ||
|
||
Preparation (host selection, remote host init, and remote install) | ||
is done asynchronously. Newly released tasks may be sent here several | ||
|
@@ -264,24 +278,9 @@ def submit_task_jobs( | |
Once preparation has completed or failed, reset .waiting_on_job_prep in | ||
task instances so the scheduler knows to stop sending them back here. | ||
|
||
This method uses prep_submit_task_job() as helper. | ||
This method uses prep_submit_task_jobs() as helper. | ||
|
||
Return (list): list of tasks that attempted submission. | ||
""" | ||
# submit "simulation/skip" mode tasks, modify "dummy" task configs: | ||
itasks, submitted_nonlive_tasks = self.submit_nonlive_task_jobs( | ||
workflow, itasks, run_mode) | ||
|
||
# submit "live" mode tasks (and "dummy" mode tasks) | ||
submitted_live_tasks = self.submit_livelike_task_jobs( | ||
workflow, itasks, curve_auth, client_pub_key_dir) | ||
|
||
return submitted_nonlive_tasks + submitted_live_tasks | ||
|
||
def submit_livelike_task_jobs( | ||
self, workflow, itasks, curve_auth, client_pub_key_dir | ||
) -> 'List[TaskProxy]': | ||
"""Submission for live tasks and dummy tasks. | ||
Return: tasks that attempted submission. | ||
""" | ||
done_tasks: 'List[TaskProxy]' = [] | ||
# Mapping of platforms to task proxies: | ||
|
@@ -327,7 +326,7 @@ def submit_livelike_task_jobs( | |
bc_mgr = self.task_events_mgr.broadcast_mgr | ||
rtconf = bc_mgr.get_updated_rtconfig(itask) | ||
try: | ||
platform = get_platform( | ||
platform = get_platform( # type: ignore[assignment] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in #6564 |
||
rtconf, | ||
bad_hosts=self.bad_hosts | ||
) | ||
|
@@ -1029,7 +1028,7 @@ def _set_retry_timers( | |
def submit_nonlive_task_jobs( | ||
self: 'TaskJobManager', | ||
workflow: str, | ||
itasks: 'List[TaskProxy]', | ||
itasks: 'Iterable[TaskProxy]', | ||
workflow_run_mode: RunMode, | ||
) -> 'Tuple[List[TaskProxy], List[TaskProxy]]': | ||
"""Identify task mode and carry out alternative submission | ||
|
@@ -1152,7 +1151,7 @@ def _prep_submit_task_job( | |
workflow: str, | ||
itask: 'TaskProxy', | ||
check_syntax: bool = True | ||
): | ||
) -> 'Union[TaskProxy, None, Literal[False]]': | ||
"""Prepare a task job submission. | ||
|
||
Returns: | ||
|
@@ -1217,7 +1216,7 @@ def _prep_submit_task_job( | |
else: | ||
# host/platform select not ready | ||
if host_n is None and platform_name is None: | ||
return | ||
return None | ||
elif ( | ||
host_n is None | ||
and rtconfig['platform'] | ||
|
@@ -1259,7 +1258,7 @@ def _prep_submit_task_job( | |
workflow, itask, '(platform not defined)', exc) | ||
return False | ||
else: | ||
itask.platform = platform | ||
itask.platform = platform # type: ignore[assignment] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in #6564 |
||
# Retry delays, needed for the try_num | ||
self._set_retry_timers(itask, rtconfig) | ||
|
||
|
@@ -1292,7 +1291,13 @@ def _prep_submit_task_job( | |
itask.local_job_file_path = local_job_file_path | ||
return itask | ||
|
||
def _prep_submit_task_job_error(self, workflow, itask, action, exc): | ||
def _prep_submit_task_job_error( | ||
self, | ||
workflow: str, | ||
itask: 'TaskProxy', | ||
action: str, | ||
exc: Union[Exception, str], | ||
) -> None: | ||
"""Helper for self._prep_submit_task_job. On error.""" | ||
log_task_job_activity( | ||
SubProcContext(self.JOBS_SUBMIT, action, err=exc, ret_code=1), | ||
|
@@ -1306,11 +1311,12 @@ def _prep_submit_task_job_error(self, workflow, itask, action, exc): | |
# than submit-failed | ||
# provide a dummy job config - this info will be added to the data | ||
# store | ||
try_num = itask.get_try_num() | ||
itask.jobs.append({ | ||
'task_id': itask.identity, | ||
'platform': itask.platform, | ||
'submit_num': itask.submit_num, | ||
'try_num': itask.get_try_num(), | ||
'try_num': try_num, | ||
}) | ||
# create a DB entry for the submit-failed job | ||
self.workflow_db_mgr.put_insert_task_jobs( | ||
|
@@ -1319,7 +1325,7 @@ def _prep_submit_task_job_error(self, workflow, itask, action, exc): | |
'flow_nums': serialise_set(itask.flow_nums), | ||
'job_id': itask.summary.get('submit_method_id'), | ||
'is_manual_submit': itask.is_manual_submit, | ||
'try_num': itask.get_try_num(), | ||
'try_num': try_num, | ||
'time_submit': get_current_time_string(), | ||
'platform_name': itask.platform['name'], | ||
'job_runner_name': itask.summary['job_runner_name'], | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of moving this task submission logic into the scheduler.
I'm guessing you did this to avoid having to populate the extra args in integration tests, fair enough.
Maybe just leave a shim in scheduler.py to do this, would rather keep submission logic in one place, Cylc logic has a habit of sprawling.