Concurrency option seems meaningless in p-settle

Viewed 71

In p-map I can see how a concurrency option makes sense: you are passing a mapper function so your promises only get invoked when p-map chooses and at the given concurrency.

With p-settle however there is no mapper function therefore the promises are invoked on creation, so why does it take a concurrency parameter? I can't see how this is not just all promises getting invoked at once and then p-settle effectively looping through the outcomes.

// From the docs, but with added files and a concurrency option:
const files = [
    'a.txt',
    'b.txt', // Doesn't exist
    'd.txt',
    'e.txt',
    'f.txt',
    'g.txt',
].map(fileName => fs.readFile(fileName, 'utf8'));
// ...I believe all readFiles have now been invoked and are 
// in progress?

// The outcome will (eventually) be correct but the 
// concurrency param is meaningless because all promises 
// are running at once
console.log(await pSettle(files, { concurrency: 2 }));
1 Answers

Edit: Fixed in v4.0.0 (thanks Sindre!)


Original solution:

Looks like this is a known issue with a suggested remediation that the PR hasn't been completed for or merged.

In the meantime I wrote my own version of p-settle using p-limit and p-reflect that functions how I'd expect:

import pLimit from 'p-limit';
import * as pReflect from 'p-reflect';
import * as fs from 'fs';

const concurrency = 5;
const limit = pLimit(concurrency);

const files = ['a.txt', 'b.txt', 'd.txt', 'e.txt', 'f.txt', 'g.txt'];

const result = await Promise.all(
  files.map((fileName) => pReflect(limit(() => fs.readFile(fileName, 'utf8')))),
);

Note, if using Node v12.9.0 or above you can remove the need for p-reflect here (which ensures promises fulfil no matter what) by using Promise.allSettled.

Related