Callback for tracking calculated memory usage per reap attempt

[tobypinder]

Along the similar vein as the pre_term lambda callback, would you accept a PR for a callback that runs per Reaper execution? My use case is that I want to record PumaWorkerKilller's reported memory usage to a StatsD server (Datadog specifically) so recording these values would require a callback that executes every Reap attempt rather than every time the reaper terminates a process.

If so I think I see what's going on with the codebase and testing strategy but is there anything else you would need before such a PR were accepted?

[schneems]

Interesting. In general I like to take PRs to get a sense of what the change would take. It's hard to greenlight something with out seeing an example implementation.

I'm not sure if i'm 100% sure on what hair you're trying to split here though. Are you seeing that PWK attempts to kill workers but is not successful? Can you walk me through an example of the behavior you're getting or think you're getting?

In general I think adding in some kind of a callback for more metrics is good, i also want to better understand your problem.

[tobypinder]

No problems, and I'm happy to add an illustrative PR for reviewal, just wanted to make sure it wasn't a project with a fixed feature set such that I'd be wasting my time.

I'll describe my use case in a bit more detail and how I think said callback would be useful. From my admittedly limited understanding of this gem, the PumaWorkerKiller::Reaper#reap method executes every "config.frequency seconds". This method calculates memory usage and either

• Memory usage is deemed too high
• The preterminate callback is called
• The largest worker is kicked.

or

• No action is taken
• Optionally, a message containing current memory usage as PWK sees it is logged.

My addition to this process would be introducing a callback that happens regardless of the memory usage. It would look something like

def reap
  return false if @cluster.workers_stopped?
  total = get_total_memory
  @per_step_callback.call(total) # name subject to improvement
  if total > @max_ram
    # ... as before ...
  elsif # ... as before ...
  end
end

This way one could set up a callback that gets regular results of the get_total_memory call without having to recalculate memory usage, and I could for report this memory usage to a StatsD server, Cloudwatch or whatever monitoring solution one wanted to use, and produce pretty graphs, autoscaling rules or whatever they wished without incurring any additional computation.

I appreciate that such functionality expands the scope of the gem somewhat but keeping it as nothing more than an optional callback like the pre_term implementation probably keeps things at a tiny surface area. I'll play around with it and figure the best approach to handle this configuration and pass the lambda (similarly to pre_term) and raise a PR that should be easier for a less abstract discussion :+1:

[schneems]

Seems good. Send me a PR