sindresorhus / p-queue

Promise queue with concurrency control
MIT License
3.45k stars 185 forks source link

REST calls not triggered to run till entire queue added. #123

Closed davidkjackson54 closed 3 years ago

davidkjackson54 commented 3 years ago

I have concurrency set very high in order to test concurrent REST api calls where I issue 140 calls in rapid succession.. I should get a HTTP 500 back due to flooding the server but I am not. When I traced the code, it appears that the REST calls are only being made 1 at at time.


const queue = new PQueue({ concurrency: 1000 });
 (async () => {
    for (const copyFile of JSONObj.result) {
      log.debug(`1. Initial ${copyFile.includedComponent}`);
      let browseOption ="B";
      log.debug(`2. Before REST ${copyFile.includedComponent}`);
      (async () => {
        data = await queue.add(() =>
          ComponentBrowse.get({
            //componentBrowse({
            session: { session: session },
            parms: {
              component: copyFile.includedComponent,
              componentType: copyFile.includedComponentType,
              package: packageName,
              browseFromOption: browseOption,
            },
          })
        );
        log.debug(`4.Before Write  ${copyFile.includedComponent}`);
        filePath = directoryPath + "\\" + copyFile.includedComponent + dirObj.copySuffix;
        fs.writeFileSync(filePath, data);
        log.debug(`5. After Write  ${copyFile.includedComponent} with filepath ${filePath}`);
        log.debug(`Queue size: ${queue.size}`);
      })();
    }
  })();

