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

Fix suicide trigger & logic #2656

Merged
merged 5 commits into from
May 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions lib/cylc/conditional_simplifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,22 @@ def listify(cls, message):

Examples:
>>> ConditionalSimplifier.listify('(foo)')
[['foo']]
['foo']

>>> ConditionalSimplifier.listify('foo & (bar | baz)')
['foo', '&', ['bar', '|', 'baz']]

>>> ConditionalSimplifier.listify('(a&b)|(c|d)&(e|f)')
[['a', '&', 'b'], '|', ['c', '|', 'd'], '&', ['e', '|', 'f']]

>>> ConditionalSimplifier.listify('a & (b, c,)')
['a', '&', ['b, c,']]
>>> ConditionalSimplifier.listify('a & (b & c)')
['a', '&', ['b', '&', 'c']]

>>> ConditionalSimplifier.listify('a & b')
['a', '&', 'b']

>>> ConditionalSimplifier.listify('a & (b)')
['a', '&', 'b']

>>> ConditionalSimplifier.listify('((foo)')
Traceback (most recent call last):
Expand All @@ -78,6 +84,8 @@ def listify(cls, message):
stack.pop()
if not stack:
raise ValueError(message)
if isinstance(stack[-1][-1], list) and len(stack[-1][-1]) == 1:
stack[-1][-1] = stack[-1][-1][0]
if len(stack) > 1:
raise ValueError(message)
return ret_list
Expand Down Expand Up @@ -183,3 +191,8 @@ def flatten_nested_expr(cls, expr):
flattened = "(" + flattened
flattened += ")"
return flattened


if __name__ == "__main__":
import doctest
doctest.testmod()
8 changes: 0 additions & 8 deletions lib/cylc/network/httpclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,12 +734,4 @@ def test_compile_url_compiler_none_specified(self):
# has gone wrong.)
self.assertTrue(url.startswith("http"))

def test_get_data_from_url_single_http(self):
"""Test the get data from call_server_impl() function"""
myclient = SuiteRuntimeServiceClient("dummy-suite")
myclient.comms1[SuiteSrvFilesManager.KEY_COMMS_PROTOCOL] = 'http'
ret = myclient.call_server_impl(
'http://httpbin.org/get', 'GET', None)
self.assertEqual(ret["url"], "http://httpbin.org/get")

unittest.main()
22 changes: 11 additions & 11 deletions lib/cylc/task_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -987,17 +987,17 @@ def remove_suiciding_tasks(self):
"""
num_removed = 0
for itask in self.get_tasks():
if itask.state.suicide_prerequisites:
if itask.state.suicide_prerequisites_satisfied():
if itask.state.status in [TASK_STATUS_READY,
TASK_STATUS_SUBMITTED,
TASK_STATUS_RUNNING]:
LOG.warning('suiciding while active', itask=itask)
else:
LOG.info('suiciding', itask=itask)
self.force_spawn(itask)
self.remove(itask, 'suicide')
num_removed += 1
if (itask.state.suicide_prerequisites and
itask.state.suicide_prerequisites_are_all_satisfied()):
Copy link
Member

@oliver-sanders oliver-sanders May 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use of itask.state.suicide_prerequisites_are_all_satisfied is purposeful. This may cause issues where multiple suicide triggers are used in combination, see https://github.com/cylc/cylc/pull/2523/files#diff-9e759d26a5f403f3e52c71605f48269cR966.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I cannot see how you can change it to not break this case - when the prerequisite list implies the & logic.

if itask.state.status in [TASK_STATUS_READY,
TASK_STATUS_SUBMITTED,
TASK_STATUS_RUNNING]:
LOG.warning('suiciding while active', itask=itask)
else:
LOG.info('suiciding', itask=itask)
self.force_spawn(itask)
self.remove(itask, 'suicide')
num_removed += 1
return num_removed

def _get_earliest_unsatisfied_point(self):
Expand Down
6 changes: 0 additions & 6 deletions lib/cylc/task_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,6 @@ def prerequisites_are_not_all_satisfied(self):
return (not self.prerequisites_are_all_satisfied() or
not self.suicide_prerequisites_are_all_satisfied())

def suicide_prerequisites_satisfied(self):
"""Return True if any suicide prerequisites are satisfied."""
if self._suicide_is_satisfied is True:
return True
return any(preq.is_satisfied() for preq in self.suicide_prerequisites)

def suicide_prerequisites_are_all_satisfied(self):
"""Return True if all suicide prerequisites are satisfied."""
if self._suicide_is_satisfied is None:
Expand Down
40 changes: 24 additions & 16 deletions tests/triggering/17-suicide-multi.t
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,27 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# Test suicide triggering
# (this is currently just a copy of the tutorial suicide example, but I
# anticipate making it more fiendish at some point).
. $(dirname $0)/test_header
#-------------------------------------------------------------------------------
set_test_number 2
#-------------------------------------------------------------------------------
install_suite $TEST_NAME_BASE suicide-multi
#-------------------------------------------------------------------------------
TEST_NAME=$TEST_NAME_BASE-validate
run_ok $TEST_NAME cylc validate $SUITE_NAME
#-------------------------------------------------------------------------------
TEST_NAME=$TEST_NAME_BASE-run
suite_run_ok $TEST_NAME cylc run --reference-test --debug --no-detach $SUITE_NAME
#-------------------------------------------------------------------------------
purge_suite $SUITE_NAME
# Test "or" outputs from same task triggerting suicide triggering
. "$(dirname "$0")/test_header"
set_test_number 3
install_suite "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"

run_ok "${TEST_NAME_BASE}-validate" cylc validate "${SUITE_NAME}"
suite_run_ok "${TEST_NAME_BASE}" \
cylc run --reference-test --debug --no-detach "${SUITE_NAME}"
if which 'sqlite3' >'/dev/null'; then
DBFILE="$(cylc get-global-config --print-run-dir)/${SUITE_NAME}/log/db"
sqlite3 "${DBFILE}" 'SELECT * FROM task_pool ORDER BY cycle, name;' \
>'sqlite3.out'
cmp_ok 'sqlite3.out' <<'__OUT__'
2|fin|1|succeeded|
3|fin|1|succeeded|
3|good|1|succeeded|
3|showdown|1|succeeded|
__OUT__
else
skip 1 "sqlite3 not installed?"
fi

purge_suite "${SUITE_NAME}"
exit
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@
graph = """
fin[-P1] => showdown

