zombocom / puma_worker_killer

Automatically restart Puma cluster workers based on max RAM available
748 stars 77 forks source link

Add a pre-worker-termination callback #49

Closed jacobsmith closed 7 years ago

jacobsmith commented 7 years ago

I have been using PumaWorkerKiller in production for a while and have had good results. However, we would also like to be able to track when and how often PumaWorkerKiller is terminating a worker. (We use Instrumental for tracking metrics, and it's much easier to track them at the time an event happens than trying to parse logs after the fact).

This PR provides a config.pre_term option that accepts any callable object (method, lambda, proc). It then invokes that pre_term lambda just prior to terminating the largest worker. It is not currently implemented on the RollingRestart class, though I can certainly add that if you would like.

The ability to pass a lambda means that one can use other Rails-defined methods, classes, etc. because the lambda is evaluated in the context of a loaded Rails application, even though it is defined in the config (usually the initializer file, I would imagine).

Please let me know what changes you might want to see; I'll gladly update the PR with comments and suggestions!

Thanks!

jacobsmith commented 7 years ago

closing and reopening to retrigger travis-ci build

schneems commented 7 years ago

I'm good with this, thanks for the contribution. Is the build for you green locally? I'm honestly not sure if master is still working.

jacobsmith commented 7 years ago

@schneems just pushed an update that made the build go green locally (I had renamed pre_term from pre_term_function and forgotten to update the filename to pre_term.ru as well).

jacobsmith commented 7 years ago

Last failure was due to post 1.9 lambda syntax. I've gone ahead and used the older lambda { |arg| } syntax to maintain compatibility with ruby 1.9.3 as tested in CI.

schneems commented 7 years ago

Thanks!

jacobsmith commented 7 years ago

@schneems thanks! Is there anything I can do to help getting v0.0.8 up to RubyGems? (I'd rather our production system point to the canonical version of puma_worker_killer than a fork to ensure we get future updates.)

schneems commented 7 years ago

I cut this into 0.1.0 since this is a pretty major feature addition (compared to teeny releases). Thanks for your work.

jacobsmith commented 7 years ago

@schneems awesome, thank you! I'm looking to get into OSS a bit more, so please let me know if there's ever any issues you'd like for me to take a look at!