Add MiqWorkerType model #19536
Add MiqWorkerType model #19536
Conversation
|
Cross repo test ManageIQ/manageiq-cross_repo-tests#15 |
| "#{class_name}::CloudManager::EventCatcher" => %i(manageiq_default), | ||
| "#{class_name}::CloudManager::MetricsCollectorWorker" => %i(manageiq_default), | ||
| "#{class_name}::CloudManager::RefreshWorker" => %i(manageiq_default), | ||
| RB |
carbonin
force-pushed
the
add_miq_worker_types
branch
from
059b777
to
7fd3bc9
Compare
|
I absolutely love this |
carbonin
force-pushed
the
add_miq_worker_types
branch
from
7fd3bc9
to
b12f595
Compare
|
Regarding the kill order priorities, this may be a good time to revisit the list. Here is what I currently have implemented in this PR:
Which, for completeness, in the database looks like this: As a simple start I would move the web service worker toward to bottom because you can't log in without the API, and move the cockpit and remote console workers up, maybe before refresh because those are not always being used. Does the VMware event catcher use the vim broker? If it does I think we should move the broker after the event catchers. Also not sure about the relative position of the event handler and the event catchers. Does it make sense to move the ems refresh core worker closer to the other refresh workers? |
|
Also unless we get the list down to a more manageable size, I'm not sure we can name all of the priorities @Fryguy, right now we have 16 levels (I used 10 to 160 to allow for new groupings between existing ones). |
|
@agrare what do you think about the provider worker priorities? |
|
I would leave them as close to default for this PR, then have a separate PR for reordering. Once the constants are in place, it's a simple matter of just changing them. |
16 doesn't feel all that bad...I'd go for it 😄 |
No, it uses the VMwareWebService gem but it doesn't rely on the MiqVimBrokerWorker running |
This adds a model which will replace the worker type constant. It seeds the table based on the leaf subclasses of MiqWorker and each worker class needs to define its own attributes, specifically its priority in the kill order and bundler groups. This allows new workers to be added by simply adding a new subclass of MiqWorker rather than maintaining a list of worker classes in the core repo. Additionally this brings us closer to the ability to run a separate process that will access the database and determine which workers should be running on which servers, all outside of the main application code-base.
carbonin
force-pushed
the
add_miq_worker_types
branch
from
b12f595
to
03e1c86
Compare
Did the best I could, but only came up with 15: Pretty sure I'd still rather just have numbers 😆 |
|
Assuming we're not defining 15 constants for priorities this will be good to go once the provider and schema PRs are merged. |
OHHHHH I totally wasn't expecting that at all... I was thinking more like in core we do: KILL_PRIORITY_METRICS_PROCESSOR_WORKERS = 10
KILL_PRIORITY_METRICS_COLLECTOR_WORKERS = 20
KILL_PRIORITY_REPORTING_WORKERS = 30
KILL_PRIORITY_SMART_PROXY_WORKERS = 40
KILL_PRIORITY_GENERIC_WORKERS = 50
KILL_PRIORITY_EVENT_HANDLERS = 60
KILL_PRIORITY_REFRESH_WORKERS = 70
KILL_PRIORITY_SCHEDULE_WORKERS = 80
KILL_PRIORITY_PRIORITY_WORKERS = 90
KILL_PRIORITY_WEB_SERVICE_WORKERS = 100
KILL_PRIORITY_REFRESH_CORE_WORKERS = 110
KILL_PRIORITY_VIM_BROKER_WORKERS = 120
KILL_PRIORITY_EVENT_CATCHERS = 130
KILL_PRIORITY_UI_WORKERS = 140
KILL_PRIORITY_REMOTE_CONSOLE_WORKERS = 150
KILL_PRIORITY_COCKPIT_WS_WORKERS = 160Then in a worker you do: # app/models/manageiq/providers/base_manager/event_catcher.rb
def self.kill_priority
KILL_PRIORITY_EVENT_CATCHERS
endSeeding will just use those values so we can change them at any time and they should update. For pluggability, eventually something like the VimBrokerWorker, which is vmware specific, can do something like the following (and we would remove def self.kill_priority
KILL_PRIORITY_WEB_SERVICE_WORKERS + 5
endwhich allows them to insert themselves in the list relative to other items in the list. |
|
ahahaha okay that makes a ton more sense @Fryguy I'll go that way. |
lib/workers/bin/run_single_worker.rb
Outdated
| STDERR.puts "ERR: `#{worker_class}` WORKER CLASS NOT FOUND! Please run with `-l` to see possible worker class names." | ||
| exit 1 | ||
| end | ||
|
|
||
| # Skip heartbeating with single worker | ||
| ENV["DISABLE_MIQ_WORKER_HEARTBEAT"] ||= options[:heartbeat] ? nil : '1' | ||
| ENV["BUNDLER_GROUPS"] = MIQ_WORKER_TYPES[worker_class].join(',') | ||
| ENV["BUNDLER_GROUPS"] = worker_type.bundler_groups.join(',') |
There was a problem hiding this comment.
Commenting regarding discussion we had in person... because line 63 loads environment which will have already read and used the BUNDLER_GROUPS env var from application.rb here, this manipulation will be too late.
There was a problem hiding this comment.
For this I moved the bundler groups envvar to the caller of run_single_worker.rb.
The logic was that we would either take the caller's env var if it was set and have some default in this script, but since that exact logic is already taken care of in application.rb, I didn't think it made sense to duplicate it here.
The only thing we lose with this approach is getting the correct bundler groups when someone runs run_single_worker.rb from the command line, but I would argue that if that person wants bundler groups, they will know to set the environment variable.
| end | ||
|
|
||
| private_class_method def self.classes_for_seed | ||
| @classes_for_seed ||= MiqWorker.descendants.select { |w| w.subclasses.empty? } - EXCLUDED_CLASS_NAMES.map(&:constantize) |
There was a problem hiding this comment.
I forget, will this autoload all workers in subclasses in external repos such as provider workers?
I wonder what will happen here when/if we dissect bundler groups into small components and therefore won't have all the worker classes. But that's for another day.
There was a problem hiding this comment.
Yes, it will autoload all the provider subclasses.
If we split out the bundler groups for providers I would imagine the server would need all of the groups to be able to deal with provider constants, right? I don't think any of the workers with pared down bundler groups would be calling this method.
The caller will set the bundler groups outside of the worker entrypoint. This also prevents the chicken-and-egg problem of needing to get the bundler groups from the database before loading the environment
|
Some comments on commits carbonin/manageiq@2947aac~...5c9469f lib/workers/bin/run_single_worker.rb
|
|
Checked commits carbonin/manageiq@2947aac~...5c9469f with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 spec/models/miq_server/worker_management/monitor_spec.rb
|
|
cross_repo tests were green, so merging. |
This adds a model which will replace the worker type constant.
It seeds the table based on the leaf subclasses of MiqWorker
and each worker class needs to define its own attributes, specifically
its priority in the kill order and bundler groups.
This allows new workers to be added by simply adding a new subclass
of MiqWorker rather than maintaining a list of worker classes in the
core repo.
Additionally this brings us closer to the ability to run a separate
process that will access the database and determine which workers
should be running on which servers, all outside of the main application
code-base.
Requires: