Drop support for forking workers, use spawn by default #19556
Conversation
| # TODO: The api process now looks at the existing UI session as of: https://github.com/ManageIQ/manageiq-api/pull/543 | ||
| # ui-classic should not be but is serialializing its classes into session, so we need to have access to them for deserialization | ||
| # sandboxes;FC:-ActiveSupport::HashWithIndifferentAccess{I"dashboard;FC;q{I"perf_options;FS:0ApplicationController::Performance::Options$typ0:daily_date0:hourly_date0: days0: | ||
| %w[manageiq_default ui_dependencies graphql_api] |
There was a problem hiding this comment.
@skateman See above... because we serialize UI classic classes into session and the web service / api worker can read this existing UI session, it also need to know about UI classic classes
For now, I'm adding ui dependencies to the web service worker but we shouldn't have to pull in all of ui classic for this.
jrafanie
force-pushed
the
spawn_instead_of_fork
branch
2 times, most recently
from
2baf4a6
to
bd32890
Compare
| if ENV['MIQ_SPAWN_WORKERS'] || !Process.respond_to?(:fork) | ||
| start_runner_via_spawn | ||
| elsif systemd_worker? | ||
| if ENV['MIQ_SYSTEMD_WORKERS'] && systemd_worker? |
| def self.build_command_line(guid, ems_id = nil) | ||
| raise ArgumentError, "No guid provided" unless guid | ||
|
|
||
| require 'awesome_spawn' | ||
| cmd = "#{Gem.ruby} #{runner_script}" | ||
| cmd = "nice #{nice_increment} #{cmd}" if ENV["APPLIANCE"] | ||
| cmd = "nice -n #{nice_increment} #{cmd}" if ENV["APPLIANCE"] |
There was a problem hiding this comment.
I'm not sure when nice went to -n format but I think it's been a long time, since we went to fork, so this is why this is changed here.
| # | ||
| # Because we fork workers from the server, if we don't disconnect the pool, | ||
| # any call to reconnect! on a connection will cause the worker's connection | ||
| # to have the server's application_name. |
There was a problem hiding this comment.
Note, this is still needed in general because the first connection to the db will happen before we can set the application name, so it defaults to the rails application name. The comment is no longer relevant though.
jrafanie
changed the title
jrafanie
force-pushed
the
spawn_instead_of_fork
branch
from
bd32890
to
cc7f7c1
Compare
Make systemd optional on systemd enabled systems Api/web service worker needs ui-classic as it can try to read an existing UI session, which can contain serialized classes from ui-classic. Previously, we tried removing fork here: ManageIQ#16130 It was reverted here: ManageIQ#16154 Some of the followups needed to fix the original problems including passing down ems_id to per ems workers, resolved in: ManageIQ#16199 and ManageIQ#18648 At this point, things should just work.
jrafanie
force-pushed
the
spawn_instead_of_fork
branch
from
cc7f7c1
to
a03c912
Compare
|
Checked commit jrafanie@a03c912 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
Make systemd optional on systemd enabled systems
Api/web service worker needs ui-classic as it can try to read an existing UI session, which
can contain serialized classes from ui-classic.
Previously, we tried removing fork here: #16130
It was reverted here: #16154
Some of the followups needed to fix the original problems including passing down
ems_id to per ems workers, resolved in:
#16199 and
#18648
At this point, things should just work.