OptimalBits / bull

Premium Queue package for handling distributed jobs and messages in NodeJS.
Other
15.55k stars 1.43k forks source link

Job.Discard() don't work properly #832

Open rodrigoords opened 6 years ago

rodrigoords commented 6 years ago

Description

Queue with a job that need to search for a near by drivers in some attempts, if user wants to cancel this search i need to remove job from queue. When i tried to job.remove() i got a error because job is lock as said here: https://github.com/OptimalBits/bull/issues/575 So checking the documentation i found job.discard(): https://github.com/OptimalBits/bull/blob/master/REFERENCE.md#jobdiscard where "job is never ran again even if attemptsMade is less than job.attempts" but if use this method the job keep trying to execute inside queue.

How can i remove job execution from queue even if attemptsMade is less than job.attempts ?

Test code to reproduce

const DEFAULT_QUEUE_OPTIONS: Queue.JobOptions = {
  attempts: 90,
  backoff: {
    type: "fixed",
    delay: 3000
  },
  removeOnComplete: true,
  removeOnFail
}

export const cancelPendingTripRequestJob = (tripRequest: TripRequest): void => {
  const { id } = tripRequest;
  pendingTripRequestsQueue.getJob(id).then((job: any) => {
    if (!job) { return; }
      console.log("discarding job");
      job.discard();
  });
};

Bull version

"bull": "^3.3.7"

Additional information

application log:

2018-01-05T10:30:07.061589+00:00 app[web.1]: [pendingTripRequestsQueue] Failed Job: No drivers available 2018-01-05T10:30:10.073846+00:00 app[web.1]: [pendingTripRequestsQueue] Failed Job: No drivers available 2018-01-05T10:30:12.202610+00:00 app[web.1]: Trip cancelled 645168d0-f203-11e7-b733-addfcf0a2fab 2018-01-05T10:30:12.206370+00:00 app[web.1]: discarding job 2018-01-05T10:30:13.076323+00:00 app[web.1]: Processing job pendingTripRequestsQueue Fri Jan 05 2018 10:30:13 GMT+0000 (UTC) (Trip request 645168d0-f203-11e7-b733-addfcf0a2fab) 2018-01-05T10:30:13.130976+00:00 app[web.1]: [pendingTripRequestsQueue] Failed Job: No drivers available 2018-01-05T10:30:16.095617+00:00 app[web.1]: Processing job pendingTripRequestsQueue Fri Jan 05 2018 10:30:16 GMT+0000 (UTC) (Trip request 645168d0-f203-11e7-b733-addfcf0a2fab) 2018-01-05T10:30:16.105862+00:00 app[web.1]: [pendingTripRequestsQueue] Failed Job: No drivers available 2018-01-05T10:30:19.103893+00:00 app[web.1]: Processing job pendingTripRequestsQueue Fri Jan 05 2018 10:30:19 GMT+0000 (UTC) (Trip request 645168d0-f203-11e7-b733-addfcf0a2fab) 2018-01-05T10:30:19.109510+00:00 app[web.1]: [pendingTripRequestsQueue] Failed Job: No drivers available

TomKaltz commented 6 years ago

@manast is there a reason discard isn’t persisted to redid?

manast commented 6 years ago

@TomKaltz I don't know, actually. It is a loosy implementation, would not work in a concurrent scenario or anything. I will see if I can implement a proper discard and also a cancel functionality in the following days. I am in a busy period right now with limited time for bull...

rysi3k commented 6 years ago

Any news with this feature?

Igor-Lopes commented 5 years ago

Hello, are there any updates on this issue ?

shubham-bluestacks commented 3 years ago

Hi, any progress on this?

mrwillis commented 3 years ago

Any updates on this?

manast commented 3 years ago

@mrwillis out of curiosity, what are your expectations on "job.discard", i.e. what would you like it to do?

mrwillis commented 3 years ago

@manast When I do Job.discard, I don't want it to retry again. Currently I'm doing something like this:

 queue.on("failed", async (job, err) => {
    if (
      err instanceof GiveupJobError
    ) {
      await job.discard();
    }
  });

However, this doesn't seem to actually discard it, it still retries. I'm adding to the queue like this:

  await queue.add(payload.id, payload, {
      attempts: 5,
      backoff: 600000,
      removeOnComplete: 1000,
      removeOnFail: 1000,
    });

If this is another bug, happy to open another issue/set up a PR.

manast commented 3 years ago

@mrwillis ok. If you need the guarantee that the failed job is not retried again then in any case the call to discard should be performed inside the process function, like this:

queue.process((job) => {
  job.discard();
  throw new Error('unrecoverable error'));
});

However the way discard is currently implemented it will only work if you have 1 process. I will take some time to improve it though so it works in a distributed environment.

mrwillis commented 3 years ago

@manast okay thanks, so the trick is I need to job.discard() in the process function, not the error handler, even though the error handler has a reference to the job object?

manast commented 3 years ago

@manast okay thanks, so the trick is I need to job.discard() in the process function, not the error handler, even though the error handler has a reference to the job object?

Yes I mean, the failed event is asynchronous and may happen after it has been retried, but the risk of this happening depends of course on the retry delay. However and as mentioned earlier it wont work in either case since discard is not implemented correctly.