job's complete event triggered after processing of next job already began

[pkyeck]

I'm trying to build jobs that depend on another job successfully finishing before running but ran into the problem that when these events happen at the "same time", the first job is not yet marked as completed when the new job gets processed.

example output:

job saved type: 'start'
job saved type: 'end'
job enqueued 138
job enqueued 139
processing ...  type: 'start'
done processing 'start' 138
processing ...  type: 'end'
job complete 138
done processing 'end' 139
job complete 139

so when I check the completed jobs at the beginning of queue.process() the start job is not yet returned 😞

anyone else having this problem?

[dindurthy]

Good timing! We just encountered a flavor of this and I think we've figured this out. The most relevant code is in worker.js:

      job.complete(function() {
        job.attempt(function() {
          if( job.removeOnComplete() ) {
            job.remove();
          }
          self.emitJobEvent('complete', job, result);
        });
      }.bind(this));
      self.start(fn);

In the createDoneCallback of processing, we mark the job complete and then emit the event. This is async. So, by the time event is fired, the next job could be in progress. We ran into this because we listen for job complete and remove the jobs, but this may not work for an active job when a pause/shutdown is applied. When we pause a worker, it will 1) complete its job, 2) call self.start, 3) emit the idle event, and 4) emit the job complete. Elsewhere, the shutdown listener will hear the idle event first, mark the worker as cleaned up, and will fail to emit the job complete.

So, maybe we should put the start into the complete/attempt callback, or emit the complete outside, or something else. Haven't thought about it too deeply.

You'll be happy to know the processing of your jobs is happening in order, even if the completion events come a little late.

[tobalsgithub]

My vote would be to put self.start inside the callback to job.attempt. Don't start the next job until the current one is totally finished. Too many race conditions to consider for things like graceful shutdown and such.

      job.complete(function() {
        job.attempt(function() {
          if( job.removeOnComplete() ) {
            job.remove();
          }
          self.emitJobEvent('complete', job, result);
          self.start(fn);
        });
      }.bind(this));

[behrad]

You guys can create a tested PR ?

[dindurthy]

Happy, behrad. Happy to.

[pkyeck]

@dindurthy for me it's more about what @tobalsgithub said: I have to make sure that the previous job is completely done before starting the next one.

In my case I have 3 events that are creating jobs (new, end, relive) and when starting a new job, I want to be able to check if the job that this one is depending on, is already complete. So when processing of end job starts and I check if new is complete I sometimes get a wrong "answer".