7

I've got a rate limiter for an API I am using which allows 20 requests per second. All requests are promise based and the promise will be resolved with the API data once there is a response.

The problem:

I setup a promiseArray which contains 58k promises all waiting for a response. So slowly the memory is increasing until I am running out of memory. In my specific situation I don't need to pass the resolved data to my then() and the data is eating up all my RAM.

The code:

  }).then(() => {
    // 2. Crawl for all clanprofiles from these leaderboards
    const promiseArray = []
    for (let i = 0; i < clanTags.length; i++) {
      // Resolved data from getClanProfile() is eating up all my RAM
      const p = backgroundScheduler.getClanProfile(clanTags[i], true)
      promiseArray.push(p)
    }
    return Promise.all(promiseArray)
  }).then(() => {

So is there a way to await until the promiseArray is resolved without needing the resolved data?

kentor
  • 16,553
  • 20
  • 86
  • 144
  • 2
    58,000 promises?! Sounds to me like you need to have a cut off point. If you get more than X requests, just abandon some of them. If you're getting that many connections you should consider some kind of load balancing and running your server in more than one node process. – Mike Cluck Oct 09 '17 at 20:14
  • 2
    I suspect this isn't possible with native Promises because they have no way of knowing that none of the attached callbacks will use the data (until the promise itself is GC'd). – Duncan Thacker Oct 09 '17 at 20:17
  • 3
    In order to avoid storing the response data, you could do a `promiseArray.push(p.then(()=>'success'))' That should drop the data that each promise is carrying and still return a usable promise. Should. I'm a little fuzzy there, hence the comment instead of an answer. – SethWhite Oct 09 '17 at 20:18
  • @Pac0 it doesn't take long to resolve these promises however the returned data is too huge for my RAM. Imagine one response has got around 100kb – kentor Oct 09 '17 at 20:18
  • 1
    Forgive me for being blunt, but the obvious solution seems to be: don't make 58,000 promises. Have some sort of wrapper or mechanism to dispatch Promises, but don't just make them all up front. Are they really that intertwined at the request level? Do you *need* 58,000? – zero298 Oct 09 '17 at 20:19
  • 1
    How about using something like void (await someAsyncFunction()); Check https://stackoverflow.com/questions/32384449/can-i-fire-and-forget-a-promise-in-nodejs-es7 – Akhil Oct 09 '17 at 20:19
  • 4
    If you don't need any results from the promises, why are you waiting for them? – Bergi Oct 09 '17 at 20:19
  • 5
    to add to @SethWhite s suggestion, i wouldnt even return success as this would result in 58k success strings being stored, just hit promise resolve `promiseArray.push(p.then(()=> Promise.resolve()))` – Johannes Merz Oct 09 '17 at 20:20
  • @Bergi fair question, they will be stored in my database if they haven't been stored before. – kentor Oct 09 '17 at 20:23
  • 1
    @kentor So you are using the data they return? – Mike Cluck Oct 09 '17 at 20:24
  • @MikeC Yes, but I pull them from the database (yes that's needed even though if it may sound weird) – kentor Oct 09 '17 at 20:25
  • @kentor Okay, now I'm confused. You are creating a bunch of promises, all of which are requesting some data. If that data does not yet exist in your database, you add it to the database. After doing your check and possibly adding it to the database, you don't need the data anymore. After all of these add-to-the-database-if-it-doesn't-exist promises go through, you need to perform more work. You can't get to this last stage because all of your memory is getting used up. Is that accurate? – Mike Cluck Oct 09 '17 at 20:28
  • @kentor Since you are using some kind of rate limiting anyway, do you need to create 58k of promises immediately at all? A sequential, recursive approach will consume a constant amount of memory. Can you show us the rate-limiting code, please? – Bergi Oct 09 '17 at 20:28
  • @Bergi I just did it that way because it was the easiest way (because I can just iterate over all `clanTags` inside of my then and add a promise for each one. A sequential, recursive approach is what I was looking for too, but I couldn't figure out a decent way to do this. – kentor Oct 09 '17 at 20:33
  • @kentor Just use straightforward recursion that iterates an array, then transform the code to promises by making each call asynchronous and chaining `then` to it. [Here's an example](https://stackoverflow.com/a/24660323/1048572). Alternatively, given you're on nodejs, just use [simple `await` syntax in a loop](https://stackoverflow.com/a/37576787/1048572) for sequential iteration. – Bergi Oct 09 '17 at 20:37

1 Answers1

8

You will use a lesser amount of memory if you don't ever have 58k promises, their associated async operations and their result data active at once.

Instead you want to run X operations at once and then when one finishes, you start the next one with never more than X in flight at the same time and never more than X promises in use at once.

You can experiment with an appropriate value of X. A value of 1 is sequential operations but you can often improve overall end-to-end operation time by using some higher value of X. If all requests are hitting the same host, then X is probably no more than 5-10 (since a given host can't really do a lots of things at once and asking it to do more than it can do at once just slows it down).

If every request is to a different host, then you may be able to make X higher. Experimentation would give you an optimal value for both peak memory usage and overall throughput and somewhat depends upon your specific circumstances.

Bluebird's Promise.map() has a concurrency option that will do this for you, but there are also numerous ways to code for only X in flight at the same time.

Here are some other coding examples of managing how many are in flight at a time:

Make several requests to an API that can only handle 20 request a minute

How to execute promises in series?

unable to complete promises due to out of memory

Fire off 1,000,000 requests 100 at a time

How to make it so that I can execute say 10 promises at a time in javascript to prevent rate limits on api calls?


If you don't need the resolved data, you can allow it to be GCed sooner by replacing it like this:

  const p = backgroundScheduler.getClanProfile(clanTags[i], true).then(data => {
      return 0;     // make resolved value just be a simple number
                    // so other data is now eligible for GC
  });
  promiseArray.push(p)    

And, here's a simple implementation that iterates an array with no more than X requests in flight at the same time:

// takes an array of items and a function that returns a promise
// runs no more than maxConcurrent requests at once
function mapConcurrent(items, maxConcurrent, fn) {
    let index = 0;
    let inFlightCntr = 0;
    let doneCntr = 0;
    let results = new Array(items.length);
    let stop = false;
    
    return new Promise(function(resolve, reject) {
        
        function runNext() {
            let i = index;
            ++inFlightCntr;
            fn(items[index], index++).then(function(val) {
                ++doneCntr;
                --inFlightCntr;
                results[i] = val;
                run();
            }, function(err) {
                // set flag so we don't launch any more requests
                stop = true;
                reject(err);
            });
        }
        
        function run() {
            // launch as many as we're allowed to
            while (!stop && inFlightCntr < maxConcurrent && index < items.length) {
                runNext();
            }
            // if all are done, then resolve parent promise with results
            if (doneCntr === items.length) {
                resolve(results);
            }
        }
        
        run();
    });
}
Kariem
  • 4,398
  • 3
  • 44
  • 73
jfriend00
  • 683,504
  • 96
  • 985
  • 979