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

Get rid of the hidden pool for partially satisfied tasks #5638

Closed
hjoliver opened this issue Jul 25, 2023 · 3 comments · Fixed by #5658
Closed

Get rid of the hidden pool for partially satisfied tasks #5638

hjoliver opened this issue Jul 25, 2023 · 3 comments · Fixed by #5658
Assignees
Milestone

Comments

@hjoliver
Copy link
Member

hjoliver commented Jul 25, 2023

(From Element chat)

I think we should ditch the scheduler's hidden task pool, which contains spawned task proxies with partially satisfied prerequisites.

The motivation for the hidden pool was the idea that tasks should only be spawned "on demand" when all of their prerequisites are satisfied, rather than when the first one is satisfied. The hidden pool achieves this while still allowing us to use task proxies to track their own prerequisites. It makes partially satisfied waiting tasks look the same as any other future "waiting" (completely unsatisfied) tasks.

(Note the idea that this was desirable came from an early purist take on the SoD concept; it predates our current conception of the "active" pool, which does not contain just submitted and running tasks, even though it was implemented a bit later on).

The problem with this is that partially satisfied waiting tasks are NOT the same as completely unsatisfied ones - we have decreed that they can stall the workflow! (i.e. their presence, when there is nothing to run, indicates that something has gone wrong).

So, thanks to the hidden pool a workflow can be stalled by tasks that are not visible in the n-window, which is likely to be very confusing to users.

Putting partially satisfied waiting tasks in the active pool would:

  • simplify the scheduler quite a bit
  • make partially satisfied tasks visible regardless of n-window size

This would be logically consistent, because the active pool (n=0) also contains other tasks that are not "active" in the trivial sense of the word:

  • incomplete tasks that are "actively" waiting to be made complete
  • waiting tasks that are actively waiting on a retry timer
  • waiting tasks that are actively waiting on an xtrigger (including clock xtriggers)
  • and now: waiting tasks that are actively waiting on their remaining prerequisites (which if not satisfied will stall the workflow)

(Use of "actively" ⬆️ is how we should explain the "active pool" content to users).

Note there would normally be incomplete tasks present and visible in n=0 that are the root cause of partially satisfied prerequisites downstream, BUT not necessarily.

Any task that is "actively" doing something that could affect the evolution of the workflow (including stall it), should be in the n=0 "active window". This includes multi-parent tasks waiting on remaining prerequisites.

@hjoliver hjoliver added this to the cylc-8.3.0 milestone Jul 25, 2023
@hjoliver hjoliver self-assigned this Jul 25, 2023
@dwsutherland
Copy link
Member

Possibly add

  • waiting tasks that are actively waiting on an xtrigger

In light of:
#5732

(as there are some situations where TUI is showing no tasks (i.e. no active tasks), however, when clock trigger/xtrigger is satisfied the task shows up and runs)

@hjoliver
Copy link
Member Author

hjoliver commented Sep 15, 2023

I do have that above. Oops I guess I wrote "external trigger" rather than xtrigger, but that's what I meant (damn those old-style external triggers). I'll fix the description.

Note this issue is done already, in my cylc set PR.

@dwsutherland
Copy link
Member

Sorry, haven't wadded through the code completely yet.. Just thought I would preempt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants