feat: parallel test-dep-graph requests #4386
Conversation
Users have complained about how slow `snyk test --all-projects` can be. This allows more than one test request to be in-flight at a time.
| for (let thread = 0; thread < MAX_CONCURRENCY; thread++) { | ||
| threads.push(execRequest()); | ||
| } | ||
| await Promise.all(threads); |
There was a problem hiding this comment.
I am not very familiar with this, but does this handle "bubbling" up errors to the user if some of the requests fail?
There was a problem hiding this comment.
Yes - in this case, the first error thrown will bubble back up to the user. Any requests that are still in flight will continue to be processed at the Snyk Platform end, but the responses to those will be ignored.
There was a problem hiding this comment.
how about when --json is used? do we collect all that info and return it as we did when it was all synchronous?
There was a problem hiding this comment.
It should do - this is essentially just an order of operations change. Where before we were taking each request, executing the request then processing the response, then moving onto the next request, here we perform all of the requests and gather the responses, then process the responses one-by-one (in the same order as before). The actual data and how it is processed are unchanged.
There was a problem hiding this comment.
Confirmed this running locally, with a CLI built from master and a CLI built from this branch - both produce identical output on the java-goof project when running snyk test --all-projects --json.
|
To address a couple of comments that have been raised here and on Slack:
|
What does this PR do?
Allows up to five requests to the
/test-dep-graphendpoint to be run at a time, reducingsnyk testwall-clock run times.There should be no change to outputs or the running order of operations.
Where should the reviewer start?
There's only one changed file -
src/lib/snyk-test/run-test.ts.How should this be manually tested?
On a sample repository with multiple Open Source projects within it (ideally ones for which the
/test-dep-graphAPI call takes non-trivial amount of time to complete - nuget projects seem to be good for this), runsnyk test --all-projectsfor this version of the CLI and compare withsnyk test --all-projectsfor another version. The outputs should be identical, but this version should run significantly quicker - specifically in the phase where the spinner displays "Querying vulnerabilities database...".Any background context you want to provide?
We've had support requests (linked below) where customers are complaining about the amount of time taken to scan large monorepos using the
--all-projectsargument while using the Azure Pipelines Snyk task. While there is a separate focus on speeding up the underlying requests (which in the linked example are taking 30-90s each), this approach seemed to be a simple way to reduce the wall-clock duration of the overall task. I've picked 5 for the concurrency level arbitrarily - it can be adjusted as needed.What are the relevant tickets?
https://snyk.zendesk.com/agent/tickets/35462