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

Handle pid in run_single_worker.rb properly #15820

Conversation

NickLaMuro
Copy link
Member

In classes that inherit from MiqQueueWorkerBase, they will fetch images using MiqQueue.get. Part of what is done in that is calling MiqWorker.my_worker. This basically makes a call to find a worker with the pid value that matches the same pid used by the current process.

Unfortunately, this is normally set by the MiqServer prior to forking the worker, so this isn't set by run_single_worker.rb.

To address this, either add to the create_options an attribute for :pid that will create a new worker with the existing pid of the current process, or if a :guid is passed in, update the attributes of the existing worker to set the pid. This is already done by the MiqServer, so this is just making the run_single_worker.rb handle setting the pid as well.

Steps for Testing/QA

  • Apply the following diff (needed so the MiqGenericWorker boots properly, and adds some handy debugging output):

    diff --git a/app/models/miq_generic_worker/runner.rb b/app/models/miq_generic_worker/runner.rb
    index 022db94..15c0175 100644
    --- a/app/models/miq_generic_worker/runner.rb
    +++ b/app/models/miq_generic_worker/runner.rb
    @@ -1,3 +1,3 @@
     class MiqGenericWorker::Runner < MiqQueueWorkerBase::Runner
    -  self.delay_startup_for_vim_broker = true # NOTE: For ems_operations and smartstate roles
    +  self.delay_startup_for_vim_broker = false # NOTE: For ems_operations and smartstate roles
     end
    diff --git a/app/models/miq_queue_worker_base/runner.rb b/app/models/miq_queue_worker_base/runner.rb
    index 206066b..0179d9d 100644
    --- a/app/models/miq_queue_worker_base/runner.rb
    +++ b/app/models/miq_queue_worker_base/runner.rb
    @@ -96,6 +96,7 @@ class MiqQueueWorkerBase::Runner < MiqWorker::Runner
       end
    
       def deliver_queue_message(msg)
    +    print "delivering queue message #{msg.class_name}.#{msg.method_name}..."
         reset_poll_escalate if poll_method == :sleep_poll_escalate
    
         begin
    @@ -128,6 +129,7 @@ class MiqQueueWorkerBase::Runner < MiqWorker::Runner
           #
           clean_broker_connection
         end
    +    puts "done!"
       end
    
       def deliver_message(msg)
    diff --git a/lib/workers/miq_worker_types.rb b/lib/workers/miq_worker_types.rb
    index 6ee8bdd..4189372 100644
    --- a/lib/workers/miq_worker_types.rb
    +++ b/lib/workers/miq_worker_types.rb
    @@ -62,7 +62,7 @@ MIQ_WORKER_TYPES = {
       "MiqEmsMetricsProcessorWorker"                                              => [],
       "MiqEmsRefreshCoreWorker"                                                   => [],
       "MiqEventHandler"                                                           => [],
    -  "MiqGenericWorker"                                                          => [],
    +  "MiqGenericWorker"                                                          => [:manageiq_default],
       "MiqPriorityWorker"                                                         => [],
       "MiqReportingWorker"                                                        => [],
       "MiqScheduleWorker"                                                         => [:scheduler],
  • Start a worker using the run_single_worker.rb script:

    $ ruby lib/workers/bin/run_single_worker.rb MiqGenericWorker
  • In a seperate tab, run an rails console with the following commands:

    $ bin/rails console
    irb> MiqQueue.put :class_name => "Kernel", :method_name => "sleep", :args => 200, :zone => MiqServer.my_server.zone.name, :queue_name => "generic"
    #=> #<MiqQueue id: XXX, ...>
    irb> MiqQueue.last  # wait until the worker picks up the work before executing this line

You should see handler_type and handler_id fields properly set to MiqWorker and the id of the worker currently running (versus setting it to the server).

You can also run this using an existing worker by calling MiqGenericWorker.create_worker_record, grabbing the resulting guid, and then running the run_single_worker.rb script from above with the --guid [GUID_FROM_BEFORE] option. Then any jobs queued and picked up will then be using the handler of that worker created in the console.

In classes that inherit from MiqQueueWorkerBase, they will fetch images
using `MiqQueue.get`.  Part of what is done in that is calling
`MiqWorker.my_worker`.  This basically makes a call to find a worker
with the `pid` value that matches the same pid used by the current
process.

Unfortunately, this is normally set by the `MiqServer` prior to forking
the worker, so this isn't set by `run_single_worker.rb`.

To address this, either add to the `create_options` an attribute for
`:pid` that will create a new worker with the existing pid of the
current process, or if a `:guid` is passed in, update the attributes of
the existing worker to set the pid.  This is already done by the
`MiqServer`, so this is just making the `run_single_worker.rb` handle
setting the pid as well.
@miq-bot
Copy link
Member

miq-bot commented Aug 15, 2017

Checked commit NickLaMuro@f2c4abd with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@NickLaMuro
Copy link
Member Author

@miq-bot assign @jrafanie

Let me know if you have any questions about this, but I think you are pretty equipped to understand what I am trying to fix here.

@jrafanie jrafanie added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 16, 2017
@jrafanie jrafanie merged commit fbafc17 into ManageIQ:master Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants