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

addBulk does not work with repeatable jobs #1731

Closed
jtassin opened this issue May 11, 2020 · 7 comments
Closed

addBulk does not work with repeatable jobs #1731

jtassin opened this issue May 11, 2020 · 7 comments

Comments

@jtassin
Copy link

jtassin commented May 11, 2020

Description

addBulk function does not work properly for repeatable jobs, the nextRepeatableJob is not invoked for repeatable jobs.

Minimal, Working Test code to reproduce the issue.

(An easy to reproduce test case will dramatically decrease the resolution time.)

The following code :

imgQueue.addBulk([{ data: { img }, opts: {repeatable: { every: 5_000 }}}])

is different than :

imgQueue.add({ img }, {repeatable: { every: 5_000 }})

In the add code we have this behaviour once the job is created :

if (opts.repeat) {
  return this.isReady().then(() => {
    return this.nextRepeatableJob(name, data, opts, true);
  });
} 

It is missing in the addBulk function.

Solutions

I'm OK to fix it. Here are two possible solutions :

  • forbid addBulk for repeatable jobs it could sounds weird to bulk add repeatable jobs, but it is kind of breaking
  • call nextRepeatableJob for each repeatable jobs of the batch once created. Bad for perfs but still better than multiple adds

which one do you prefer ?

Bull version

3.14

Additional information

Thx for your lib :)

@DmitryOlkhovoi
Copy link

DmitryOlkhovoi commented Jun 4, 2020

Looks like similar bug #1761
Does .add work for you?

@epechuzal
Copy link

I also want to add that addBulk does not work with jobs that repeat with a cron expression, e.g.

const jobs = [
  { name: 'cron_job_hourly', opts: { repeat: { cron: '0 * * * *' } } },
  { name: 'cron_job_daily', opts: { repeat: { cron: '0 0 * * *' } } },
]
queue.addBulk(jobs);

If you have code like this, then two unexpected issues will happen:

  • the jobs will execute immediately rather than wait until the designated cron time
  • the jobs will also never repeat, as mentioned by @jtassin above

Solution would be to either add support for bulk adding repeatable jobs to a queue, or disable the feature entirely for repeatables to avoid confusion.

@manast
Copy link
Member

manast commented Jun 4, 2020

addBulk does not support repeatable jobs, and I do not think it is worth implementing. The idea of addBulk is to remove overhead when adding huge amounts of jobs, it does not make so much sense for repeatable jobs which usually are not so many.

@epechuzal
Copy link

Makes sense to me - might be worth adding some kind of type check to stop jobs being added like this, or at least adding to declaration or documentation that it is not supported

@jtassin
Copy link
Author

jtassin commented Jun 8, 2020

Makes sense to me - might be worth adding some kind of type check to stop jobs being added like this, or at least adding to declaration or documentation that it is not supported

+1 for type check and documentation

Looks like similar bug #1761
Does .add work for you?

Yes, .add works perfectly in my case

@manast
Copy link
Member

manast commented May 17, 2021

fixed with taskforcesh/bullmq#544

@manast manast closed this as completed May 17, 2021
Cellule added a commit to Cellule/DefinitelyTyped that referenced this issue Dec 9, 2021
I innocently tried to use addBulk to add a bunch of jobs with repeat options and was confused why it didn't work.
Turns out it's simply not supported as concluded in OptimalBits/bull#1731
Hopefully this little update to the typings avoids someone else hitting the same issue I did
@ebrahimmfadae
Copy link

addBulk does not support repeatable jobs, and I do not think it is worth implementing. The idea of addBulk is to remove overhead when adding huge amounts of jobs, it does not make so much sense for repeatable jobs which usually are not so many.

It is not just about performance, sometimes we need atomicity.

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.

5 participants