search

samsondav / rihanna

Rihanna is a high performance postgres-backed job queue for Elixir
MIT License
439 stars 49 forks source link

Retry jobs even when they raise/exit #96

Open jonnystoten opened 4 years ago

jonnystoten commented 4 years ago

Fixes #95.

samsondav commented 4 years ago

I am not sure if this was a bug, or intended behaviour.

@lpil What do you think?

tpitale commented 4 years ago

I thought this was intentional. If a job wanted to, it could catch known exceptions and trigger failure. Whereas, expecting all raises to be retry-able seems a bit dangerous. But maybe that's just me.

samsondav commented 4 years ago

I agree @tpitale . I do not think we ought to auto-retry on exception. The user can handle these and return errors if he wants to.

jonnystoten commented 4 years ago

Hey guys, thanks for getting back to me!

I pretty much agree with the sentiment here, but the actual case I was hitting against here was one where I had a process linked to my job which was raising. Ideally, I could catch this and return an error, but AFAIK there is no way to prevent a process from crashing if one of its linked processes crash, so this is the only way I can retry. On the other hand, after this PR we can match on particular errors in the retry_at callback to retry or not based on the exception/exit.

Let me know what you think - if you still disagree I will find another way.

lpil commented 4 years ago

I could catch this and return an error, but AFAIK there is no way to prevent a process from crashing if one of its linked processes crash

You can signal for the process to trap exits and then watch for exit messages :)

Having said that I think it makes sense for jobs to retry on crash. After all, this is the approach that OTP takes.

derekbrown commented 4 years ago

Perhaps it'd be nice to have this behavior configurable? A retry_on_raise: flag or something similar?