fix reload failures on active/incomplete orphans #6817

dwsutherland · 2025-06-26T00:46:35Z

closes #6814
closes #6815

Note - A good way to see the bugs is to comment out a fix and run the test for each.

Check List

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
Tests are included (or explain why tests are not needed).
Changelog entry included if this is a change that can affect users
Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

wxtim · 2025-07-07T10:53:08Z

changes.d/6817.fix.md

@@ -0,0 +1 @@
+Fixes reload failures about orphaned tasks.


Cryptic IMO

Suggested change

Fixes reload failures about orphaned tasks.

Fixes a bug where reload removed information about orphaned tasks from the data store.

Even simpler:

Suggested change

Fixes reload failures about orphaned tasks.

Fixes a couple of rare scheduler crashes.

IMO we should at least hint at what triggers the bug. How about this?

Suggested change

Fixes reload failures about orphaned tasks.

Fixes two rare bugs associated with reloading the workflow configuration

after removing tasks or xtriggers from it.

wxtim · 2025-07-07T10:56:09Z

tests/integration/test_reload.py

+    start,
+    log_filter,
+):
+    """Reload should not fail about orphaned tasks.


Suggested change

"""Reload should not fail about orphaned tasks.

"""Reload should not remove orphaned task data.

The bug was not data being removed, it was the scheduler crashing

wxtim · 2025-07-07T10:57:31Z

tests/integration/test_reload.py

+        - Removal of both xtrigger and associated active/incomplete task.
+        - Broadcast deltas generated after reload.


Check that I have the issue mapping the right way around!

Suggested change

- Removal of both xtrigger and associated active/incomplete task.

- Broadcast deltas generated after reload.

- Removal of both xtrigger and associated active/incomplete task.

https://github.com/cylc/cylc-flow/issues/6814

- Broadcast deltas generated after reload.

https://github.com/cylc/cylc-flow/issues/6815

I think it's the other way round

wxtim · 2025-07-07T10:59:28Z

tests/integration/test_reload.py

+        'scheduler': {
+            'allow implicit tasks': True
+        },


Not required: The fixture adds this as a default. 😄

Suggested change

'scheduler': {

'allow implicit tasks': True

},

wxtim · 2025-07-07T10:59:43Z

tests/integration/test_reload.py

+        'scheduler': {
+            'allow implicit tasks': True
+        },


Suggested change

'scheduler': {

'allow implicit tasks': True

},

MetRonnie · 2025-07-14T11:31:42Z

changes.d/6817.fix.md

@@ -0,0 +1 @@
+Fixes reload failures about orphaned tasks.


Even simpler:

Suggested change

Fixes reload failures about orphaned tasks.

Fixes a couple of rare scheduler crashes.

MetRonnie · 2025-07-14T11:36:36Z

tests/integration/test_reload.py

+        - Removal of both xtrigger and associated active/incomplete task.
+        - Broadcast deltas generated after reload.


I think it's the other way round

MetRonnie · 2025-07-14T11:41:11Z

tests/integration/test_reload.py

+    schd = scheduler(id_)
+    async with start(schd):
+        # spawn in bar
+        foo = schd.pool.get_tasks()[0]


Suggested change

foo = schd.pool.get_tasks()[0]

foo = schd.pool._get_task_by_id('20010101T0000Z/foo')

MetRonnie · 2025-07-14T11:41:13Z

tests/integration/test_reload.py

+        bar = schd.pool.get_tasks()[0]
+        assert bar.identity == '20010101T0000Z/bar'


Suggested change

bar = schd.pool.get_tasks()[0]

assert bar.identity == '20010101T0000Z/bar'

bar = schd.pool._get_task_by_id('20010101T0000Z/bar')

MetRonnie · 2025-07-14T11:41:46Z

tests/integration/test_reload.py

+        schd.pool.task_events_mgr.process_message(
+            foo, '20010101T0000Z', 'succeeded')


The 2nd arg is severity, not cycle point

Suggested change

schd.pool.task_events_mgr.process_message(

foo, '20010101T0000Z', 'succeeded')

schd.pool.task_events_mgr.process_message(foo, 'INFO', 'succeeded')

MetRonnie · 2025-07-14T11:41:48Z

tests/integration/test_reload.py

+        schd.pool.task_events_mgr.process_message(
+            bar, '20010101T0000Z', 'failed')


Suggested change

schd.pool.task_events_mgr.process_message(

bar, '20010101T0000Z', 'failed')

schd.pool.task_events_mgr.process_message(bar, 'INFO', 'failed')

hjoliver · 2025-07-15T07:40:28Z

tests/integration/test_reload.py

+):
+    """Reload should not fail about orphaned tasks.
+
+    The following aspects of reload about orphans are tested:


