Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.0] Display worker CPU and memory utilization in supervisor list #589

Merged
merged 1 commit into from May 17, 2019
Merged

[3.0] Display worker CPU and memory utilization in supervisor list #589

merged 1 commit into from May 17, 2019

Conversation

AJenbo
Copy link
Contributor

@AJenbo AJenbo commented May 7, 2019

So here is my first PR in relation to #586

This adds information about how much system resources (CPU and memory) the workers are currently consuming:
image

I have tried to communicate this in the most user friendly way possible, but here are some additional details:

  • CPU Threads shows how many logical CPU cores the workers are currently occupying, if the number is close to the actual number of logical CPU cores then the system is probably being maxed out by the workers. Other process will also take resources, so this is not meant to show you system load but rather how much is being used by the workers. As such if the server is doing other tasks it might not be a replacement for other devops tools.
  • Memory, this shows how much memory is currently being used by the workers. This is shown as a percentage of total system memory.

@driesvints driesvints changed the title Display worker CPU and memory utilization in supervisor list [3.0] Display worker CPU and memory utilization in supervisor list May 7, 2019
src/Repositories/RedisSupervisorRepository.php Outdated Show resolved Hide resolved
src/SystemProcessCounter.php Outdated Show resolved Hide resolved
@driesvints driesvints dismissed their stale review May 7, 2019 12:05

Changes were made.

@taylorotwell
Copy link
Member

I feel like "CPU Threads" is going to be a bit confusing for people. I'm not used to seeing that as a metric.

@AJenbo
Copy link
Contributor Author

AJenbo commented May 8, 2019

It's what is normally implied by UNIX systems when they show "CPU" (see uptime and top for example). In my experience people are often taking that as being the entire CPU and thus gets confused when the number reaches 2.27, so that's why I'm trying to be more explicit here.

Trying to normalize the number to get total CPU usage is non trivial on virtualized systems where not all resources may be available to the running container.

@taylorotwell
Copy link
Member

This works on Windows, Mac, and Linux?

@AJenbo
Copy link
Contributor Author

AJenbo commented May 13, 2019

Horizon 3.0 as is doesn't work on Windows, as it depends on php_pcntl, ps and a few other UNIX only features (Redis isn't officially supported for Windows either).

This change simply parses the output from ps and so doesn't add any new dependencies, the changes have been tested on both Linux and Mac.

The UI of cause displays correctly on Windows as before.

@taylorotwell taylorotwell merged commit a4ae4e5 into laravel:3.0 May 17, 2019
@AJenbo AJenbo deleted the stats-pr branch May 21, 2019 16:24
@JayBizzle
Copy link
Contributor

@driesvints I see you released v3.2.0 but I think the JS needs rebuilding for this PR to work as the resources/js/screens/dashboard.vue file was changed 👍

@driesvints
Copy link
Member

@JayBizzle that's odd. I see @themsaid compiled it here: 8ee690a

So it should be working afaik...

@JayBizzle
Copy link
Contributor

Weird, can't get it to work after updating....let me try again...

@driesvints
Copy link
Member

Nvm. Seems the pr was merged after it. I'll release a patch version with the compiled assets 👍

@driesvints
Copy link
Member

@JayBizzle released v3.2.1

@JayBizzle
Copy link
Contributor

Yep, just tested locally, had to rebuild the assets 👍

@JayBizzle
Copy link
Contributor

Updated to v3.2.1, all working as expected 👍

@Cannonb4ll
Copy link
Contributor

Great work @AJenbo, love this addition to Horizon 🔥

@AJenbo
Copy link
Contributor Author

AJenbo commented May 22, 2019

Thanks ☺️, I'll work on adding some more goodies over the next few weeks

@tomswinkels
Copy link
Contributor

Very nice @AJenbo !

@andreladocruz
Copy link

andreladocruz commented May 25, 2019

If the front end is running in a different machine from the queue processor one, the stats are not showing.

Maybe saving the stats in redis should solve the issue. ;)

@AJenbo
Copy link
Contributor Author

AJenbo commented May 25, 2019

I have tested it on systems where the frontend is running in a different machine. And the results are saved in Redish. @andreladocruz Could you describe more accurately exactly what you are experiencing? Did you restart your supervisor after updating the package?

@andreladocruz
Copy link

@AJenbo, we have our frontend running in google appengine, our queue processor running in a Computer Engine machine, and our redis is a Memorystore stack.

Just updated horizon to 3.2.1 version and can´t any information about cpu and memory usage:

image

That´s what I´m talking about.

If the queue processor is in another server rather then the frontend one, no information is shown.

@AJenbo
Copy link
Contributor Author

AJenbo commented May 25, 2019

Since the columns labels are entirely missing your browser is still displaying Horizon 3.2.0 or older, maybe you have some caching that hasn't been cleared after the deployment or your AppEngine wasn't updated properly. This has nothing to do with running on multiple servers.

@driesvints
Copy link
Member

Please remember to follow the upgrade guide and re-publish the assets when updating horizon: https://laravel.com/docs/5.8/horizon#upgrading

@andreladocruz
Copy link

@AJenbo and @driesvints , thanks for reminding to publish the assets. =P

Everything is working okay.

Sorry about that.

@CyrilMazur
Copy link

I experience a performance hit (+5-6% CPU usage) when upgrading from 3.1.2 to 3.2.1. Anybody else? I opened an issue: #601

@driesvints
Copy link
Member

Since this PR was causing quite some bugs for others and also introduced a performance hit we've decided to revert it entirely. We also won't be re-considering a new PR anymore. We'd like to encourage you to use other tools to track and measure CPU/Memory consumption and just keep Horizon to its core features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants