Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

whenCurrentJobsFinished initializes bclient unnecessarily #1346

Closed
gabegorelick opened this issue Jun 11, 2019 · 0 comments · Fixed by #1347
Closed

whenCurrentJobsFinished initializes bclient unnecessarily #1346

gabegorelick opened this issue Jun 11, 2019 · 0 comments · Fixed by #1347

Comments

@gabegorelick
Copy link
Contributor

gabegorelick commented Jun 11, 2019

Description

Bull typically does a good job of lazily connecting to Redis. But the following code causes the blocking client to be initialized unnecessarily:

bull/lib/queue.js

Lines 1140 to 1147 in c0ee6be

Queue.prototype.whenCurrentJobsFinished = function() {
return new Promise((resolve, reject) => {
//
// Force reconnection of blocking connection to abort blocking redis call immediately.
//
const forcedReconnection = redisClientDisconnect(this.bclient).then(() => {
return this.bclient.connect();
});

If bclient hasn't been initialized yet, then we're connecting for no reason.

Minimal, Working Test code to reproduce the issue.

const Queue = require('bull');
const Redis = require('ioredis');

const queue = new Queue('foo', {
  createClient (type) {
    if (type === 'bclient') {
      // enforce that we don't waste connections for read-only queues
      throw new Error('This queue is read-only');
    }

    return new Redis();
  }
});
queue.whenCurrentJobsFinished();

Bull version

develop branch

Additional information

Also reproducible when calling queue.pause(true) since that calls whenCurrentJobsFinished internally. Workaround for that is to call queue.pause(true, true) so that whenCurrentJobsFinished is not called. But that only works if you're sure there are no jobs to wait for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant