Automattic / kue

Kue is a priority job queue backed by redis, built for node.js.
http://automattic.github.io/kue
MIT License
9.45k stars 862 forks source link

Job which was updated executes earlier than established #1132

Open PaulMaly opened 6 years ago

PaulMaly commented 6 years ago

Hello guys! Thanks for kue, it's really cool thing!

I've got an interesting problem and I hope that you could help me understand and fix it. First of all, I create my job like this:

let data = new Date(/* target ISO date string*/),
     ttl = date.getTime() + 300000;
const j = queue.create('mytype', data)
            .backoff({
                type: 'exponential',
                delay: 0.5
            }).attempts(3)
            .removeOnComplete( true )
            .delay(data)
            .ttl(ttl)
            .priority('normal')
            .save();

And this part working correctly, but in some cases I need to update this job by ID. That's why I do this:

let data = new Date(/* target ISO date string*/),
     ttl = date.getTime() + 300000;
     kue.Job.get(jobId, (err, j) => {
            j.attempts(3)
            .delay(date)
            .ttl(ttl)
            .priority('normal')
            .save();
    });

Interesting point is, after that, job will be executed not in ther time. Jobs which wasn't updated will executing correctly.

So, I had the small investigation of the code and find out these few strange things:

In this fragment, we set delay property like the difference between target time point and a current time.

Job.prototype.delay = function( ms ) {
  if( 0 == arguments.length ) return this._delay;
  if( _.isDate(ms) ) {
    ms = parseInt(ms.getTime() - Date.now())
  }
  if( ms > 0 ) {
    this._delay = ms;
  }
  return this;
}

Ex: ms = 11 am, now = 10 am, delay = 1 h in milliseconds, right?

Further, when we update this job and call save() method inside of which calls update() method if id exists, we execute this fragment:

....
  if( this._delay ) {
    this.set('delay', this._delay);
    if( this.created_at ) {
      var timestamp   = parseInt(this.failed_at || this.created_at, 10)
        , delay       = parseInt(this._delay);
      this.promote_at = timestamp + delay;
      this.set('promote_at', this.promote_at);
    }
  }
....

And here we already use delay & created_at timestamp (10 am) to calculate promote_at timestamp. This mean, if job updated in 10:30 am, delay become only 30 min (target time 11 am), but promote_at will be set on 10 am + 30 min = 10:30 am. So, it mean that job will be triggered instantly, but not in their target time.

Could you please, explain is this wrong or right behavior, design intent or what?

One more thing, even when I try to create some workaround to hack this problem, like this:

j.set('created_at', Date.now());

i.e manually setting up created_at with a current timestamp before the save() method to fix promote_at calculation, but it doesn't help me.

This is very strange for me. Do you have any idea what's happening here? I'll be glad for any help!