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

timeout still working? #136

Closed
wenq1 opened this issue Feb 12, 2020 · 29 comments
Closed

timeout still working? #136

wenq1 opened this issue Feb 12, 2020 · 29 comments
Labels
enhancement New feature or request PRO

Comments

@wenq1
Copy link

wenq1 commented Feb 12, 2020

In bull3, there's a job option (timeout)

In bullmq4, it is commented out in worker.ts:

 if (timeoutMs) {
        jobPromise = jobPromise.timeout(timeoutMs);
      }

is the feature still working?

@manast
Copy link
Contributor

manast commented Feb 12, 2020

good that you bring this up, actually is not implemented in BullMQ, because I was planing a better cancellation mechanism than the one we have in bull v3

@manast manast added the enhancement New feature or request label Feb 12, 2020
@wenq1
Copy link
Author

wenq1 commented Feb 13, 2020

Thanks for the update. Do you consider at least make bullmq feature compatible (not necessarily api-compatible) so that people could make a switch over? There's also a bug I would like to bring up. Is pull requests welcome at all?

@manast
Copy link
Contributor

manast commented Feb 13, 2020

BullMQ its already basically backwards compatible with Bull, the major difference is how real time events are handled, and also this cancellation thing that you pointed out. I am not aware of anything else, we also provide a v3 backwards compatibility layer, you can read some of this here (WIP): https://docs.bullmq.io/
And yes, PR always welcome of course :).

@wenq1
Copy link
Author

wenq1 commented Feb 13, 2020

I'll create an issue in another thread.

@Deadly0
Copy link

Deadly0 commented Jun 4, 2021

@manast any progress with timeout? If no, I will try to implement it and make a PR. Can you give me a recommendation or requirement about implementation of task cancellation?

@manast
Copy link
Contributor

manast commented Jun 7, 2021

@Deadly0 the main reason it is not implemented is because I have not figured out a nice way to implement cancelation. There is no native cancellation support for async functions in javascript, and none of the alternatives are very nice, but I am open to compromise if somebody can propose a working solution.

@adamreisnz
Copy link

This property is documented here but it still doesn't seem to work.

https://github.com/taskforcesh/bullmq/blob/master/docs/gitbook/api/bullmq.jobsoptions.timeout.md

What about the approach that was discussed in OptimalBits/bull#479 4 years ago?

@manast
Copy link
Contributor

manast commented Aug 3, 2021

@adamreisnz I think I can pull out something soon that is good enough.

@adamreisnz
Copy link

That'd be great, thanks

@dtboy1995
Copy link

mark

@manast manast added the PRO label Aug 26, 2021
@acro5piano
Copy link

I'm doing something like this:

new Worker('paint', async () => {
  await Promise.race([
    timeout(5000),
    doSomeAsyncTask()
  ])
})

async function timeout(ms: number) {
  return new Promise((resolve, reject) => {
    setTimeout(() => reject(), ms)
  })
}

@adamreisnz
Copy link

@manast so it seems this feature has been implemented, but it's only available in the paid/pro version, is that correct?

@adamreisnz
Copy link

@acro5piano that's an interesting solution. Note that if using Bluebird promises, you can also simply use Promise.timeout() as the second array item for Promise.race()

@manast
Copy link
Contributor

manast commented Jan 22, 2022

@adamreisnz yes, it is using observables which is probably the only correct way to cancel asynchronous calls in a generic way in Javascript, more documentation and examples coming in the following days: https://docs.bullmq.io/bullmq-pro/observables

@manast
Copy link
Contributor

manast commented Jan 22, 2022

@acro5piano it is important to point out that even though the job will "fail" if it times out, the "doSomeAsyncTask" will continue to work with potential side-effects as a result... if this is acceptable or not depends on what that function is doing.

@acro5piano
Copy link

@acro5piano it is important to point out that even though the job will "fail" if it times out, the "doSomeAsyncTask" will continue to work with potential side-effects as a result... if this is acceptable or not depends on what that function is doing.

