Support ArrayNode subNode timeouts

[hamersaw]

Tracking issue

N/A

Why are the changes needed?

Currently, timeouts are not supported on ArryaNode subNodes. If there is a timeout set in the task decorator, that is applied only to the overall ArrayNode and not individual subNodes. This change ensures subNodes are executed with timeouts as a standalone execution would be.

What changes were proposed in this pull request?

We store a deltaTimestamp to track startedAt for subNode executions. This delta is computed from the overall ArrayNode start time. This is stored using a bitarray (just like other ArrayNode subNode metadata), with a preset 18 bits of precision. This means for 5k subNodes there will be approximately 90kb added to the FlyteWorkflow CR - reducing scalability by a small amount. The 18 bits gives us around 3 days of delta time, meaning subNodes start times can be tracked up to 3 days after the overall ArrayNode starts. If this proves to be too imprecise, we can increase this value and incur further reductions to scale.

It is important to note that we do not have a task phase for TIMED_OUT. So in the UI this will display as FAILED. This is a restriction of the current eventing scheme for map tasks.

How was this patch tested?

This was tested locally under a variety of failure scenarios (ex. retryable failures).

Setup process

from datetime import timedelta
from flytekit import map_task, task, workflow
from typing import List

import time
from flytekit.exceptions.user import FlyteRecoverableException


@task(retries=3, timeout=timedelta(minutes=1))
def say_hello(name: str) -> str:
    #time.sleep(120)

    time.sleep(10)
    raise FlyteRecoverableException(f"testing failure for {name}")

    return f"hello {name}"


@workflow
def say_hello_wf(name: List[str]) -> List[str]:
    return map_task(say_hello)(name=name)

Screenshots

N/A

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

N/A

Docs link

N/A

[codecov]

Codecov Report

Attention: Patch coverage is 27.53623% with 50 lines in your changes missing coverage. Please review.

Project coverage is 37.09%. Comparing base (ab04192) to head (36601df).
Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
...eworkflow/v1alpha1/mocks/MutableArrayNodeStatus.go	0.00%	20 Missing ⚠️
...rkflow/v1alpha1/mocks/ExecutableArrayNodeStatus.go	0.00%	18 Missing ⚠️
...ler/pkg/apis/flyteworkflow/v1alpha1/node_status.go	0.00%	7 Missing ⚠️
...opeller/pkg/controller/nodes/node_state_manager.go	0.00%	4 Missing ⚠️
...lytepropeller/pkg/controller/nodes/transformers.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #6054    +/-   ##
========================================
  Coverage   37.08%   37.09%            
========================================
  Files        1318     1318            
  Lines      132284   132399   +115     
========================================
+ Hits        49062    49117    +55     
- Misses      78950    79010    +60     
  Partials     4272     4272            

Flag	Coverage Δ
unittests-datacatalog	51.58% <ø> (ø)
unittests-flyteadmin	54.10% <ø> (ø)
unittests-flytecopilot	30.99% <ø> (ø)
unittests-flytectl	62.29% <ø> (-0.05%)	⬇️
unittests-flyteidl	7.23% <ø> (-0.01%)	⬇️
unittests-flyteplugins	53.82% <ø> (+0.09%)	⬆️
unittests-flytepropeller	42.59% <27.53%> (-0.04%)	⬇️
unittests-flytestdlib	57.57% <ø> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pvditt]

changes look great. This reminded me of this issue https://linear.app/unionai/issue/COR-2212/[bug]-setting-task-metadata-for-the-parent-array-node-causes-incorrect where the timeout gets applied to the parent node. I'll get a small PR up to fix this on the flytekit end.

[hamersaw]

changes look great. This reminded me of this issue https://linear.app/unionai/issue/COR-2212/[bug]-setting-task-metadata-for-the-parent-array-node-causes-incorrect where the timeout gets applied to the parent node. I'll get a small PR up to fix this on the flytekit end.

This example workflow in ^^ works here. I think the timeout gets applied to the parent and subNodes. I know I looked through flytekit and convinced myself of it before implementing this, but can't seem to find the line now.

[pvditt]

confirmed this works as expected w/ flyteorg/flytekit#2979