jarthod
commented
5 years ago Yep, I don't agree with the two lines per process though, I'm the one who asked for one because we have a lot in production and it means you can see twice as much in one console screen.
Also @w3st3ry you (still) didn't take into account half of the things I asked in my last feedback (which I can't see any more because I sent it via PM in Slack), I gave you the benefit of the doubt but I guess by now I can consider you will never do them. This is not acceptable, you can't waste our time like that. You need to at least read what we say, and if you don't want to do something it must be written somewhere, with arguments, you can't just ignore.
So OK I'll spend some time again to try to give you (mostly) the same feedback, please do it this time:
- For the CPU time of course we should display the instant value it if we can, last time you told me it was not possible I didn't check more but if it is standard in
/proc/statof course we should - We don't need the process index, start with the PID directly to simplify the line, maybe start with "└" for the process to make it look like a child of the application line
- All numbers must be padded on a fixed size to preserve alignement
- Process phase should only be displayed if it's different from application phase to save space, at the end of the line, in orange.
- Uptime should have 1 space between number and letter of course, you should only pad the whole string to maintain alignement, so extra space should be on the left (or right) of the date.
- Last checkin should only be disabled if there's an issue (> 10 sec)
- We don't really need the headers just to loose lines
- I think we can remove the number of workers now that we have the little graph showing the total
- No need for digit in memory size
- Would be better to use human names than path if we have a configuration file
Example output (made quickly by hand):
$ puma-helper status
Version: 1.0.0 | Date: Mon, 06 May 2019 17:19:42 +0200
38423 answers_integration Phase: 28 Active threads: 0/2 [░░]
└ 27771 CPU: 98.4% Mem: 1230MiB Uptime: 9d 3h Threads: 0/1 [░]
└ 27949 CPU: 0.2% Mem: 230MiB Uptime: 11m 29d 23h Threads: 0/1 [░] (Last checkin: 13s) (Phase: 27)
38424 answers_integration API Phase: 28 Active threads: 0/2 [░░]
└ 27892 CPU: 0.1% Mem: 172MiB Uptime: 11m 29d 23h Threads: 0/1 [░]
└ 27961 CPU: 0.1% Mem: 171MiB Uptime: 11m 29d 23h Threads: 0/1 [░]WDYT @spuyet @ylecuyer?
ylecuyer
spuyet
w3st3ry
I think that we should rework the
puma-helperoutput, the current output is really hard to read and we should usepassenger-statusas example:VS
Here is a first draft of something a bit readable and largely inspired from the passenger output:
I know that the go dependency that you're using doesn't support current CPU percentage and to be honest portability is a welcome feature but current CPU usage is a mandatory one IMHO, a software cannot be restricted only because of portability mostly for a tiny project like this one.
This is a first draft, comments and improvements are welcome.
Notes: CPU percentage => use /proc/PID/stat Uptime => use /proc/PID/stat too Queued => corresponding to the backlog value per process in puma stats