-
-
Notifications
You must be signed in to change notification settings - Fork 12
Open
Description
It looks like we also can let go of `fastparallel`. It is currently only used in: https://github.com/moscajs/aedes-cached-persistence/blob/47bcbdc98b36598ae5c87f0e0e56c7c795bb0864/index.js#L162-L167
Using callbacks:
if (!subs || subs.length === 0) {
process.nextTick(cb)
return
}
let completed = 0
let errored = false
const total = subs.length
const context = {
persistence: this,
packet
}
function done (err) {
if (err && !errored) {
errored = true
cb(err)
return
}
completed++
if (completed === total && !errored) {
cb()
}
}
for (let i = 0; i < total; i++) {
outgoingEnqueue.call(context, subs[i], done)
}
}
or using Promise.all:
outgoingEnqueueCombi (subs, packet, cb) {
// Create an array of promises for each subscription
const promises = subs.map(sub => {
return new Promise((resolve, reject) => {
this.outgoingEnqueue(sub, packet, (err) => {
if (err) reject(err);
else resolve();
});
});
});
// Use Promise.all to execute them in parallel and only report the first failure if it occurs
Promise.all(promises)
.then(() => cb(null))
.catch(err => cb(err));
}
I am however not sure whether either solution captures the full behaviour of fastparallel.
The Promise.all looks more clean to me but performs an extra loop to create the list of promises. This might have performance impact if subs is large, but that seems unlikely to me.
So the question is: can we let go of the dependency on fastparallel and if so does it matter which of the variants above we use?
Kind regards,
Hans
Originally posted by @seriousme in #59 (comment)
Metadata
Metadata
Assignees
Labels
No labels