flyteorg / flyte

Scalable and flexible workflow orchestration platform that seamlessly unifies data, ML and analytics stacks.
https://flyte.org
Apache License 2.0
5.17k stars 550 forks source link

[BUG] array node sub node control flow inheritance #5505

Open pvditt opened 5 days ago

pvditt commented 5 days ago

Tracking issue

Why are the changes needed?

Array node increments workflow parallelism from the parent node. Right now, the same control flow that tracks parallelism for the workflow is passed down to subnodes allowing subnodes to bump the workflow parallelism which could lead to a double counting of nodes getting evaluated in a workflow.

If/when ArrayNode supports subworkflows or dynamics then we can pass back in the parent control flow.

What changes were proposed in this pull request?

Init a new control flow when creating the exec context for array node subnodes

How was this patch tested?

Ran a few workflows locally

Setup process

Screenshots

Check all the applicable boxes

Related PRs

Docs link

codecov[bot] commented 5 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 61.01%. Comparing base (8805613) to head (a00afea).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #5505 +/- ## ========================================== + Coverage 60.15% 61.01% +0.85% ========================================== Files 646 794 +148 Lines 45883 51445 +5562 ========================================== + Hits 27603 31389 +3786 - Misses 15672 17164 +1492 - Partials 2608 2892 +284 ``` | [Flag](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | Coverage Δ | | |---|---|---| | [unittests-datacatalog](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `69.31% <ø> (ø)` | | | [unittests-flyteadmin](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `58.73% <ø> (ø)` | | | [unittests-flytecopilot](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `17.79% <ø> (ø)` | | | [unittests-flytectl](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `68.03% <ø> (?)` | | | [unittests-flyteidl](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `79.04% <ø> (ø)` | | | [unittests-flyteplugins](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `61.85% <ø> (ø)` | | | [unittests-flytepropeller](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `57.31% <100.00%> (+<0.01%)` | :arrow_up: | | [unittests-flytestdlib](https://app.codecov.io/gh/flyteorg/flyte/pull/5505/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg) | `65.82% <ø> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=flyteorg#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pvditt commented 4 days ago

@hamersaw This would mess with the node + task count. I'll look back into this.