showdown:good => good & ! bad & ! ugly
showdown:bad => bad & ! good & ! ugly
showdown:ugly => ugly & ! good & ! bad
showdown:good => good
showdown:bad => bad
showdown:ugly => ugly
showdown:good | showdown:bad => ! ugly
showdown:good | showdown:ugly => ! bad
showdown:bad | showdown:ugly => ! good

good | bad | ugly => fin
"""
Expand All @@ -23,11 +26,11 @@
[[showdown]]
script = """
if ! (( ${CYLC_TASK_CYCLE_POINT} % 3 )); then
cylc message 'The-Good'
cylc message -- "${CYLC_SUITE_NAME}" "${CYLC_TASK_JOB}" 'The-Good'
elif ! (( ( ${CYLC_TASK_CYCLE_POINT} + 1 ) % 3 )); then
cylc message 'The-Bad'
cylc message -- "${CYLC_SUITE_NAME}" "${CYLC_TASK_JOB}" 'The-Bad'
else
cylc message 'The-Ugly'
cylc message -- "${CYLC_SUITE_NAME}" "${CYLC_TASK_JOB}" 'The-Ugly'
fi
"""
[[[outputs]]]
Expand Down
22 changes: 22 additions & 0 deletions tests/triggering/18-conditional_simplifier-doc.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#!/bin/bash
# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
# Copyright (C) 2008-2018 NIWA
#
# 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 <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# cylc.conditional_simplifier doctest
. "$(dirname "$0")/test_header"
set_test_number 1

run_ok "${TEST_NAME_BASE}" python -m 'cylc.conditional_simplifier'
40 changes: 40 additions & 0 deletions tests/triggering/19-and-suicide.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#!/bin/bash
# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
# Copyright (C) 2008-2018 NIWA
#
# 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 <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# Test "and" outputs from 2 tasks triggering suicide.
# https://github.com/cylc/cylc/issues/2655
. "$(dirname "$0")/test_header"
set_test_number 3
install_suite "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"

run_ok "${TEST_NAME_BASE}-validate" cylc validate "${SUITE_NAME}"
suite_run_fail "${TEST_NAME_BASE}-run" \
cylc run --reference-test --debug --no-detach "${SUITE_NAME}"
if which 'sqlite3' >'/dev/null'; then
DBFILE="$(cylc get-global-config --print-run-dir)/${SUITE_NAME}/log/db"
sqlite3 "${DBFILE}" 'SELECT * FROM task_pool ORDER BY name;' >'sqlite3.out'
cmp_ok 'sqlite3.out' <<'__OUT__'
1|t0|1|succeeded|
1|t1|1|failed|
1|t2|1|succeeded|
__OUT__
else
skip 1 "sqlite3 not installed?"
fi

purge_suite "${SUITE_NAME}"
exit
5 changes: 5 additions & 0 deletions tests/triggering/19-and-suicide/reference.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
2018-05-08T15:09:34Z INFO - Initial point: 1
2018-05-08T15:09:34Z INFO - Final point: 1
2018-05-08T15:09:34Z INFO - [t0.1] -triggered off []
2018-05-08T15:09:34Z INFO - [t1.1] -triggered off []
2018-05-08T15:09:47Z INFO - [t2.1] -triggered off ['t0.1']
21 changes: 21 additions & 0 deletions tests/triggering/19-and-suicide/suite.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[cylc]
UTC mode = True
[[events]]
abort on stalled = True
[[reference test]]
expected task failures = t1.1
[scheduling]
[[dependencies]]
graph = """
t0:failed & t1:failed => !t2
t0 | t1 => t2
"""
[runtime]
[[t0]]
# https://github.com/cylc/cylc/issues/2655
# "t2.1" should not suicide on "t1.1:failed"
script = sleep 10
[[t1]]
script = false
[[t2]]
script = true
42 changes: 42 additions & 0 deletions tests/triggering/20-and-outputs-suicide.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/bin/bash
# THIS FILE IS PART OF THE CYLC SUITE ENGINE.
# Copyright (C) 2008-2018 NIWA
#
# 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 <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# Test "and" outputs from same task triggering suicide.
. "$(dirname "$0")/test_header"
set_test_number 3
install_suite "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"

run_ok "${TEST_NAME_BASE}-validate" cylc validate "${SUITE_NAME}"
suite_run_ok "${TEST_NAME_BASE}-run" \
cylc run --reference-test --debug --no-detach "${SUITE_NAME}"
if which 'sqlite3' >'/dev/null'; then
DBFILE="$(cylc get-global-config --print-run-dir)/${SUITE_NAME}/log/db"
sqlite3 "${DBFILE}" 'SELECT * FROM task_pool ORDER BY cycle, name;' \
>'sqlite3.out'
cmp_ok 'sqlite3.out' <<'__OUT__'
2|fin|1|succeeded|
3|bad|1|succeeded|
3|fin|1|succeeded|
3|good|1|succeeded|
3|showdown|1|succeeded|
__OUT__
else
skip 1 "sqlite3 not installed?"
fi

purge_suite "${SUITE_NAME}"
exit
14 changes: 14 additions & 0 deletions tests/triggering/20-and-outputs-suicide/reference.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
2018-05-08T15:09:34Z INFO - Initial point: 1
2018-05-08T15:09:34Z INFO - Final point: 3
2017-12-27T14:42:10Z INFO - [showdown.1] -triggered off []
2017-12-27T14:42:13Z INFO - [bad.1] -triggered off ['showdown.1']
2017-12-27T14:42:13Z INFO - [ugly.1] -triggered off ['showdown.1']
2017-12-27T14:42:16Z INFO - [fin.1] -triggered off ['bad.1', 'ugly.1']
2017-12-27T14:42:19Z INFO - [showdown.2] -triggered off ['fin.1']
2017-12-27T14:42:22Z INFO - [good.2] -triggered off ['showdown.2']
2017-12-27T14:42:22Z INFO - [ugly.2] -triggered off ['showdown.2']
2017-12-27T14:42:25Z INFO - [fin.2] -triggered off ['good.2', 'ugly.2']
2017-12-27T14:42:28Z INFO - [showdown.3] -triggered off ['fin.2']
2017-12-27T14:42:32Z INFO - [bad.3] -triggered off ['showdown.3']
2017-12-27T14:42:32Z INFO - [good.3] -triggered off ['showdown.3']
2017-12-27T14:42:35Z INFO - [fin.3] -triggered off ['bad.3', 'good.3']
44 changes: 44 additions & 0 deletions tests/triggering/20-and-outputs-suicide/suite.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
[cylc]
UTC mode = True
[[events]]
abort on stalled = True
[scheduling]
cycling mode = integer
initial cycle point = 1
final cycle point = 3
[[dependencies]]
[[[P1]]]
graph = """
fin[-P1] => showdown

showdown:good => good & ! bad & ! ugly
showdown:bad => bad & ! good & ! ugly
showdown:ugly => ugly & ! good & ! bad

# Note: The above implies:
# showdown:good => good
# showdown:bad => bad
# showdown:ugly => ugly
# showdown:good & showdown:bad => ! ugly
# showdown:good & showdown:ugly => ! bad
# showdown:bad & showdown:ugly => ! good

good | bad | ugly => fin
"""
[runtime]
[[showdown]]
script = """
if ((${CYLC_TASK_CYCLE_POINT} == 1)); then
cylc message -- "${CYLC_SUITE_NAME}" "${CYLC_TASK_JOB}" 'bad' 'ugly'
elif ((${CYLC_TASK_CYCLE_POINT} == 2)); then
cylc message -- "${CYLC_SUITE_NAME}" "${CYLC_TASK_JOB}" 'good' 'ugly'
else
cylc message -- "${CYLC_SUITE_NAME}" "${CYLC_TASK_JOB}" 'good' 'bad'
fi
"""
[[[outputs]]]
good = good
bad = bad
ugly = ugly
[[good, bad, ugly, fin]]
script = true