Flow-specific hold/release #5698

hjoliver · 2023-08-17T02:41:50Z

Close #4277

Hold/release tasks in a specific flow (or any flow, by default)

Implement cylc hold --flow=n
Same for cylc release --flow=n

If --flow is not used, matching tasks will be held or released regardless of flow number.

(most of the time there will only be one flow)

If --flow=n is specified, matching tasks will be held or released if their flow numbers include n

(if a merged task foo has flow numbers {2,3} it belongs to flow 2 (and 3), so if I ask to hold foo in flow 2 (or 3) it should be held. Ditto for release).

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).
CHANGES.md entry included if this is a change that can affect users
Cylc-Doc pull request opened if required at document flow-specific hold and release cylc-doc#646
If this is a bug fix, PR should be raised against the relevant ?.?.x branch. (not a bug fix)

cylc/flow/scheduler.py

hjoliver · 2023-08-17T02:52:15Z

Test case:

[scheduling]
    [[graph]]
        R1 = "a => b => c => d"
[runtime]
    [[a,b,c,d]]
        pre-script = "sleep 2"
    [[a]]
        script = """
            if (( CYLC_TASK_SUBMIT_NUMBER == 1)); then
                cylc hold --flow=2 ${CYLC_WORKFLOW_ID}//1/b
            fi
        """
    [[c]]
        script = """
            if (( CYLC_TASK_SUBMIT_NUMBER == 1)); then
                cylc trigger --flow=new ${CYLC_WORKFLOW_ID}//1/a
            fi
        """

at start-up, a puts a future hold on b in flow 2
flow 1 runs b and c, which starts flow 2 at a
flow 2 gets held at b

hjoliver · 2023-09-01T23:05:50Z

One thing to note, on "hold after cycle point" functionality:

This branch makes it flow-specific, in that you can start a new flow and tell just that flow to hold after a given point:

cylc trigger --flow=new ...
cylc hold --after=4 --flow=2

However, there is still only one global hold-after point, whether it is for all tasks or for a single flow, so canceling the hold point doesn't need to be flow-specific:

cylc release --all ...

In future we should probably allow multiple flow-specific hold points, but I don't think that's a high priority. This branch is still a significant enhancment as-is. I expect most demand will be for single-flow re-runs that we don't want to carry on indefinitely (so stop or hold at a future point).

hjoliver · 2023-09-01T23:22:20Z

This bad boy is ready for review.

hjoliver · 2023-09-07T00:33:39Z

Test coverage good without any functional tests, although that makes me a bit nervous for this sort of thing. Any opinions on that?

dwsutherland · 2023-09-07T02:06:42Z

With the example, does the 2nd flow get merged into the first flow?

It ends, after release;

(flow) sutherlander@cortex:cylc-flow$ cylc release --flow=2 fhold/run1//1/b
Done

(confirmed working)

(flow) sutherlander@cortex:cylc-flow$ cylc release --flow=1 fhold/run1//1/b
Done
(flow) sutherlander@cortex:cylc-flow$ cylc release --flow=3 fhold/run1//1/b
Done

(confirmed not working 👍 )
with flow 2 coming out the other end:

{
  "data": {
    "workflows": [
      {
        "id": "~sutherlander/fhold/run1",
        "taskProxies": [
          {
            "id": "~sutherlander/fhold/run1//1/c",
            "state": "succeeded",
            "flowNums": "[1]",
            "isHeld": false
          },
          {
            "id": "~sutherlander/fhold/run1//1/d",
            "state": "running",
            "flowNums": "[2]",
            "isHeld": false
          }
        ],
        "familyProxies": [
          {
            "id": "~sutherlander/fhold/run1//1/root",
            "state": "running"
          }
        ]
      }
    ]
  }
}

Kind of beyond the scope of this PR, but I guess I'll need to look into whether the flow numbers are correct in the data-store (perhaps no-merge is default but I would have expected flow number "[1, 2]" coming out of it otherwise).

Looks good.

hjoliver · 2023-09-07T02:43:54Z

With the example, does the 2nd flow get merged into the first flow?

No, merging only occurs if the same task from different flows meet in n=0. Here, flow=1 finishes before flow-2 can catch it up:

$ cylc log exa --no-timestamp | grep "=> succeeded" | sed -e 's/^.*-//g'
 [1/a running job:01 flows:1] => succeeded
 [1/b running job:01 flows:1] => succeeded
 [1/c running job:01 flows:1] => succeeded
 [1/a running job:02 flows:2] => succeeded
 [1/d running job:01 flows:1] => succeeded  # <----- flow 1 finished
 [1/b running job:02 flows:2] => succeeded
 [1/c running job:02 flows:2] => succeeded
 [1/d running job:02 flows:2] => succeeded

oliver-sanders · 2023-10-11T15:48:54Z

cylc/flow/scripts/hold.py

+    parser.add_option(
+        "--flow",
+        help="Hold tasks that belong to a specific flow.",
+        metavar="INT", action="store", dest="flow_num")


Presumably --flow=all is the default?

oliver-sanders · 2023-10-11T15:51:41Z

cylc/flow/network/schema.py

+        flow_num = Int(
+            description='Number of flow to hold.'
+        )


Should the default be flow=all?

The pattern used in other mutations is a field called flow of type Flow which accepts either an integer or a string. See FlowMutationArguments in this file.

In this case new and none don't make sense, so it might need a new type.

wxtim · 2023-10-12T10:41:58Z

cylc/flow/flow_mgr.py

@@ -30,6 +31,15 @@
 FLOW_NONE = "none"


+def validate_flow_opt(val):


Doesn't this semi duplicate the logic in cylc trigger? Surely all use-cases of --flow should behave roughly the same, and probably be centralized in option_parser.py. Centralizing them here is reasonable too, but on balance this is more about options than flows?

You might want to give it a kwarg where you pass it a list of acceptable strings?

Note, there are some differences, e.g. cylc trigger --flow=new makes sense, but cylc hold --flow=new doesn't.

wxtim · 2023-10-12T14:42:30Z

cylc/flow/flow_mgr.py

@@ -30,6 +31,15 @@
 FLOW_NONE = "none"


+def validate_flow_opt(val):
+    """Validate command line --flow opions."""
+    if val is not None:


How will the parser allow this value to be None?

wxtim · 2023-10-12T15:29:26Z

cylc/flow/task_pool.py

+    ) -> None:
+        """Flag that we should hold a future task."""
+        self.hold[(name, point)] = flow_num
+        self._update_stores((name, point, True))


I'm concerned that this means that you can only flag a future task for one flow at a time:

> cylc hold flows-hold//10100101T0000Z/a --flow 1 Done > sqlite3 ~/cylc-run/flows-hold/runN/log/db 'SELECT * FROM tasks_to_hold' a|10100101T0000Z|1 > cylc hold flows-hold//10100101T0000Z/a --flow 2 Done > sqlite3 ~/cylc-run/flows-hold/runN/log/db 'SELECT * FROM tasks_to_hold' a|10100101T0000Z|2

I would expect to get either

a|10100101T0000Z|1,2 # or a|10100101T0000Z|1 a|10100101T0000Z|2

I also think that this means that:

> cylc hold flows-hold//10100101T0000Z/a --flow 1 --flow 2 Done > sqlite3 ~/cylc-run/flows-hold/runN/log/db 'SELECT * FROM tasks_to_hold' a|10100101T0000Z|2

Is not what I as a user would expect: I would expect this to either return an error ("don't use --flow twice please") or set that task as held in both flows as above.

Looking at cylc trigger --help:

--flow=FLOW Assign the triggered task to all active flows (all); no flow (none); a new flow (new); or a specific flow (e.g. 2). The default is all. Reuse the option to assign multiple specific flows.

Ok, in that case it's only the Reuse the option that I have a problem with.

wxtim

I have had a good look at this. I think it mostly works as advertised, but I have a couple of questions.

hjoliver · 2024-02-27T04:40:38Z

[Update]: I think this is basically done, I just have to come back to respond to some review comments. It's possible I can do that in time for 8.3.0, let's see.

hjoliver added 2 commits August 17, 2023 14:34

Task pool flow hold.

e8d957c

Future flow hold.

58dc6c5

hjoliver added this to the cylc-8.3.0 milestone Aug 17, 2023

hjoliver self-assigned this Aug 17, 2023

hjoliver commented Aug 17, 2023

View reviewed changes

