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.

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

feat: set default max concurrency to 100 for web-api client #1583

Merged
merged 1 commit into from Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/_packages/web_api.md
Expand Up @@ -642,8 +642,8 @@ Each of the API method calls the client starts are happening **concurrently**, o
to perform a lot of method calls, let's say 100 of them, at the same time, each one of them would be competing for the
same network resources (such as bandwidth). By competing, they might negatively affect the performance of all the rest,
and therefore negatively affect the performance of your app. This is one of the reasons why the `WebClient` limits the
**concurrency** of requests by default to ten, which means it keeps track of how many requests are waiting, and only
starts an eleventh request when one of them completes. The exact number of requests the client allows at the same time
**concurrency** of requests by default to one hundred, which means it keeps track of how many requests are waiting, and only
starts an additional request when one of them completes. The exact number of requests the client allows at the same time
can be set using the `maxRequestConcurrency` option.

```javascript
Expand Down
18 changes: 8 additions & 10 deletions packages/web-api/src/WebClient.spec.js
Expand Up @@ -1040,25 +1040,23 @@ describe('WebClient', function () {
});
});

it('should have a default conncurrency of 3', function () {
it('should have a default conncurrency of 100', function () {
const client = new WebClient(token);
const requests = [
callumsteele4 marked this conversation as resolved.
Show resolved Hide resolved
client.apiCall('1'),
client.apiCall('2'),
client.apiCall('3'),
client.apiCall('4'),
];
const requests = [];
for (let i = 0; i < 101; i++) {
requests.push(client.apiCall(`${i}`));
}
return Promise.all(requests)
.then((responses) => {
// verify all responses are present
assert.lengthOf(responses, 4);
assert.lengthOf(responses, 101);

// verify that maxRequestConcurrency requests were all sent concurrently
const concurrentResponses = responses.slice(0, 3); // the first 3 responses
const concurrentResponses = responses.slice(0, 100); // the first 100 responses
concurrentResponses.forEach((r) => assert.isBelow(r.diff, responseDelay));

// verify that any requests after maxRequestConcurrency were delayed by the responseDelay
const queuedResponses = responses.slice(3);
const queuedResponses = responses.slice(100);
const minDiff = concurrentResponses[concurrentResponses.length - 1].diff + responseDelay;
queuedResponses.forEach((r) => assert.isAtLeast(r.diff, minDiff));
});
Expand Down
2 changes: 1 addition & 1 deletion packages/web-api/src/WebClient.ts
Expand Up @@ -163,7 +163,7 @@ export class WebClient extends Methods {
slackApiUrl = 'https://slack.com/api/',
logger = undefined,
logLevel = undefined,
maxRequestConcurrency = 3,
maxRequestConcurrency = 100,
retryConfig = tenRetriesInAboutThirtyMinutes,
agent = undefined,
tls = undefined,
Expand Down