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

new ForEach topology show task in "parallel" while concurrency is set to 1 #4474

Closed
Ben8t opened this issue Jul 29, 2024 · 3 comments · Fixed by #4477
Closed

new ForEach topology show task in "parallel" while concurrency is set to 1 #4474

Ben8t opened this issue Jul 29, 2024 · 3 comments · Fixed by #4477
Assignees
Labels
area/backend Needs backend code changes bug Something isn't working

Comments

@Ben8t
Copy link
Member

Ben8t commented Jul 29, 2024

Describe the issue

When using the new ForEach task with concurency: 1 the topology still show tasks like if they were run in parallel. Not a big issue but could mislead users - especially those used to EachSequential + concurrency: 1 is the default value

image

Environment

  • Kestra Version: 0.18-SNAPSHOT
  • Operating System (OS/Docker/Kubernetes): Docker
  • Java Version (if you don't run kestra in Docker):
@Ben8t Ben8t added the bug Something isn't working label Jul 29, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Issues Jul 29, 2024
@anna-geller
Copy link
Member

anna-geller commented Jul 29, 2024

I can confirm topology view is not correct

image

even though backend-wise everything is fine (see execution is correctly running each task groups sequentially one after the other rather than in parallel as shown in the topology view):

image

id: test_for_each
namespace: kestra.qa

tasks:
  - id: for_each
    type: io.kestra.plugin.core.flow.ForEach
    values: ['value1', 'value2', 'value3']
    concurrencyLimit: 1
    tasks:
      - id: log1
        type: io.kestra.plugin.core.log.Log
        message: "{{ taskrun.value }}"
      
      - id: log2
        type: io.kestra.plugin.core.log.Log
        message: "{{ taskrun.value }}"

      - id: log3
        type: io.kestra.plugin.core.log.Log
        message: "{{ taskrun.value }}"

@MilosPaunovic can you have a look? the tasks in the group should be shown as a sequential group within ForEach

@anna-geller anna-geller added the area/frontend Needs frontend code changes label Jul 29, 2024
@MilosPaunovic MilosPaunovic self-assigned this Jul 30, 2024
@MilosPaunovic
Copy link
Member

As I am far away from being an expert for this Topology/VueFlow implementation, there is a question for someone from the BE team to guide me a bit:

Right now the response of the endpoint returning the data for graph looks like this:

[
    // ...
    {
        "uid": "root.for_each.log1",
        "type": "io.kestra.core.models.hierarchies.GraphTask",
        "error": false,
        "task": {
            "id": "log1",
            "type": "io.kestra.plugin.core.log.Log",
            "message": "{{ taskrun.value }}"
        },
        "relationType": "PARALLEL"
    },
    {
        "uid": "root.for_each.log2",
        "type": "io.kestra.core.models.hierarchies.GraphTask",
        "error": false,
        "task": {
            "id": "log2",
            "type": "io.kestra.plugin.core.log.Log",
            "message": "{{ taskrun.value }}"
        },
        "relationType": "PARALLEL"
    },
    {
        "uid": "root.for_each.log3",
        "type": "io.kestra.core.models.hierarchies.GraphTask",
        "error": false,
        "task": {
            "id": "log3",
            "type": "io.kestra.plugin.core.log.Log",
            "message": "{{ taskrun.value }}"
        },
        "relationType": "PARALLEL"
    }
    // ...
]

Notice the "relationType": "PARALLEL" for the 3 tasks from flow Anna posted in a comment above. That is why the rendered output is parallel. Is this by design and should I try to manually handle it differently on the UI side or is there a need to change something on BE side also?

Possibly @loicmathieu could be the one who knows the answer to that?

@MilosPaunovic MilosPaunovic moved this from Backlog to In progress in Issues Jul 30, 2024
@loicmathieu
Copy link
Member

Yes this is a backend issue not a frontend issue.
The topology is driven by a graph computed by the backend

@loicmathieu loicmathieu added area/backend Needs backend code changes and removed area/frontend Needs frontend code changes labels Jul 30, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Issues Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/backend Needs backend code changes bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants