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

Release tasks from hidden pool even if workflow paused. #4436

Merged

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Sep 28, 2021

Close #4131

  • if the workflow is paused, hold tasks just before job prep (i.e., not in the hidden pool, or in the queue)
  • new icon to distinguish succeeded from expired in cylc tui (note the datastore, as a temporary measure, is marking historical tasks with the expired state)
  • spawn parentless tasks out to the runahead limit immediately instead of relying partially on the main loop to do it

Requirements 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.py and conda-environment.yml.
  • Already covered by existing tests. (tests/integration/test_task_pool.py modified)
  • Appropriate change log entry included.
  • No documentation update required.

@hjoliver hjoliver added this to the cylc-8.0b3 milestone Sep 28, 2021
@hjoliver hjoliver self-assigned this Sep 28, 2021
@oliver-sanders
Copy link
Member

Tested, task now shows up as queued rather than runahead which makes much more sense 👍.

@hjoliver hjoliver force-pushed the release-from-runahead-even-if-paused branch from 0a84fd3 to a902d5f Compare September 29, 2021 02:58
@hjoliver hjoliver changed the title Release tasks from runahead even if workflow paused. Release tasks from hidden pool even if workflow paused. Sep 29, 2021
@hjoliver
Copy link
Member Author

hjoliver commented Sep 29, 2021

The example from #4131, with the workflow paused and a.10 triggered manually:

  • a.1 and a.10 are waiting in the scheduler task pool, for the workflow to be released
  • a.2 and a.11 (and a.9) are in the n=1 window relative to the scheduler task pool
  • a.9 is a historical task at n=1, in the sense that it comes before an active task in the graph; the datastore marks these as expired for now (in future, it will consult the database)

shot

@hjoliver
Copy link
Member Author

hjoliver commented Sep 29, 2021

Note I chose a circle with a strike through it for the expired task state icon. Think of a waiting task that has been "cancelled" because it has expired. Is it too similar to the submit-failed icon? Maybe not. The meaning isn't entirely different (both mean the task is no longer waiting but never started running) and it's the most rarely seen state. However, it's easy to change if someone has a better idea.

shot

@hjoliver hjoliver force-pushed the release-from-runahead-even-if-paused branch 4 times, most recently from 58640b4 to 8c63d63 Compare September 30, 2021 06:52
@MetRonnie
Copy link
Member

Note I chose a circle with a strike through it for the expired task state icon. Think of a waiting task that has been "cancelled" because it has expired. Is it too similar to the submit-failed icon? Maybe not. The meaning isn't entirely different (both mean the task is no longer waiting but never started running) and it's the most rarely seen state. However, it's easy to change if someone has a better idea.

Some ideas:

  • U+229D "Circled Dash": ⊝
  • U+2296 "Circled Minus": ⊖
  • U+24D4 "Circled Latin Small Letter E": ⓔ
  • U+25CC "Dotted Circle": ◌
  • U+29B8 "Circled Reverse Solidus": ⦸

I'm not sure about font support for these

@hjoliver
Copy link
Member Author

New feature added to the Issue description above:

  • spawn parentless tasks out to the runahead limit immediately instead of relying partially on the main loop to do it

This branch was giving me trouble with some functional and integration tests because it was hard to understand and predict the task pool content before the limit was reached. Now, even with cylc play --pause parentless tasks will exist out to the limit.

[scheduling]
    cycling mode = integer
    initial cycle point = 1
    runahead limit = P5
    [[graph]]
        P1 = foo => bar => baz
[runtime]
    [[foo, bar, baz]]

Start up paused:
shot1

Delayed case, not paused:

[scheduling]
    cycling mode = integer
    initial cycle point = 1
    runahead limit = P5
    [[graph]]
        R1 = foo
        P1 = foo[^] => bar => baz
[runtime]
    [[foo, bar, baz]]
        script = sleep 2

shot2

@hjoliver
Copy link
Member Author

On current master, the start-paused case will show only foo.1 (as runahead limited, which is confusing) and the delayed absolute trigger case will spawn out the runahead limit somewhat incrementally.

@hjoliver
Copy link
Member Author

hjoliver commented Oct 1, 2021

@oliver-sanders - tagging you for review, can you assign another from your end according to availability?

@hjoliver hjoliver force-pushed the release-from-runahead-even-if-paused branch from 5e93d25 to 5ae4d52 Compare October 1, 2021 04:23
@oliver-sanders
Copy link
Member

U+25CC "Dotted Circle": ◌

I thought that was how I had set it up in the first instance. Perhaps I thought that expired might become a badge rather than a top-level task state. Dunno.

Dotted circle comes from the same font table as the other circles so should have the same relative dimensions and font support.

cylc/flow/task_job_mgr.py Outdated Show resolved Hide resolved
cylc/flow/task_pool.py Show resolved Hide resolved
cylc/flow/task_pool.py Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member Author

hjoliver commented Oct 8, 2021

U+25CC "Dotted Circle": ◌

I thought that was how I had set it up in the first instance. Perhaps I thought that expired might become a badge rather than a top-level task state. Dunno.

Dotted circle comes from the same font table as the other circles so should have the same relative dimensions and font support.

OK, changing to dotted circle. It looks OK in TUI.

cylc/flow/taskdef.py Outdated Show resolved Hide resolved
cylc/flow/taskdef.py Outdated Show resolved Hide resolved
tests/integration/test_task_pool.py Show resolved Hide resolved
@hjoliver hjoliver force-pushed the release-from-runahead-even-if-paused branch from 22a3cf7 to c524cb6 Compare October 13, 2021 03:42
@oliver-sanders oliver-sanders merged commit 970a9c7 into cylc:master Oct 14, 2021
@hjoliver hjoliver deleted the release-from-runahead-even-if-paused branch October 14, 2021 18:18
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 this pull request may close these issues.

triggering tasks beyond the scheduler window
3 participants