Error in decoding job in WorkerManager causes Application crash about verk HOT 8 CLOSED

First, thanks for the bug report!

Why is our application crashing?
Is this expected?

The application dies cause the Verk.Supervisor is restarting too many times (because the workers manager is restarting too many times). It's really simple to tweak the restart strategy to be something like: "max restart times 99999" and possibly :infinity so that Verk.Supervisor will never cause a restart of the supervisor above (the App supervisor probably)

We could also discuss that the workers manager should never give up restarting (but then what if there's actually a bug on it or something?)

What if we call Poison.decode instead and then move jobs with bad JSON into a malformed key in Redis?

That's a hard one. I'm not sure what's the best approach when the data can't be parsed properly. This should be an edge case, right? I would like some solution that doesn't complicate a lot the flow of data but it also gives enough information to the user that something was wrong. Maybe logging for now would be enough?

from verk.

Would it be possible to add a new GenEvent and report it that way along with logging?

from verk.

Thank you both for providing some help and suggestions! To answer them:

We could also discuss that the workers manager should never give up restarting (but then what if there's actually a bug on it or something?)

I agree, this would not be a good thing to do. I'm fine with not changing that.

I'm not sure what's the best approach when the data can't be parsed properly. This should be an edge case, right?

Possibly? Our usage right now is to encode the jobs into JSON from a legacy PHP app and write to Redis, Verk then picks up the job. I imagine this is likely a common workflow, although maybe coming from Ruby applications.

I would like some solution that doesn't complicate a lot the flow of data but it also gives enough information to the user that something was wrong.

I agree that a complication of data movement is not ideal. I did some initial work, but admittedly it does complicate things: https://github.com/edgurgel/verk/compare/master...keyan:malformed_set_encoding_issues?expand=1

Maybe logging for now would be enough?

Are you suggesting we delete the job and log it? I think that would be reasonable in general, but in my case I have ~5000 emails stuck in Redis, probably a large number are malformed. I probably have to recover the data somehow if Verk cannot process it, so logging and deleting the job wouldn't be enough.

Would it be possible to add a new GenEvent and report it that way along with logging?

@mitchellhenke, as in do the logging inside of a handler? Is this what you mean?

def handle_event({:malformed, job}, message) do
  Logger.error "#{message}"
end

I agree we have to log something. What are your thoughts on finding a way to put the jobs into the DeadSet? I think it makes sense that malformed jobs would be sent there, it doesn't complicate the flow too much, and still allows for a way to keep the jobs should you need to recover them.

Thoughts?

from verk.

Okay, I poked around and tried implementing something to send jobs to the dead queue when malformed, and it was pretty messy. I'm okay with just logging the job for now.

Any suggestions on how to handle this? QueueManager.dequeue will have already moved the job into the inprogress queue, see here. We need to remove the job before logging and skipping it. Any input is appreciated!

from verk.

Yeah we need to add to the dead set and then remove from the inprogress acknowledging the job.

from verk.

@edgurgel, oh okay. I misunderstood. I thought you didn't want to move it to the dead set, just log and remove from inprogress (which is what I did in #91). I'll update there.

from verk.

Yeah I wasn't clear! I'm not super decided on which solution to take but logging is way easier :D

from verk.

Haha no worries. I agree, logging is way easier. I worked on first pass which sent the jobs to the dead set and it complicated things more than I'd like. Given that you are on the fence and the complication you mentioned in #91 (comment), I am going to go forward with just logging and removing the job.

I ran the changes in #91 in production and we were able to clear our queues, only 5 jobs had the malformed data issue. I think this is an edge case as you mentioned and probably not worth muddying up dead set because of it. If we decide to eventually send these to the dead set, the change will be add on #91 easily.

from verk.