```}
sindresorhus commented 3 years ago

You are using synchronous calls in the async IIFE: fs.writeFileSync(filePath, data);

We already have a concurrency test: https://github.com/sindresorhus/p-queue/blob/55d74e14c5ea6b77a26410089ea548a10d727bf9/test/test.ts#L55-L69

Submit a pull request with a failing test and I'll take a closer look.

davidkjackson54 commented 3 years ago

I spent some time debugging this and cleaned up the code. and added more debugging. I do a queue add in a loop containing 140 REST api calls. For each call a text file is returned which I then write to a file locally. I am finding that all the REST calls are being queued but the first REST call is not being resolved until after all 140 calls have been queued. I have tried with a concurrency value of 1 and 300 but it doesn't seem to make any difference. Should the REST calls be started to be issued immediately ?

The code is now much simplified: This example is with a concurrency value of just 1.

   const queue = new PQueue({ concurrency: 1 });

  JSONObj.result.map(async (copyFile) => {
    const component: string = copyFile.includedComponent;
    const componentType: string = copyFile.includedComponentType;
    const location: string = copyFile.whereFrom;

    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    log.debug(`1. Queue add ${component}`);

    (async () => {
      data = await queue.add(async () =>
         componentBrowse({
          session: { session: session },
          parms: {
            component: component,
            componentType: componentType,
            package: packageName,
          },
        })
      ); // end queue add

      // promise is resolved
      log.debug(`2. Resolved ${component}`);
      const filePath: string = directoryPath + "\\" + component + dirObj.copySuffix;
      fs.writeFile(filePath, data, (err) => {
        if (err) {
          vscode.window.showInformationMessage(err.message);
          return;
        } else {
          log.debug(`3. Saving local Copybook ${component}`);
        }
      });
      log.debug(`4. Queue size: ${queue.size} Queue pending: ${queue.pending}`);
    })(); // end async
  }); // end map

  await queue.onIdle();
  log.debug(`5. All done`);
  log.debug(`Downloaded all copybooks for ${packageName} ${component} ${componentType}`);
  vscode.window.showInformationMessage(`${JSONObj.result.length} copy files havce been retrieved`);

The debug shows:

[2020/12/27 15:16:32.822] [DEBUG] [retrieveCopybooks.js:57] Retrieved 140 file locations for Package STEV001571 Component ASMPGM33 Type SRC
[2020/12/27 15:16:32.824] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY001
[2020/12/27 15:16:32.827] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY002
[2020/12/27 15:16:32.828] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY003
[2020/12/27 15:16:32.829] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY004
[2020/12/27 15:16:32.830] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY005
[2020/12/27 15:16:32.831] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY006
[2020/12/27 15:16:32.832] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY007
[2020/12/27 15:16:32.832] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY008
[2020/12/27 15:16:32.833] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY009
[2020/12/27 15:16:32.834] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY010
.
.
.
[2020/12/27 15:16:32.953] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY138
[2020/12/27 15:16:32.954] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY139
[2020/12/27 15:16:32.954] [DEBUG] [retrieveCopybooks.js:83] 1. Queue add COPY140
[2020/12/27 15:16:33.242] [DEBUG] [retrieveCopybooks.js:97] 2. Resolved COPY001
[2020/12/27 15:16:33.243] [DEBUG] [retrieveCopybooks.js:108] 4. Queue size: 138 Queue pending: 1
[2020/12/27 15:16:33.245] [DEBUG] [retrieveCopybooks.js:105] 3. Saving local Copybook COPY001
[2020/12/27 15:16:33.517] [DEBUG] [retrieveCopybooks.js:97] 2. Resolved COPY002
[2020/12/27 15:16:33.518] [DEBUG] [retrieveCopybooks.js:108] 4. Queue size: 137 Queue pending: 1
[2020/12/27 15:16:33.520] [DEBUG] [retrieveCopybooks.js:105] 3. Saving local Copybook COPY002
[2020/12/27 15:16:33.809] [DEBUG] [retrieveCopybooks.js:97] 2. Resolved COPY003
[2020/12/27 15:16:33.810] [DEBUG] [retrieveCopybooks.js:108] 4. Queue size: 136 Queue pending: 1
[2020/12/27 15:16:33.813] [DEBUG] [retrieveCopybooks.js:105] 3. Saving local Copybook COPY003
[2020/12/27 15:16:34.072] [DEBUG] [retrieveCopybooks.js:97] 2. Resolved COPY004
[2020/12/27 15:16:34.073] [DEBUG] [retrieveCopybooks.js:108] 4. Queue size: 135 Queue pending: 1
[2020/12/27 15:16:34.075] [DEBUG] [retrieveCopybooks.js:105] 3. Saving local Copybook COPY004
.
.
.
.
[2020/12/27 15:17:14.014] [DEBUG] [retrieveCopybooks.js:97] 2. Resolved COPY139
[2020/12/27 15:17:14.015] [DEBUG] [retrieveCopybooks.js:108] 4. Queue size: 0 Queue pending: 1
[2020/12/27 15:17:14.017] [DEBUG] [retrieveCopybooks.js:105] 3. Saving local Copybook COPY139
[2020/12/27 15:17:14.292] [DEBUG] [retrieveCopybooks.js:97] 2. Resolved COPY140
[2020/12/27 15:17:14.293] [DEBUG] [retrieveCopybooks.js:108] 4. Queue size: 0 Queue pending: 0
[2020/12/27 15:17:14.294] [DEBUG] [retrieveCopybooks.js:130] 5. All done
[2020/12/27 15:17:14.295] [DEBUG] [retrieveCopybooks.js:131] Downloaded all copybooks for TEST123 ASMPGM33 SRC
[2020/12/27 15:17:14.297] [DEBUG] [retrieveCopybooks.js:105] 3. Saving local Copybook COPY140
davidkjackson54 commented 3 years ago

Any update on this?

sindresorhus commented 3 years ago

As commented above:

Submit a pull request with a failing test and I'll take a closer look.

I'm asking this as most issues like this as just mistakes in the user's code and not an issue with the package.

davidkjackson54 commented 3 years ago

I cannot give you a recreation as it is making REST api calls. Hence why I provided the simple code to see if there is an issue. It may well be a user error (mine) but if so some indication as to what might be the cause would be useful as I am mostly using the same coding style of your examples. If you think that many issues like this are user errors then maybe the supporting doc might be a contributing factor. Thanks anyway. I will look at an alternative solution.