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

Delayed job "Will run at" wrong source value #544

Closed
chriscarpenter12 opened this issue Mar 6, 2023 · 8 comments · Fixed by #545
Closed

Delayed job "Will run at" wrong source value #544

chriscarpenter12 opened this issue Mar 6, 2023 · 8 comments · Fixed by #545

Comments

@chriscarpenter12
Copy link

When a delayed job is added it displays a "Will run at" in the UI, but if the job uses changeDelay that value is never reflected in the UI leading to confusion as the job properties look correct.

Screenshot 2023-03-06 at 11 58 04 AM

I'm not completely sure, but I think here is where it's reading from job.opts, but instead should just use the delay property on the job itself.

Comparing two job properties that have/have not been updated.

Unchanged:

{
  ...
  opts: {
    attempts: 0,
    delay: 2429862,
    removeOnComplete: true,
    jobId: undefined,
    backoff: undefined
  },
  id: '20',
  progress: 0,
  returnvalue: null,
  stacktrace: null,
  attemptsMade: 0,
  delay: 2429862,
}

Job called with changeDelay

{
  ...
  opts: {
    attempts: 0,
    delay: 2519881,
    removeOnComplete: true,
    backoff: undefined
  },
  id: '19',
  progress: 0,
  returnvalue: null,
  stacktrace: [],
  attemptsMade: 0,
  delay: 3299877,
  repeatJobKey: undefined,
  timestamp: 1678119480121,
  parentKey: undefined,
  parent: undefined,
}
@felixmosh
Copy link
Owner

Hi @chriscarpenter12 thank you for reporting this issue, I'm not using changeDelay so I'm not familiar with it :]
Let me check that.

@hyperair
Copy link

hyperair commented Mar 7, 2023

Related issue in bullmq: taskforcesh/bullmq#1728

There isn't a good way to solve this:

  • job.timestamp + job.opts.delay doesn't take into account a changed delay (via job.changeDelay or job.moveToDelayed`
  • job.timestamp + job.delay yields an incorrect value, because:
    • job.delay is updated by job.changeDelay and job.moveToDelayed`; but
    • job.timestamp is job creation time and not updated by either of those calls
    • job's "will run at" time should be lastUpdated + job.delay, but we don't have lastUpdated

Internally, the "will run at" time can be obtained from the job's score in the delayed zset in redis, but there isn't a public API to get that

@felixmosh
Copy link
Owner

Released in v4.12.2

@felixmosh
Copy link
Owner

usage of changeDelay, well, changes the job's delay attribute, for some reason we used the option value instead... :]

@hyperair
Copy link

hyperair commented Mar 7, 2023

@felixmosh You'll still get wrong values, just in differently now:

If a job.changeDelay() happens with a small value a long time after the job was processed:

startTS = Date.now();
job = await queue.add(..., { delay: 16 * 60 * 1000 );

// ...after 15 minutes
await job.changeDelay(60 * 1000);

job = await queue.getJob(job.id);
job.timestamp + job.delay == startTs + 60 * 1000;  // this is wrong, it should be startTS + 16 minutes

@felixmosh
Copy link
Owner

@hyperair you are right, but seems that it should be solved on the BullMQ side, not on this board.
When it will be solved by your issue, it will be reflected on the board, currently the board reflects the common use properly.

@wenq1
Copy link

wenq1 commented Mar 8, 2023

see my comment in the bullmq issue

@lindesvard
Copy link

lindesvard commented Jul 15, 2024

Since I really need this feature I patched the api to get the correct delayed value. Feels like the issue at bullmq is inactive.

Here is the patch I use if anyone needs it

diff --git a/dist/src/queueAdapters/bullMQ.js b/dist/src/queueAdapters/bullMQ.js
index 3e5a2e61c9600459678fda0e29397faee7db9a1e..6cf94ac4384d86d99f292c8f296d4b95ab8f2d19 100644
--- a/dist/src/queueAdapters/bullMQ.js
+++ b/dist/src/queueAdapters/bullMQ.js
@@ -21,11 +21,31 @@ class BullMQAdapter extends base_1.BaseAdapter {
     addJob(name, data, options) {
         return this.queue.add(name, data, options);
     }
-    getJob(id) {
-        return this.queue.getJob(id);
+    getDelayedScoreName() {
+        return `${this.queue.opts.prefix}:${this.queue.name}:delayed`;
+    }
+    transformJob(job, actualDelay) {
+        job.opts.delay = actualDelay ? actualDelay - job.timestamp : job.delay
+        return job
+    }
+    async getJob(id) {
+        const client = await this.queue.client
+        const job = await this.queue.getJob(id);
+        if(job) {
+            const score = await client.zscore(this.getDelayedScoreName(), id)
+            return this.transformJob(job, score ? score / 0x1000 : null)
+        }
+        return undefined
     }
-    getJobs(jobStatuses, start, end) {
-        return this.queue.getJobs(jobStatuses, start, end);
+    async getJobs(jobStatuses, start, end) {
+        const jobs = await this.queue.getJobs(jobStatuses, start, end);
+        if(jobs.length === 0) {
+            return []
+        }
+        const client = await this.queue.client
+        const scores = await client.zmscore(this.getDelayedScoreName(), jobs.map(job => job.id))
+        const delays = jobs.map((_, i) => scores[i] !== null ? scores[i] / 0x1000 : null)
+        return jobs.map((job, i) => this.transformJob(job, delays[i]))
     }
     getJobCounts() {
         return this.queue.getJobCounts();

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