From f891fa1776b57e4517faa1f749be43d7a04c853e Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 10:44:55 +1200 Subject: [PATCH 1/8] Handle self-suicide properly. --- cylc/flow/task_pool.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cylc/flow/task_pool.py b/cylc/flow/task_pool.py index b9c265747fb..b3572ade308 100644 --- a/cylc/flow/task_pool.py +++ b/cylc/flow/task_pool.py @@ -1169,7 +1169,8 @@ def spawn_on_output(self, itask, output, forced=False): self._get_hidden_task_by_id(c_taskid) or self._get_main_task_by_id(c_taskid) ) - if c_task is not None: + if c_task is not None and c_task != itask: + # (Avoid self-suicide: a:fail => !a) # Child already exists, update it. self.merge_flows(c_task, itask.flow_nums) self.workflow_db_mgr.put_insert_task_states( @@ -1181,7 +1182,11 @@ def spawn_on_output(self, itask, output, forced=False): ) # self.workflow_db_mgr.process_queued_ops() - elif (itask.flow_nums or forced) and not itask.flow_wait: + elif ( + c_task is None + and (itask.flow_nums or forced) + and not itask.flow_wait + ): c_task = self.spawn_task( c_name, c_point, itask.flow_nums, ) From d35ecdca246bf8d36eb6a9ac45e1dac8db9928de Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 11:37:18 +1200 Subject: [PATCH 2/8] Don't count self-suicide as a normal trigger. --- cylc/flow/taskdef.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cylc/flow/taskdef.py b/cylc/flow/taskdef.py index 8519629f7c5..8cb94bffcc0 100644 --- a/cylc/flow/taskdef.py +++ b/cylc/flow/taskdef.py @@ -291,7 +291,9 @@ def has_only_abs_triggers(self, point): for trig in dep.task_triggers: if ( trig.offset_is_absolute or - trig.offset_is_from_icp + trig.offset_is_from_icp or + # Don't count self-suicide as a normal trigger. + dep.suicide and trig.task_name == self.name ): has_abs = True else: From 267a0e13cc89efa49b3b0c08222e5e78fe545d48 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 13:41:14 +1200 Subject: [PATCH 3/8] Add new func tests for self-induced suicide. --- .../16-c7backcompat-self-suicide.t | 24 +++++++++++++++++ .../reference.log | 2 ++ .../16-c7backcompat-self-suicide/suite.rc | 19 +++++++++++++ .../17-c7backcompat-self-suicide-cycling.t | 24 +++++++++++++++++ .../reference.log | 3 +++ .../suite.rc | 27 +++++++++++++++++++ 6 files changed, 99 insertions(+) create mode 100644 tests/functional/spawn-on-demand/16-c7backcompat-self-suicide.t create mode 100644 tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/reference.log create mode 100644 tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc create mode 100644 tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling.t create mode 100644 tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/reference.log create mode 100644 tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc diff --git a/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide.t b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide.t new file mode 100644 index 00000000000..d9f79880797 --- /dev/null +++ b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide.t @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +#------------------------------------------------------------------------------- +# Cylc 8 back-compat mode. +# Test self-induced suicide. + +. "$(dirname "$0")/test_header" +set_test_number 2 +reftest +exit diff --git a/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/reference.log b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/reference.log new file mode 100644 index 00000000000..7a534a2f236 --- /dev/null +++ b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/reference.log @@ -0,0 +1,2 @@ +1/foo -triggered off [] in flow 1 +1/baz -triggered off ['1/foo'] in flow 1 diff --git a/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc new file mode 100644 index 00000000000..720152c52e6 --- /dev/null +++ b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc @@ -0,0 +1,19 @@ +# suite.rc: Cylc 8 back-compat mode. + +# GitHub cylc-flow #4968: self-induced suicide should not retrigger foo below. +[scheduler] + [[events]] + stall timeout = PT10S + abort on stall timeout = True + expected task failures = 1/foo +[scheduling] + [[dependencies]] + graph = """ + foo => bar + foo:fail => !foo & !bar + foo:fail | bar => baz + """ +[runtime] + [[foo]] + script = false + [[bar, baz]] diff --git a/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling.t b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling.t new file mode 100644 index 00000000000..3f5e04d2d6f --- /dev/null +++ b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling.t @@ -0,0 +1,24 @@ +#!/usr/bin/env bash +# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE. +# Copyright (C) NIWA & British Crown (Met Office) & Contributors. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +#------------------------------------------------------------------------------- +# Cylc 8 back-compat mode. +# Test self-induced suicide (cycling workflow, absolute triggers). + +. "$(dirname "$0")/test_header" +set_test_number 2 +reftest +exit diff --git a/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/reference.log b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/reference.log new file mode 100644 index 00000000000..bce9543df26 --- /dev/null +++ b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/reference.log @@ -0,0 +1,3 @@ +1/init -triggered off [] in flow 1 +1/bad -triggered off ['1/init'] in flow 1 +2/bad -triggered off ['1/init'] in flow 1 diff --git a/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc new file mode 100644 index 00000000000..68038bb1ed4 --- /dev/null +++ b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc @@ -0,0 +1,27 @@ +# suite.rc: Cylc 8 back compat mode. + +# GitHub cylc-flow #4968: self-induced suicide in the example below should not +# cause shutdown after the initial cycle point. + +[scheduler] + [[events]] + stall timeout = PT10S + abort on stall timeout = True + expected task failures = 1/bad, 2/bad +[scheduling] + cycling mode = integer + initial cycle point = 1 + final cycle point = 2 + [[dependencies]] + [[[R1]]] + graph = init + [[[P1]]] + graph = """ + init[^] => bad => good + bad:fail => !bad & !good + """ +[runtime] + [[init, good]] + script = true + [[bad]] + script = false From 5402eebddf9238fd98b0369927762507f6a046d7 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 14:02:44 +1200 Subject: [PATCH 4/8] Update change log. --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index da39722d144..53693725efc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -51,6 +51,9 @@ default job runner directives for platforms. ### Fixes +[#4970](https://github.com/cylc/cylc-flow/pull/4970) - Fix handling of suicide +triggers in back-compat mode. + [#4887](https://github.com/cylc/cylc-flow/pull/4887) - Disallow relative paths in `global.cylc[install]source dirs`. From 0cef1b2edd7971473d81888d6f3218f08db5c968 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 22:26:56 +1200 Subject: [PATCH 5/8] Update cylc/flow/task_pool.py Co-authored-by: Tim Pillinger <26465611+wxtim@users.noreply.github.com> --- cylc/flow/task_pool.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/task_pool.py b/cylc/flow/task_pool.py index b3572ade308..32738deb826 100644 --- a/cylc/flow/task_pool.py +++ b/cylc/flow/task_pool.py @@ -1170,8 +1170,8 @@ def spawn_on_output(self, itask, output, forced=False): or self._get_main_task_by_id(c_taskid) ) if c_task is not None and c_task != itask: - # (Avoid self-suicide: a:fail => !a) # Child already exists, update it. + # (unless trying to remove itself: a:fail => !a) self.merge_flows(c_task, itask.flow_nums) self.workflow_db_mgr.put_insert_task_states( c_task, From 518ffed57365a999a8c00f6517483bee263a21f7 Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 22:27:53 +1200 Subject: [PATCH 6/8] Update tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- .../spawn-on-demand/16-c7backcompat-self-suicide/suite.rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc index 720152c52e6..c73bead89ae 100644 --- a/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc +++ b/tests/functional/spawn-on-demand/16-c7backcompat-self-suicide/suite.rc @@ -3,7 +3,7 @@ # GitHub cylc-flow #4968: self-induced suicide should not retrigger foo below. [scheduler] [[events]] - stall timeout = PT10S + stall timeout = PT0S abort on stall timeout = True expected task failures = 1/foo [scheduling] From 549299bb1b71ee4fc1fd68f31b442ae7c600572c Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 22:28:19 +1200 Subject: [PATCH 7/8] Update tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> --- .../17-c7backcompat-self-suicide-cycling/suite.rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc index 68038bb1ed4..1912ff21152 100644 --- a/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc +++ b/tests/functional/spawn-on-demand/17-c7backcompat-self-suicide-cycling/suite.rc @@ -5,7 +5,7 @@ [scheduler] [[events]] - stall timeout = PT10S + stall timeout = PT0S abort on stall timeout = True expected task failures = 1/bad, 2/bad [scheduling] From d3680ff0b3cb500b716798ace51f40c5e8d8528b Mon Sep 17 00:00:00 2001 From: Hilary James Oliver Date: Thu, 7 Jul 2022 23:07:23 +1200 Subject: [PATCH 8/8] Add a comment. --- cylc/flow/task_pool.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cylc/flow/task_pool.py b/cylc/flow/task_pool.py index 32738deb826..528d33cb5ae 100644 --- a/cylc/flow/task_pool.py +++ b/cylc/flow/task_pool.py @@ -1187,6 +1187,9 @@ def spawn_on_output(self, itask, output, forced=False): and (itask.flow_nums or forced) and not itask.flow_wait ): + # If child is not in the pool already, and parent belongs to a + # flow (so it can spawn children), and parent is not waiting + # for an upcoming flow merge before spawning ... then spawn it. c_task = self.spawn_task( c_name, c_point, itask.flow_nums, )