Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Do_while task relevant loop over task calculation fix #3351

Merged
merged 10 commits into from Dec 7, 2022

Conversation

manan164
Copy link
Contributor

Pull Request type

  • Bugfix

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

Describe the new behavior from this PR, and why it's needed
Issue ##3271
There was error in calculating the relevant task for do_while task.

The cases where decision/ fork_join, dynamic_fork is part of loop over task then ideally do_while task must wait before all the task gets completed.
@manan164 manan164 changed the title Decision fix Do_while task relevant loop over task calculation fix Nov 17, 2022
@manan164
Copy link
Contributor Author

Hi @apanicker-nflx , One test is failing, I am working on a fix and also will add a few more tests.

The test case was when decision task is part of loop over task and decision task contains two tasks and only one task is scheduled. But there will not be any case where we schedule only one task given the decision case has two tasks.
@aravindanr aravindanr merged commit 21abd2f into Netflix:main Dec 7, 2022
ayushanand308 added a commit to ayushanand308/conductor that referenced this pull request Dec 27, 2022
Do_while task relevant loop over task calculation fix (Netflix#3351)
charybr pushed a commit to CiscoM31/conductor that referenced this pull request Mar 5, 2023
@charybr
Copy link
Contributor

charybr commented Mar 7, 2023

Hi @manan164, While testing this change with a existing DoWhile workflow found couple of issues:

  1. it stops after first iteration.
  2. When testing with 100 iteration loop, usually after 50 iterations, output variable "iteration" doesn't get inserted.

@manan164
Copy link
Contributor Author

manan164 commented Mar 7, 2023

Hi @charybr , thanks for reporting.
For

  1. Is possible for you to share the definition? May be the condition is in such a way that it executes only one iteration or some parsing error.
  2. Are you trying to put task output in external storage? The conductor has a limit on that. By default, it is 10MB.
    Let me know if this works, or we can chat here for more realtime collaboration.

@charybr
Copy link
Contributor

charybr commented Mar 7, 2023

Thank you, for responding.

Attached workflow we are using to test DoWhile, task is a simple sleep task.
DoWhileLoop.json.txt

We are using Dynomite cluster as persistence.

@manan164
Copy link
Contributor Author

manan164 commented Mar 7, 2023

Hi @charybr , I see the execution works fine.
Can you please try with below definition,

{
  "createTime": 1678218111006,
  "updateTime": 1678218236428,
  "name": "TestLoop20221208_63f649c1696f6e2d30e7228c",
  "version": 1,
  "tasks": [
    {
      "name": "serialLoop1",
      "taskReferenceName": "serialLoop1_0",
      "description": "Serial Loop",
      "inputParameters": {
        "iterCount": "100"
      },
      "type": "DO_WHILE",
      "loopCondition": "if ($.serialLoop1_0['iteration'] < $.iterCount ) { true; } else { false; }",
      "loopOver": [
        {
          "name": "InvokeSleepTask_1_63f649c0696f6e2d30e7224b",
          "taskReferenceName": "InvokeSleepTask1_0",
          "description": "Sleep Task",
          "inputParameters": {
            "duration": "2s"
          },
          "type": "WAIT"
        }
      ]
    }
  ],
  "inputParameters": [],
  "outputParameters": {},
  "schemaVersion": 2,
  "restartable": true,
  "workflowStatusListenerEnabled": false,
  "ownerEmail": "test@test.com",
  "timeoutPolicy": "ALERT_ONLY",
  "timeoutSeconds": 0,
  "variables": {},
  "inputTemplate": {}
}

For simplicity I have removed other task and input

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants