zombocom / puma_worker_killer

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

Refine worker termination and rolling restart log messages #96

Closed lipniagov closed 2 years ago

lipniagov commented 3 years ago

Clarifies that total memory is consumed by all workers AND the master process in the log messages.

I found the current message a bit misleading, as it could drive to the conclusion that the listed amount of memory consumed by workers only. My suggestion is to refine it, mentioning the master.

The only concern that somebody may parse this message in the code, so it could lead to incompatibility for them. On the other hand, it's not the best practice to do that, and the gem provides the ability to configure hooks for such events.

lipniagov commented 3 years ago

@schneems What's your opinion?

P.S. I didn't fix rubocop, as goes into a rabbit hole a bit: splitting a string in two violates method length, and so either method split is required or some other manipulations. As I am not sure the whole change is going to be moved forward, do not go further now.