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

[Podification] Removal of the MiqVimBrokerWorker #484

Closed
agrare opened this issue Nov 21, 2019 · 4 comments
Closed

[Podification] Removal of the MiqVimBrokerWorker #484

agrare opened this issue Nov 21, 2019 · 4 comments
Assignees
Milestone

Comments

@agrare
Copy link
Member

agrare commented Nov 21, 2019

After investigating strategies for using the VimBroker in a containerized environment (#469) we decided that the last option should be pursued.

This involves complete removal of the MiqVimBrokerWorker due to DRb being fundamental to the functioning of the broker and DRb being completely unusable in a containerized environment for use across pods.

The broker is currently used for a number of different services on MIQ

  1. Inventory Refresh (cache)
  2. Provider Operations (connection brokering + cache)
  3. Smartstate (connection brokering + cache)
  4. Metrics Capture (connection brokering + cache)

For inventory refresh we have the streaming refresh option which will have to be made the only option.
Provider operations can be handled by a new OperationsWorker (ManageIQ/manageiq#19476, #468) can keep a local connection and inventory cache which can be used within the process rather than going out over DRb
Smartstate is essentially just another type of provider operation and can be processed by the OperationsWorker
Metrics capture is also another provider operation, it could either be handled by the operations worker or we could have another cached session in the MetricsCaptureWorker.

Getting the operations worker to work is where most of the time will be spent. Currently all ems_operations queue roles are handled by the generic/priority workers. We will need to add a queue_name to all of these to allow provider workers to optionally handle the this type of work (initial effort ManageIQ/manageiq#19479)

ManageIQ/manageiq#19543

@djberg96
Copy link
Contributor

djberg96 commented Dec 3, 2019

Mainly for my own sake, a current list of all files where I see an ems_operations role in core:

Will cross these off as they're done (or if they're already done).

ems_operations appears, but not specifically in the context of a role:

@carbonin
Copy link
Member

carbonin commented Dec 4, 2019

Also, the broker is the last user of the MiqWorker#send_message_to_worker_monitor method, so something like this patch can also go in when the broker is gone 🙏 :

diff --git a/app/models/miq_server/worker_management/heartbeat.rb b/app/models/miq_server/worker_management/heartbeat.rb
index 74a2d0ef63..6a77e217b9 100644
--- a/app/models/miq_server/worker_management/heartbeat.rb
+++ b/app/models/miq_server/worker_management/heartbeat.rb
@@ -41,11 +41,6 @@ module MiqServer::WorkerManagement::Heartbeat
     end unless @workers_lock.nil?
   end
 
-  def message_for_worker(wid, message, *args)
-    w = MiqWorker.find_by(:id => wid)
-    worker_set_message(w, message, *args) unless w.nil?
-  end
-
   def request_workers_to_sync_config(resync_needed = false)
     processed_worker_ids = []
     miq_workers.each do |w|
diff --git a/app/models/miq_worker.rb b/app/models/miq_worker.rb
index 3ea825f03a..f174c1fb54 100644
--- a/app/models/miq_worker.rb
+++ b/app/models/miq_worker.rb
@@ -291,25 +291,6 @@ class MiqWorker < ApplicationRecord
     MiqWorker.log_status(level)
   end
 
-  def self.send_message_to_worker_monitor(wid, message, *args)
-    w = MiqWorker.find_by(:id => wid)
-    raise _("Worker with id=<%{id}> does not exist") % {:id => wid} if w.nil?
-    w.send_message_to_worker_monitor(message, *args)
-  end
-
-  def send_message_to_worker_monitor(message, *args)
-    MiqQueue.put_deprecated(
-      :class_name  => 'MiqServer',
-      :instance_id => miq_server.id,
-      :method_name => 'message_for_worker',
-      :args        => [id, message, *args],
-      :queue_name  => 'miq_server',
-      :zone        => miq_server.zone.name,
-      :server_guid => miq_server.guid
-    )
-  end
-
-
   # Overriding queue_name as now some queue names can be
   # arrays of names for some workers not just a singular name.
   # We use JSON.parse as the array of names is stored as a string.

@chessbyte chessbyte changed the title Removal of the MiqVimBrokerWorker [Podification] Removal of the MiqVimBrokerWorker Dec 19, 2019
@djberg96
Copy link
Contributor

djberg96 commented Dec 20, 2019

Here's everything that has "vim_broker" or "VimBroker" within it yet:

  • app/models/miq_generic_worker/runner.rb
  • app/models/miq_server/server_smart_proxy.rb
  • app/models/miq_worker/runner.rb
  • app/models/job_proxy_dispatcher.rb
  • app/models/vm_scan.rb
  • app/models/miq_worker_type.rb
  • app/models/manageiq/providers/base_manager/refresh_worker/runner.rb
  • app/models/miq_queue_worker_base/runner.rb
  • app/models/vm_or_template/operations/snapshot.rb

@Fryguy
Copy link
Member

Fryguy commented Feb 3, 2020

Closing now, as the rest are pure cleanup, but the "Removal of the Broker" is done at this point.

🎉 🌮 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Jansa
Development

No branches or pull requests

6 participants