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

[Bug]: immediately on upsertJobScheduler kicks in multiple times with pattern. #2860

Closed
1 task done
vorillaz opened this issue Oct 23, 2024 · 7 comments · Fixed by #2891
Closed
1 task done

[Bug]: immediately on upsertJobScheduler kicks in multiple times with pattern. #2860

vorillaz opened this issue Oct 23, 2024 · 7 comments · Fixed by #2891
Labels
bug Something isn't working

Comments

@vorillaz
Copy link

Version

5.21.2

Platform

NodeJS

What happened?

According to the docs the immediately option on upsertJobScheduler kicks in the worker instantly once registered, but once using a cron pattern along with immediately the worker starts processing multiple times in a row, from 20 to 30 times during my tests. As an additional information, according to my tests limit option actually the kick in.

How to reproduce.

Take the docs sample along with a test worker:

const worker = async () => {
  console.log('workkkkkkks again');
};
await myQueue.upsertJobScheduler(
  "end-soon-job",
  {
    every: 60000, // every minute
    immediately: true,
  },
  {
    name: "timed-end-job",
    opts: {
      attempts: 0,
      removeOnComplete: 5,
      removeOnFail: 20,
    },
  }
);

Once changing the every property to cron pattern the worker kicks in multiple times:

await myQueue.upsertJobScheduler(
  "end-soon-job",
  {
    pattern: '* * * * *', // every minute in cron
    immediately: true,
  },
  {
    name: "timed-end-job",
    opts: {
      attempts: 0,
      removeOnComplete: 5,
      removeOnFail: 20,
    },
  }
);

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@vorillaz vorillaz added the bug Something isn't working label Oct 23, 2024
@manast
Copy link
Contributor

manast commented Oct 23, 2024

immediately should be ignored for cron expressions, as it really does not make a lot of sense in that case. Does it work properly without it?

@vorillaz
Copy link
Author

Hey @manast
It makes sense to use immediately for recurring jobs like data rollups that they are not so time sensitive.

immediately works as expected along with every, triggered once before the scheduled call. Upon using a corn pattern though the worker kicks in multiple times in a row.

And that’s a regression bug in my honest opinion cause it indicates something is not properly calculated, time wise.

@manast
Copy link
Contributor

manast commented Oct 23, 2024

In fact we are planing to deprecate "immediately" in the next major version of BullMQ. "every" will work as it had immediately to true all the time, as that is what users expects of every. However as cron specifies specific points in time, having an option as immediately does not make a lot of sense. If you really need it, it would be possible to implement it as a custom strategy, but it is also as simple as adding an initial job when you upsert your cron expression for the first time.

@vincentrolfs
Copy link

Dear manast, I am facing the same situation as vorillaz. The immediately option is very useful combined with pattern (cron), and it used to work prior to version 5.19. It makes a lot of sense to have scripts running regularly (say every hour) but additionaly at startup time. For example, after a server outage, I do not want to wait for the full hour to pass for my cleanup script to trigger - I want to run it immediately at startup.

We will add the necessary job manually now, but I would ask you to reconsider if this functionality might be useful for more people.

Also, I just want to point out that 5.19 introduced a breaking change which had quite grave consequences. Combining immediately with pattern suddenly causes the job to be triggered over and over again, in some cases more than once per second. If you have a pattern that specifies "once per day" it is quite jarring to see the job being executed every second suddenly.

This is a serious bug that should be fixed now, in my opinion - at least an error should be thrown when combining immediately with pattern. The current approach could have serious consequences.

@hungnkb
Copy link

hungnkb commented Oct 30, 2024

I have an issued that Job repeated every 1s instead of what i set before. Here is my implement:
"bullmq": "^5.21.2"

scheduleJobQueue.upsertJobScheduler(JobConstants.CLEAR_MANY_TO_MANY, {
      every: 10000,
      immediately: false,
    });
new Worker(
  JobConstants.CLEAR_MANY_TO_MANY,
  async job => {
    console.log('Clearing many-to-many table...', job.id);
    return;
  },
  { connection: bullmq.connection, concurrency: 1 },
);

These are logs:

Clearing many-to-many table... repeat:ccd85b75dc0d9a9906f94f47fe6f7ee9:1730272882420
Clearing many-to-many table... repeat:ccd85b75dc0d9a9906f94f47fe6f7ee9:1730272883420
Clearing many-to-many table... repeat:ccd85b75dc0d9a9906f94f47fe6f7ee9:1730272884420
Clearing many-to-many table... repeat:ccd85b75dc0d9a9906f94f47fe6f7ee9:1730272885420

@manast
Copy link
Contributor

manast commented Nov 8, 2024

Combining immediately with pattern suddenly causes the job to be triggered over and over again, in some cases more than once per second. If you have a pattern that specifies "once per day" it is quite jarring to see the job being executed every second suddenly.

This is a serious bug that should be fixed now, in my opinion - at least an error should be thrown when combining immediately with pattern. The current approach could have serious consequences.

Thanks for highlighting this issue, we will look into it asap.

@manast
Copy link
Contributor

manast commented Nov 10, 2024

For the record, we may keep immediately for pattern, if this is something valuable although it is very easy to mimic by just adding a standard job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants