node-rate-limiter icon indicating copy to clipboard operation
node-rate-limiter copied to clipboard

Callbacks processed out of order?

Open kpruden opened this issue 6 years ago • 4 comments

It looks like maybe the callbacks provided to removeTokens can be called out of order. I'm not sure if ordering is an intended feature, but if not it'd be good call that out clearly in the docs.

var RateLimiter = require('limiter').RateLimiter;
var l = new RateLimiter(1, 10000);
var i = 0;
var f = () => {
  var j = i++;
  var d = new Date();
  l.removeTokens(1, () => console.log(`${j}: enqueued: ${d}; dequeued: ${new Date()}`));
}
for (var k = 0; k < 5; k++) setTimeout(() => f(), 1000*k);

Running this code several times shows arbitrary ordering:

> i = 0
> for (var k = 0; k < 5; k++) setTimeout(() => f(), 1000*k);
0: enqueued: Thu Feb 01 2018 12:41:18 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:41:18 GMT-0800 (PST)
2: enqueued: Thu Feb 01 2018 12:41:20 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:41:28 GMT-0800 (PST)
1: enqueued: Thu Feb 01 2018 12:41:19 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:41:38 GMT-0800 (PST)
3: enqueued: Thu Feb 01 2018 12:41:21 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:41:48 GMT-0800 (PST)
4: enqueued: Thu Feb 01 2018 12:41:22 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:41:58 GMT-0800 (PST)

> i = 0
> for (var k = 0; k < 5; k++) setTimeout(() => f(), 1000*k);
0: enqueued: Thu Feb 01 2018 12:45:00 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:45:00 GMT-0800 (PST)
1: enqueued: Thu Feb 01 2018 12:45:01 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:45:10 GMT-0800 (PST)
3: enqueued: Thu Feb 01 2018 12:45:03 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:45:20 GMT-0800 (PST)
4: enqueued: Thu Feb 01 2018 12:45:04 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:45:30 GMT-0800 (PST)
2: enqueued: Thu Feb 01 2018 12:45:02 GMT-0800 (PST); dequeued: Thu Feb 01 2018 12:45:40 GMT-0800 (PST)

I can work around this limitation so this isn't an urgent issue for me. However if I'm misunderstanding something any pointers would be helpful. I have a slight suspicion that maybe setTimeout doesn't quite work the way I think it does :)

kpruden avatar Feb 01 '18 20:02 kpruden

Wondering if there is any solution for this problem? I havn't thought out workaround in my scenario ...

thinkhy avatar Mar 11 '18 15:03 thinkhy

Thank you for calling this out. This is due to using setTimeout to queue pending callbacks on the event loop rather than maintaining an explicitly ordered queue. Several timeouts will expire as soon as enough tokens are available and there is a race to see which one claims the token(s) first, while the rest set new timeouts.

If someone wants to take a stab at refactoring this I would be happy to review / provide feedback / merge.

jhurliman avatar Apr 17 '18 05:04 jhurliman

@kpruden @thinkhy @jhurliman If you still looking for solution, rate-limiter-flexible package supports FIFO queue rate limiter. There is also migration guide, if you wish to migrate from limiter.

animir avatar Jul 14 '19 05:07 animir

I'm not in love with this solution, but I was able to order by calls by ignoring the execution order of limiter and manually increment an index. I thought I'd post it here in case somebody else finds this approach useful.

/**
 * This performs concurrent operations using the provided limiter. This function ensures the array
 * is processed in a FIFO order, which is not a feature of the limiter library.
 */
async function mapWithRateLimiter(array, limiter, callback) {

  // We can't rely on the call order with limiter, so we have to manually increment the index.
  let result = [];
  let index = 0;

  // Run the limiter the necessary number of times.
  await Promise.all(_.times(array.length, async () => {
    await limiter.removeTokens(1);

    let currentIndex = index;
    index++;
    result[currentIndex] = await callback(array[currentIndex], currentIndex);
  }));

  return result;
}

LandonSchropp avatar Nov 07 '21 20:11 LandonSchropp