Search code examples
node.jsecmascript-6promisees6-promiseecmascript-2016

converting promiseAll to gradual promises resolve(every 3promises for example) does not work


I have a list of promises and currently I am using promiseAll to resolve them

Here is my code for now:

   const pageFutures = myQuery.pages.map(async (pageNumber: number) => {
      const urlObject: any = await this._service.getResultURL(searchRecord.details.id, authorization, pageNumber);
      if (!urlObject.url) {
        // throw error
      }
      const data = await rp.get({
        gzip: true,
        headers: {
          "Accept-Encoding": "gzip,deflate",
        },
        json: true,
        uri: `${urlObject.url}`,
      })

      const objects = data.objects.filter((object: any) => object.type === "observed-data" && object.created);
      return new Promise((resolve, reject) => {
        this._resultsDatastore.bulkInsert(
          databaseName,
          objects
        ).then(succ => {
          resolve(succ)
        }, err => {
          reject(err)
        })
      })
    })
    const all: any = await Promise.all(pageFutures).catch(e => {
       console.log(e)
     }) 

So as you see here I use promise all and it works:

const all: any = await Promise.all(pageFutures).catch(e => {
   console.log(e)
 }) 

However I noticed it affects the database performance wise so I decided to resolve every 3 of them at a time. for that I was thinking of different ways like cwait, async pool or wrting my own iterator but I get confused on how to do that?

For example when I use cwait:

let promiseQueue = new TaskQueue(Promise,3);
const all=new Promise.map(pageFutures, promiseQueue.wrap(()=>{}));

I do not know what to pass inside the wrap so I pass ()=>{} for now plus I get

Property 'map' does not exist on type 'PromiseConstructor'. 

So whatever way I can get it working(my own iterator or any library) I am ok with as far as I have a good understanding of it. I appreciate if anyone can shed light on that and help me to get out of this confusion?


Solution

  • First some remarks:

    • Indeed, in your current setup, the database may have to process several bulk inserts concurrently. But that concurrency is not caused by using Promise.all. Even if you had left out Promise.all from your code, it would still have that behaviour. That is because the promises were already created, and so the database requests will be executed any way.

    • Not related to your issue, but don't use the promise constructor antipattern: there is no need to create a promise with new Promise when you already have a promise in your hands: bulkInsert() returns a promise, so return that one.

    As your concern is about the database load, I would limit the work initiated by the pageFutures promises to the non-database aspects: they don't have to wait for eachother's resolution, so that code can stay like it was.

    Let those promises resolve with what you currently store in objects: the data you want to have inserted. Then concatenate all those arrays together to one big array, and feed that to one database bulkInsert() call.

    Here is how that could look:

    const pageFutures = myQuery.pages.map(async (pageNumber: number) => {
      const urlObject: any = await this._service.getResultURL(searchRecord.details.id, 
                                                             authorization, pageNumber);
      if (!urlObject.url) { // throw error }
      const data = await rp.get({
        gzip: true,
        headers: { "Accept-Encoding": "gzip,deflate" },
        json: true,
        uri: `${urlObject.url}`,
      });
      // Return here, don't access the database yet...
      return data.objects.filter((object: any) => object.type === "observed-data" 
                                               && object.created);
    });
    const all: any = await Promise.all(pageFutures).catch(e => {
      console.log(e);
      return []; // in case of error, still return an array
    }).flat(); // flatten it, so all data chunks are concatenated in one long array
    // Don't create a new Promise with `new`, only to wrap an other promise. 
    //   It is an antipattern. Use the promise returned by `bulkInsert`
    return this._resultsDatastore.bulkInsert(databaseName, objects);
    

    This uses .flat() which is rather new. In case you have no support for it, look at the alternatives provided on mdn.