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.

Sign up for GitHub

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

Jump to bottom

NX Parallel doesn't properly sequence the dependent packages #28599

Closed
1 of 4 tasks
brsanthu opened this issue Oct 23, 2024 · 3 comments · Fixed by #28669
Closed
1 of 4 tasks

NX Parallel doesn't properly sequence the dependent packages #28599

brsanthu opened this issue Oct 23, 2024 · 3 comments · Fixed by #28669
Assignees
@xiongemi
Labels
scope: core core nx functionality type: bug

Comments

@brsanthu
Copy link

brsanthu commented Oct 23, 2024 •
edited
Loading

Current Behavior

We have a monorepo with about 15 packages with inter-dependencies but no circular dependencies. When we run nx to process in parallel, it always fails as it tries to compile dependent package before compiling its dependency. Same command works when we run --parallel 1.

Expected Behavior

Nx should take dependency into consideration before processing dependent packages

GitHub Repo

No response

Steps to Reproduce

  1. Have a repo with multiple packages with dependencies
  2. Try to process with concurrency higher than 1. For ex., --parallel 5

Nx Report

❯ nx report

 NX   Report complete - copy this into the issue template

Node           : 20.10.0
OS             : darwin-arm64
Native Target  : aarch64-macos
yarn           : 4.4.1

nx (global)    : 20.0.5
nx             : 20.0.5
@nx/js         : 20.0.5
@nx/workspace  : 20.0.5
@nx/devkit     : 20.0.5
@nrwl/tao      : 19.8.4
typescript     : 5.5.4
---------------------------------------
The following packages should match the installed version of nx
  - @nrwl/[email protected]

To fix this, run `nx migrate [email protected]`

Failure Logs

This is with [nx run-many -t tsc --parallel 5]. Notice that @company/js-csv depends on @company/js-utils but js-utils is not processed first.

❯ yarn tsc

 NX   Running target tsc for 32 projects

   ✖  nx run @company/js-csv:tsc
      src/csvUtils.ts(3,32): error TS2307: Cannot find module '@company/js-utils' or its corresponding type declarations.
      src/models.ts(1,31): error TS2307: Cannot find module '@company/js-utils' or its corresponding type declarations.

This is with [nx run-many -t tsc --parallel 1]. Notice that js-utils is compiled first as that is base for all other packages.

❯ yarn tsc

   ✔  nx run @company/js-utils:tsc (2s)
   ✔  nx run @company/js-csv:tsc (2s)

Package Manager Version

No response

Operating System

  • Linux
  • macOS
  • Windows
  • Other (Please specify)

Additional Information

No response
@Coly010
Copy link
Contributor

Coly010 commented Oct 24, 2024 •
edited
Loading

@brsanthu Nx does take dependency order into account. If you're noticing that it is not happening, it is either a bug, or a configuration issue.

Is there any way you could provide a repo that reproduces this issue so that we can investigate it more thoroughly? It's hard to reproduce as we have a bias of experience in setting up an Nx workspace wherein the Project Graph would have the correct edges.

I also see that you're using yarn v4 which means this could be a Yarn Berry related issue 🤔

@phillipCouto
Copy link

phillipCouto commented Oct 24, 2024 •
edited
Loading

We are also experiencing a similar issue, we are only seeing this when we use affected or run-many. When we run the individual task it will execute the dependent tasks in the correct order. Also when we look at the graph we are seeing the tasks identified in the correct order.

This issue popped up when we went from nx 19.4.x to 19.5.x.

The graph is like so:

libs/backend/models:migrate-db -> libs/backend/models:prebuild -> libs/backend/requestcache:audit

The libs/backend/models package does have many other projects depend on it and it does execute correctly at the beginning but nx will incorrectly schedule a dependent task to run in parallel with libs/backend/models:prebuild. This scheduled task will fail but then the remaining dependent tasks will complete successfully.

I ran an additional test were I artificially added a long delay to libs/backend/models:prebuild and nx did wait correctly to schedule any new tasks that depend on it until libs/backend/models:prebuild completes but libs/backend/requestcache:audit still failed as it was schedule in parallel.

So far with my testing it seems nx is incorrectly scheduling dependent tasks if there is parallel capacity. Once libs/backend/models:prebuild is started then nx will wait.

I will follow-up with a repro of the issue tonight if no one else posts one but I wanted to document my observations thus far.

@phillipCouto
Copy link

So it seems the issue I was experiencing in 19.5.x was resolved in 19.6.x. Funny enough there is a similar issue that was introduced in 19.8.4 that is still present even in 20.0.5.

I have created a project with instructions on how to reproduce. This was done on a macbook but I also tested it on a fedora core VM and the behaviour is the same. Due to the commands I used to simulate the tasks it may not work on windows.

https://github.com/phillipCouto/nx-task-ordering-issue-19.8.4

@jaysoo jaysoo added scope: core core nx functionality and removed blocked: repro needed labels Oct 25, 2024
@xiongemi xiongemi mentioned this issue Oct 25, 2024
Closed
4 tasks
@murphomatic murphomatic mentioned this issue Oct 26, 2024
Open
@xiongemi xiongemi mentioned this issue Oct 29, 2024
Merged
xiongemi added a commit that referenced this issue Nov 1, 2024
@xiongemi
fix(core): fix dependency with multiple dependent packages (#28669)
9adb9e0 
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
jaysoo pushed a commit that referenced this issue Nov 1, 2024
@xiongemi @jaysoo
fix(core): fix dependency with multiple dependent packages (#28669)
d03134d 
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
jaysoo pushed a commit that referenced this issue Nov 1, 2024
@xiongemi @jaysoo
fix(core): fix dependency with multiple dependent packages (#28669)
c1e77a1 
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
jaysoo pushed a commit that referenced this issue Nov 1, 2024
@xiongemi @jaysoo
fix(core): fix dependency with multiple dependent packages (#28669)
f405544 
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

<!-- This is the behavior we have today -->
- we introduce a feature to allow circular project dependencies to
execute tasks in pr https://github.com/nrwl/nx/pull/28227/files
- for this feature, we introduce the concept of dummy task as a
temporary placeholder so we can still generate a task graph even if
dependencies contain circular dependencies
- however, it does not handle a task graph of multiple dummy task deps.
for example:
lib1 -> lib2 -> lib3 -> lib4
if we got task graph like:
lib1:build -> lib2:dummy
lib2:dummy -> lib3:dummy
lib3:dummy -> lib4:rebuild
currently, it does not create a dependency between
lib1:build->lib4:prebuild

<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xiongemi xiongemi

Labels
scope: core core nx functionality type: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(core): fix dependency with multiple dependent packages nrwl/nx
5 participants
@jaysoo @phillipCouto @brsanthu @Coly010 @xiongemi