cylc/flow/scheduler.py Outdated Show resolved Hide resolved

hjoliver added 8 commits August 18, 2023 10:35

Flow-specific hold release.

95fa798

Centralise task hold/release code.

1e7d548

Style fixes.

b5f83a4

Fix integration tests.

584bea8

fix hold on kill

77acf44

Fix func tests.

ad81aa3

Fix more func tests.

2b570d5

Update change log.

ff07668

hjoliver added the sod-follow-up label Aug 31, 2023

hjoliver added 8 commits September 1, 2023 11:07

Fix hold on kill.

d3c776c

hold mgr: clear not needed.

b2e7440

Validate CLI --flow opt.

151537e

Update flow hold integration tests.

f554c96

fix release from hold.

6597431

Update flow release integration tests.

e1fb7fe

hold_point with flow num

9e93454

Fix recent.

e912f63

hjoliver marked this pull request as ready for review September 1, 2023 23:06

hjoliver requested a review from dwsutherland September 7, 2023 00:28

hjoliver mentioned this pull request Sep 7, 2023

document flow-specific hold and release cylc/cylc-doc#646

Open

dwsutherland approved these changes Sep 7, 2023

View reviewed changes

hjoliver mentioned this pull request Oct 7, 2023

future task globbing #5763

Closed

oliver-sanders reviewed Oct 11, 2023

View reviewed changes

oliver-sanders requested a review from wxtim October 11, 2023 15:52

wxtim reviewed Oct 12, 2023

View reviewed changes

oliver-sanders modified the milestones: cylc-8.3.0, cylc-8.4.0 Feb 19, 2024

oliver-sanders removed the sod-follow-up label Apr 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flow-specific hold/release #5698

Flow-specific hold/release #5698

hjoliver commented Aug 17, 2023 •

edited

hjoliver commented Aug 17, 2023 •

edited

hjoliver commented Sep 1, 2023 •

edited

hjoliver commented Sep 1, 2023 •

edited

hjoliver commented Sep 7, 2023

dwsutherland commented Sep 7, 2023 •

edited

hjoliver commented Sep 7, 2023 •

edited

oliver-sanders Oct 11, 2023

oliver-sanders Oct 11, 2023

wxtim Oct 12, 2023 •

edited

oliver-sanders Oct 12, 2023

wxtim Oct 12, 2023

wxtim Oct 12, 2023

oliver-sanders Oct 12, 2023 •

edited

wxtim Oct 13, 2023

wxtim left a comment

hjoliver commented Feb 27, 2024

		@@ -30,6 +31,15 @@
		FLOW_NONE = "none"


		def validate_flow_opt(val):

Flow-specific hold/release #5698

Are you sure you want to change the base?

Flow-specific hold/release #5698

Conversation

hjoliver commented Aug 17, 2023 • edited

hjoliver commented Aug 17, 2023 • edited

hjoliver commented Sep 1, 2023 • edited

hjoliver commented Sep 1, 2023 • edited

hjoliver commented Sep 7, 2023

dwsutherland commented Sep 7, 2023 • edited

hjoliver commented Sep 7, 2023 • edited

oliver-sanders Oct 11, 2023

Choose a reason for hiding this comment

oliver-sanders Oct 11, 2023

Choose a reason for hiding this comment

wxtim Oct 12, 2023 • edited

Choose a reason for hiding this comment

oliver-sanders Oct 12, 2023

Choose a reason for hiding this comment

wxtim Oct 12, 2023

Choose a reason for hiding this comment

wxtim Oct 12, 2023

Choose a reason for hiding this comment

oliver-sanders Oct 12, 2023 • edited

Choose a reason for hiding this comment

wxtim Oct 13, 2023

Choose a reason for hiding this comment

wxtim left a comment

Choose a reason for hiding this comment

hjoliver commented Feb 27, 2024

hjoliver commented Aug 17, 2023 •

edited

hjoliver commented Aug 17, 2023 •

edited

hjoliver commented Sep 1, 2023 •

edited

hjoliver commented Sep 1, 2023 •

edited

dwsutherland commented Sep 7, 2023 •

edited

hjoliver commented Sep 7, 2023 •

edited

wxtim Oct 12, 2023 •

edited

oliver-sanders Oct 12, 2023 •

edited