-
Notifications
You must be signed in to change notification settings - Fork 898
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
Run MiqServer.status_update in server process #20904
Run MiqServer.status_update in server process #20904
Conversation
1c3d8a7
to
1b1aa03
Compare
@agrare I have a followup I'm working on to do the same for other logging here. While this other logging is in the same place, it doesn't blow up so I though it's better to keep it separate so this surgical fix is easy to backport. The other logging is just convenient to be logged in the server/orchestrator process in pods since you don't have mixed server/worker processes logging to the same file. EDIT: PR: #20909 |
Fixes ManageIQ#20835 [Errno::ESRCH]: No such process ... when run from a worker. In appliance/systemd, you can run status_update in the server's workers. In podified, each process is isolated so we must run this, specifically process_status, in the server process. It didn't make sense to run this is in the worker processes anyway so change all installations to run this in the server process.
1b1aa03
to
e2018db
Compare
Checked commit jrafanie@e2018db with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint |
In appliances it doesn't matter since the evm.log is the used by all of the processes with the same server guid, but in podified, these processes are isolated and logging is all over the place. It's more convenient to log this in the server process. Followup to ManageIQ#20904
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
In appliances it doesn't matter since the evm.log is the used by all of the processes with the same server guid, but in podified, these processes are isolated and logging is all over the place. It's more convenient to log this in the server process. Followup to ManageIQ#20904
This change benefits pods by having this logging all in the same log file, like appliances, by making the same server process log these messages. This doesn't affect appliances much but will make finding these log message much easier in pods. Followup to ManageIQ#20904
…in_server_process Run MiqServer.status_update in server process (cherry picked from commit e7297f0)
Kasparov backport details:
|
…in_server_process Run MiqServer.status_update in server process (cherry picked from commit e7297f0)
Jansa backport details:
|
This change benefits pods by having this logging all in the same log file, like appliances, by making the same server process log these messages. This doesn't affect appliances much but will make finding these log message much easier in pods. Followup to ManageIQ#20904
Fixes #20835
[Errno::ESRCH]: No such process ... when run from a worker.
In appliance/systemd, you can run status_update in the server's workers.
In podified, each process is isolated so we must run this, specifically
process_status, in the server process.
It didn't make sense to run this is in the worker processes anyway so
change all installations to run this in the server process.
Note, I'm looking at the surrounding lines in this file and seeing that we're logging stuff in random generic/priority workers and much of it doesn't make sense in pods or even appliances... I'll add
:queue_name => 'miq_server'
to them in a followup as they're not BROKEN but just weird. I'll keep this separate since it's broken and more backportable.