feat(worker): add discardTtl option #1653
Conversation
src/classes/worker.ts
Outdated
| @@ -139,6 +139,8 @@ export interface WorkerListener< | |||
|
|
|||
| const RATE_LIMIT_ERROR = 'bullmq:rateLimitExceeded'; | |||
|
|
|||
| const DISCARD_TTL_ERROR = 'bullmq:discardTtlExceeded'; | |||
There was a problem hiding this comment.
I guess we will need to replace this constants by an enum soon.
src/classes/worker.ts
Outdated
| @@ -669,6 +670,12 @@ export class Worker< | |||
| lockExtender(); | |||
There was a problem hiding this comment.
ok, seems a bit counterintuitive that we need to extend the lock if we are going to fail the job, but I guess this is necessary to avoid the risk of not having a lock when handling the failed job, which requires a valid lock.
src/classes/worker.ts
Outdated
| @@ -669,6 +670,12 @@ export class Worker< | |||
| lockExtender(); | |||
|
|
|||
| try { | |||
| if (this.opts.discardTtl) { | |||
| if (this.opts.discardTtl < Date.now() - job.timestamp) { | |||
| return handleFailed(new Error(DISCARD_TTL_ERROR)); | |||
There was a problem hiding this comment.
Something hit me about this. Are we really sure we want to fail the job here? I am thinking that discarded jobs would probably just pollute the failed set where there could be legitimate jobs that failed for some reason that needs to be debugged. Maybe it is better to just remove the job while sending an event for it.
|
Hi team! Are there any plans to merge this functionality? |
There was a problem hiding this comment.
Unfortunatelly I think this functionality cannot be implemented after the job has been fetched from the queue, as it will affect rate limiting. So it should instead be moved to moveToActive, where it can be discarded before it affects the rate limiter. Also it will allow for discarding several jobs in one go, cannot be too many though or it will keep Redis busy for too long.
| @@ -649,7 +651,7 @@ export class Worker< | |||
|
|
|||
| if ( | |||
| err instanceof DelayedError || | |||
| err.name == 'DelayedError' || | |||
| err.message == 'DelayedError' || | |||
| err instanceof WaitingChildrenError || | |||
| err.name == 'WaitingChildrenError' | |||
There was a problem hiding this comment.
what about this err.name no need to also change to message ?
ref #301