Leaking Timeout In LocalQueueDriver

[MRobertEvers]

Describe the bug The LocalQueueDriver leaks setTimeout in a Promise.race.

To Reproduce Steps to reproduce the behavior:

1. Set continueWaitTimeout to a high number, 20+ seconds or so.
2. Make a load request to Cubejs
3. Call shutdown on the server.
4. Process runs until timeout ends.

Expected behavior Timeout should be canceled if nothing is awaiting it.

In LocalQueueDriver.js,

 async getResultBlocking(queryKey) {
    const resultListKey = this.resultListKey(queryKey);
    if (!this.queryDef[this.redisHash(queryKey)] && !this.resultPromises[resultListKey]) {
      return null;
    }

    const timeoutPromise = (timeout) => new Promise((resolve) => setTimeout(() => resolve(null), timeout));

    const res = await Promise.race([
      this.getResultPromise(resultListKey),

      // LEAKING TIMEOUT HERE

      timeoutPromise(this.continueWaitTimeout * 1000),
    ]);

    if (res) {
      delete this.resultPromises[resultListKey];
    }
    return res;

Example Config

    orchestratorOptions: {
      queryCacheOptions: {
        queueOptions: {
          continueWaitTimeout: 30,
        },
      },
    },

[github-actions[bot]]

If you are interested in working on this issue, please leave a comment below and we will be happy to assign the issue to you. If this is the first time you are contributing a Pull Request to Cube.js, please check our contribution guidelines. You can also post any questions while contributing in the #contributors channel in the Cube.js Slack.

[eugene1g]

This is still an issue.

One quick & safe way to resolve this is to tell NodeJS it's OK to quit the process even if this timer is still active, via .unref(). In https://github.com/cube-js/cube/blob/d3c06a7cd724a9cab5ba06d515e00b5e855d438d/packages/cubejs-query-orchestrator/src/orchestrator/LocalQueueDriver.js#L57-L62

The change would be to add .unref() to setTimeout like so -

 const timeoutPromise = (timeout) => new Promise((resolve) => setTimeout(() => resolve(null), timeout).unref()); 

Another (cleaner) way would be to cancel the timeout when the main promise is fulfilled, such as -

let timeoutRef = null;
const timeoutPromise = (timeout) => new Promise((resolve) => timeoutRef = setTimeout(() => resolve(null), timeout));
const res = await Promise.race([
    this.getResultPromise(resultListKey).then(result=>{ clearTimeout(timeoutRef); return result; }),
    timeoutPromise(this.continueWaitTimeout * 1000),
]);