Thanks, I didn't realize it!

@motiejunas
Copy link

So what is the plan with this? Why does this settings exists if it is not expected to work now? It took me hours to find out that this is just a dummy setting.

I thinktimeout prop should be either completely removed to reduce confusion or implemented in a way @acro5piano suggested (with a potential side-effect warning).

@manast
Copy link
Contributor

manast commented Feb 13, 2022

@motiejunas yes, we should take care of this ASAP. I think we will remove this setting entirely but maybe add another one that works in a different way, not as a timeout that kills the current job that is being processed but that fails it directly if the job is too old when it arrives at the worker. For a clean mechanism to actually timeout jobs I would refer to the Pro version, where we support observables and can therefore cleanly cancel a running job.

@autarchprinceps
Copy link

autarchprinceps commented Jun 9, 2022

I'm doing something like this:

new Worker('paint', async () => {
  await Promise.race([
    timeout(5000),
    doSomeAsyncTask()
  ])
})

async function timeout(ms: number) {
  return new Promise((resolve, reject) => {
    setTimeout(() => reject(), ms)
  })
}

This only implements a timeout inside a single run of the worker task. What I expected timeout to do and would need, is for the message to be removed from the queue, if it hasn't been (successfully) processed, including wait & retries, for a certain amount of time.
What you are describing sounds more like the stalled job feature to me. "When a worker is not able to notify the queue that it is still working on a given job, that job is moved back to the waiting list." Assuming retries are not exhausted, that is exactly what your timeout reject would do, at least from a use case perspective, right?

@acro5piano
Copy link

@autarchprinceps
Yeah, as other people mentioned, that implementation just tells BullMQ "the job failed" and the job is still running even if it was failing. However it should be better than nothing reported.

I strongly agree with @motiejunas as the current type definition of the option is confusing. It actually doesn't work but it has the option.

@roggervalf
Copy link
Collaborator

closed by #1284

@Rush
Copy link

Rush commented Oct 4, 2022

So let's say the workers are dead and then some code accumulates thousands of jobs in the queue. I thought of using timeout to prevent bullmq from processing some very old jobs that should be scrapped. What's the pattern to achieve this in bullmq?

@GeoffreyPlitt
Copy link

I've scanned through all of the comments and can't find a clear answer about whether BullMQ timeouts are working or not, is there a conclusion we can draw?

@isaachinman
Copy link

@manast Is the timeout option still relevant on jobs in v5? I see mentions of it in various issues and changelogs, but the TypeScript types themselves do not mention it anywhere.

I want to be able to kick off a job with an absolute timeout, such that if it enters an active state and does not succeed or fail in X minutes, it should be forcibly failed.

Have been searching for a half a day, would appreciate any guidance here.

@manast
Copy link
Contributor

manast commented Feb 26, 2024

There is no timeout anymore, since the Promise standard got rid of cancellation support. The timeout would then have given the impression that the job was cancelled when in fact it was still running. The options here are to implement the timeout yourself in the processor function of the worker, or use Observables from the Pro version... even using observables is still not a walk in the park but at least it is doable.

@isaachinman
Copy link

implement the timeout yourself in the processor function of the worker

What exactly do you mean by that? Like, a race condition setTimeout that rejects?

@manast
Copy link
Contributor

manast commented Feb 26, 2024

What exactly do you mean by that? Like, a race condition setTimeout that rejects?

For example, and that cleans up whatever it needs to clean up before rejecting.

@manast
Copy link
Contributor

manast commented Feb 26, 2024

As an example, this is what I am doing in bullmq-proxy: https://github.com/taskforcesh/bullmq-proxy/blob/main/src/controllers/http/worker-http-controller.ts#L32

@isaachinman
Copy link

Cool, thanks mate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request PRO
Projects
None yet
Development

No branches or pull requests