Can "reload about orphans" be rephrased more clearly? "reload after removing tasks from the workflow config" maybe?

fix reload removed xtrigger data-store task deltas

7fc0703

dwsutherland added this to the 8.4.4 milestone Jun 26, 2025

dwsutherland self-assigned this Jun 26, 2025

dwsutherland added the bug Something is wrong :( label Jun 26, 2025

dwsutherland added 2 commits June 26, 2025 13:07

test added

36d7e29

changelog entry added

6e14ae8

dwsutherland force-pushed the reload-xtriggered-task-fixes branch from e1b725c to 6e14ae8 Compare June 26, 2025 01:07

dwsutherland added small labels Jun 26, 2025

dwsutherland mentioned this pull request Jun 25, 2025

set non-existent task while reloading causes crash #6814

Open

dwsutherland requested a review from MetRonnie July 2, 2025 03:47

MetRonnie requested a review from wxtim July 2, 2025 11:29

wxtim reviewed Jul 7, 2025

View reviewed changes

MetRonnie reviewed Jul 14, 2025

View reviewed changes

This was linked to issues Jul 14, 2025

reload fail on xtrigger removal causing depleted task pool #6815

Open

set non-existent task while reloading causes crash #6814

Open

hjoliver reviewed Jul 15, 2025

View reviewed changes

oliver-sanders modified the milestones: 8.4.4, 8.4.x Jul 18, 2025

MetRonnie force-pushed the 8.4.x branch from bba6f72 to 970df19 Compare July 22, 2025 09:05

	Fixes reload failures about orphaned tasks.
	Fixes a bug where reload removed information about orphaned tasks from the data store.

	Fixes reload failures about orphaned tasks.
	Fixes a couple of rare scheduler crashes.

	Fixes reload failures about orphaned tasks.
	Fixes two rare bugs associated with reloading the workflow configuration
	after removing tasks or xtriggers from it.

	"""Reload should not fail about orphaned tasks.
	"""Reload should not remove orphaned task data.

		- Removal of both xtrigger and associated active/incomplete task.
		- Broadcast deltas generated after reload.

	foo = schd.pool.get_tasks()[0]
	foo = schd.pool._get_task_by_id('20010101T0000Z/foo')

		bar = schd.pool.get_tasks()[0]
		assert bar.identity == '20010101T0000Z/bar'

	bar = schd.pool.get_tasks()[0]
	assert bar.identity == '20010101T0000Z/bar'
	bar = schd.pool._get_task_by_id('20010101T0000Z/bar')

		schd.pool.task_events_mgr.process_message(
		foo, '20010101T0000Z', 'succeeded')

		schd.pool.task_events_mgr.process_message(
		bar, '20010101T0000Z', 'failed')

fix reload failures on active/incomplete orphans #6817

Are you sure you want to change the base?

fix reload failures on active/incomplete orphans #6817

Uh oh!

Conversation

dwsutherland commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxtim Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hjoliver Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dwsutherland commented Jun 26, 2025 •

edited

Loading

wxtim Jul 7, 2025 •

edited

Loading

hjoliver Jul 15, 2025 •

edited

Loading