Kill workers that are stuck in long running queires #21
Conversation
src/pgapp_worker.erl
Outdated
| gen_server:call(W, {squery, Sql}, | ||
| Timeout) | ||
| catch | ||
| exit:E -> |
There was a problem hiding this comment.
Maybe more specific like exit:timeout?
src/pgapp_worker.erl
Outdated
| catch | ||
| exit:E -> | ||
| exit(W, kill), | ||
| exit(E) |
There was a problem hiding this comment.
In general, probably it would be better to keep original stacktrace by using erlang:raise(exit, E, get_stacktrace())
src/pgapp_worker.erl
Outdated
| gen_server:call(W, {equery, Sql, Params}, | ||
| Timeout) | ||
| catch | ||
| exit:E -> |
There was a problem hiding this comment.
the same here. Maybe use generic wrapper?
src/pgapp_worker.erl
Outdated
| gen_server:call(W, {transaction, Fun}, | ||
| Timeout) | ||
| catch | ||
| exit:E -> |
|
very good points! |
| try | ||
| Fun(Worker) | ||
| catch | ||
| Type:Error -> |
There was a problem hiding this comment.
So, are you sure you want to catch all errors, not exit = Type:{timeout, _} = Reason?
There was a problem hiding this comment.
Mostly yes, given that the current code can return incapacitated workers to the pool which can cause serious issues I think killing a worker too many might be better then killing a worker too few.
But all the calls wrapped are nothing more then a gen_server:call so the only other exception possibly being thrown I can think of are all exits anyway.
This PR take a brutal approach at assassinating workers that happen to time out due to long running requests! This might not be the perfect solution, I'm more then open to better alternatives.
This is fixes the issue described in #20
Please be aware that as a side effect it includes hex related changes.