task icon indicating copy to clipboard operation
task copied to clipboard

"probably an cyclic dep or infinite loop" on huge graphs

Open alex65536 opened this issue 3 years ago • 9 comments

If the graph is large, you may get the following error:

task: maximum task call exceeded (100) for task: probably an cyclic dep or infinite loop

The reasons are as follows: https://github.com/go-task/task/blob/bf9cd7625bdc9510a309b09a3b47b0fd4b23ac8e/task.go#L110-L112 So, if we try to execute the task >= 100 times, then we crash.

The issue is that RunTask is executed once from each of the dependencies: https://github.com/go-task/task/blob/bf9cd7625bdc9510a309b09a3b47b0fd4b23ac8e/task.go#L200-L206 but we increment the counter before checking if the task is already running or finished. Such check is done below, in startExecution: https://github.com/go-task/task/blob/bf9cd7625bdc9510a309b09a3b47b0fd4b23ac8e/task.go#L300-L327

So, if there is a task on which 100 other tasks depend, then the graph will fail with an error.

Moreover, if you read startExecution carefully, then it would appear that the error has nothing to do with cyclic deps. The simplified algorithm is as follows:

RunTask(task) {
  if (hashes.contains(task)) {
    // wait for task
    return;
  }
  hashes.insert(task);
  // iterate over deps
  // run commands
}

So, even for cyclic dependency, the task will be executed once (though, it will eventually stuck waiting for itself).

alex65536 avatar Jul 24 '22 11:07 alex65536

Running into this when running a task on ~150 files using the new for command option.

q0rban avatar Aug 03 '23 20:08 q0rban

Here's an example Taskfile.yml to replicate this:

version: 3

tasks:
  default:
    vars:
      ITEMS:
        sh: echo {1..101}
    cmds:
      - for: { var: ITEMS }
        task: process-item
        vars:
          ITEM: '{{.ITEM}}'

  process-item:
    cmds:
      - echo {{.ITEM}}

What would be ideal in my mind is for Task to count the number of items that is in the for loop and only throw an error if it is called more than that number.

In addition, a parameter on the task to configure this limit would be desirable:

version: 3

tasks:
  default:
    vars:
      ITEMS:
        sh: echo {1..101}
    cmds:
      - for: { var: ITEMS }
        task: process-item
        vars:
          ITEM: '{{.ITEM}}'

  process-item:
    max-call-count: 101
    cmds:
      - echo {{.ITEM}}

q0rban avatar Aug 04 '23 18:08 q0rban

What would be ideal in my mind is for Task to count the number of items that is in the for loop and only throw an error if it is called more than that number.

In addition, a parameter on the task to configure this limit would be desirable:

I think it's a very hacky workaround which doesn't solve the issue fully. My comment above states that it's good either:

  • to move this check below, so it will actually check for cyclic deps
  • to get rid of this check at all, because actually it doesn't check for cyclic dependencies at all, but prevents valid configurations from running

alex65536 avatar Aug 12 '23 15:08 alex65536

to get rid of this check at all, because actually it doesn't check for cyclic dependencies at all, but prevents valid configurations from running

That would be fine by me!

I did try to verify that adding a circular dependency triggers the error, which it does, though interestingly it catches it right away, before reaching 100 calls. I think this confirms what you're saying? That the "max call of 100" isn't actually a useful limit since the circular dependency is caught before ever reaching it?

task: Failed to run task "default": task: Maximum task call exceeded (0) for task "one": probably an cyclic dep or infinite loop

The salient bit: "Maximum task call exceeded (0)" instead of "Maximum task call exceeded (100)".

q0rban avatar Aug 12 '23 17:08 q0rban

Just doing some spring cleaning and a quick update on this issue:

Related issues/PRs

  • #1933
  • #1321
  • #1332
  • #1954

Update

As discussed in #1332 and committed in a70f5aafc23dcbbe25d6a05b138493add70e448a, we increased the limit from 100 to 1000 to try and mitigate this issue. Obviously, this is not a proper solution to the problem though and this number is still arbitrary and does not actually "detect" cycles.

In https://github.com/go-task/task/pull/1332#issuecomment-1719985409 I mentioned that we were moving to DAG for Taskfiles. This was merged in #1563. However, this did not add DAG to tasks themselves. DAGs (as per their name) do not allow cycles and so would give us proper cycle detection. This is still something that we would like to do in the future and so the fundamental issue remains for now.

I would be open to a change that implemented proper cycle detection as a temporary solutions. This is almost certainly quicker and easier to do than implementing a task DAG right now. However, increasing the limit above 1000 just feels like chasing our tail a bit (where do we stop - people will probably always hit the limit unless we raise it to something unacceptable).

pd93 avatar Nov 29 '24 18:11 pd93

This is almost certainly quicker and easier to do than implementing a task DAG right now. However, increasing the limit above 1000 just feels like chasing our tail a bit (where do we stop - people will probably always hit the limit unless we raise it to something unacceptable).

I think a better solution would be to remove this check at all rather than increasing the limit, because, as I have shown above, it doesn't do anything with cycles at all. It only prevents one task from having more than 1000 reverse dependencies.

alex65536 avatar Nov 30 '24 11:11 alex65536

Just wanted to weigh in. I have a particularly extreme use-case where I need to call a task thousands of times. This limit is preventing me from adopting this software for that particular project. Being able to turn off that limit would help me in particular a lot. If there's anything I can do to help this get over the line, I'd be happy to help.

And as for the particular implementation, my wild use-case has me spawning tens of thousands of tasks sometimes multiple levels deep. This check has never caught a mistake for me, but it has hindered me in several places from implementing things in the most ideal and straightforward way for my project.

korverdev avatar Jan 07 '25 00:01 korverdev

Hello, just adding my 2cents, I am also hitting this limitation, specially for "deps" as it provides out of the box parallel execution.

I get the idea but I would like to have a "I know what I am doing" toggle.

Maybe we could get a warning log when reaching the limit without stopping the process ?

NargiT avatar Feb 28 '25 10:02 NargiT

I created a fork that increases the hardcoded limit, maybe someone finds it useful as well.

holzingk avatar May 14 '25 08:05 holzingk

We really need the ability to set a manual limit. We generate a huge number of Helm charts through dependencies. Right now, we have to generate them in batches using a Taskfile on top of the current setup.

DjinNO avatar Sep 04 '25 07:09 DjinNO