cylc / cylc-flow

Cylc: a workflow engine for cycling systems.
https://cylc.github.io
GNU General Public License v3.0
335 stars 94 forks source link

Housekeep broadcasts on task completion #6359

Closed wxtim closed 3 weeks ago

wxtim commented 2 months ago

Closes #6308

[!NOTE] This is a moderate change in behaviour, so I've targeted 8.4.

Previously broadcasts have been housekept when they refer to a Cycle point the scheduler has moved on from. This probably doesn't make sense in a multi-flow system.

This PR moves the housekeeping to the point of completion, and doesn't attempt to re-expire broadcasts with every cycle of the main-loop. This allows users to broadcast a task from the past and trigger it without the broadcast being deleted before the trigger.

I've left the interface to expire broadcasts alone, so that users can still use if if they like.

Check List

wxtim commented 2 months ago

Question (originally posed by @oliver-sanders in a meeting):

Do we need to housekeep broadcasts?

I think that any method of housekeeping broadcasts will interact with multiple flows and/or workflow interventions in non-intuititive ways for users.

oliver-sanders commented 2 months ago

Do we need to housekeep broadcasts?

An alternative approach might be to not housekeep broadcasts (i.e allow them to pile up in the DB), but only hold in memory the broadcasts that apply to the cycles that are active at any time (avoiding memory-leak type issues, not that broadcasts are likely to use much memory).

I think that any method of housekeeping broadcasts will interact with multiple flows and/or workflow interventions in non-intuititive ways for users.

E.G. If you try to re-run a collection of tasks, you might be surprised to find that the settings previously broadcasted to them (perhaps by a start_cycle task) have been cleared.

E.G. If you're developing a task using the "edit runtime" feature in the GUI, you might be surprised to find that if the task fails, the broadcast is preserved, but if it succeeds, the broadcast is cleared (assuming success is the only required output).

wxtim commented 1 month ago

@oliver-sanders - are we waiting for a comment from @hjoliver ?

oliver-sanders commented 1 month ago

Yes. I think, in light of our team meeting, we should talk this over before pushing ahead.

oliver-sanders commented 3 weeks ago

Thanks for this fix @wxtim.

At the meeting, we decided to have a go at a no-housekeep approach to make broadcasts work more in line with expectation, see https://github.com/cylc/cylc-flow/issues/6308#issuecomment-2456675250