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

Add a check in the task runner to wait for dependents to finish #20

Merged
merged 8 commits into from
Dec 14, 2018

Conversation

i-like-robots
Copy link
Contributor

@i-like-robots i-like-robots commented Nov 23, 2018

What

This PR adds logic to improve the usage of the --concurrency option with a new --preserve-order flag.

Why

Running concurrent tasks is great, but not always. This is because tasks can be kicked off for packages which may depend on another package that has not yet finished running... e.g.:

  • There are two modules: A and B
  • A depends on B
  • Both modules have a build step
  • Running a build task with a concurrency of 2 would immediately kick off both builds
  • The task will fail because A cannot find B

How

This PR adds an additional queue to the semaphore concurrency pattern which ensures tasks can wait until any dependent packages finish running. There are a number of changes to aid this:

  1. I have refactored each action to return the package info along with the function to run the task. This was done in order to access the package name and dependencies.
  2. I have added an evented queue to the "run-parallel.js" module. This will maintain a list of packages with currently queued and still running tasks. The evented queue can be asked for a promise which resolves when the queue no longer contains any dependent packages.
  3. I have added an allDependencies property to the package wrapper class as this was duplicated in several places
  4. I have refactored the tests to use a shared helper in order to create mock package wrappers

I tested by running various tasks within the the x-dash repository.

Show

Usage:

# long
athloi run build --concurrency 5 --preserve-order

#short
athloi run build -C 5 -P

@i-like-robots i-like-robots added the feature Addition of new functionality label Nov 23, 2018
src/run-parallel.js Outdated Show resolved Hide resolved
Copy link
Member

@apaleslimghost apaleslimghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really nice!

Copy link
Member

@apaleslimghost apaleslimghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops that was meant to be ✅

@@ -93,18 +93,25 @@ athloi publish -- --access=public

### concurrency

A global concurrency option which can be used to execute multiple tasks in parallel. By default only one task will run at a time.
A global option which will execute up to the given number of tasks concurrently. By default one task will be run at a time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be an upper-limit to the number of tasks we can run concurrently? 🤔

@i-like-robots i-like-robots merged commit 7b3f234 into master Dec 14, 2018
@i-like-robots i-like-robots deleted the matth/depends-on branch December 14, 2018 13:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Addition of new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants