[V2V] Allow active InfraConversionJob to be throttled

[ghost]

The JobProxyDispatcher.dispatch is called only if JobProxyDispatcher.waiting? returns true. With existing implementation, it means that there is a Job in state waiting_to_start. However, the dispatch method calls the InfraConversionThrottler.apply_limits method that we need to run every 15 seconds as long as at least 1 InfraConversionJob is active, i.e. not in finished or waiting_to_start state.

This PR extends the check to include verification that active InfraConversionJob exists.

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1690851

[ghost]

@miq-bot add-label transformation, bug, ivanchuk/yes
@miq-bot add-reviewer @agrare
@miq-bot add-reviewer @djberg96

[djberg96]

I feel like there should be a way for other classes to hook into this and define their own method that the JobProxyDispatcher then uses, but it doesn't look like there's a way.

Hypothetically, I'm thinking of something like this:

def self.waiting?(job_class = Job)
  job_class.waiting_jobs?
end

And then each subclass of Job would define its own waiting_jobs? method.

[djberg96]

Would like to see this refactored in general down the road, but this is good for now.

[agrare]

I don't like that this does two queries given how often this runs.

Instead of || can you use .or() something like this?

>> Job.where(:state => "waiting_to_start").or(Job.where(:type => "InfraConversionJob").where.not(:state => ["finished", "waiting_to_start"]))
  Job Load (0.9ms)  SELECT  "jobs".* FROM "jobs" WHERE ("jobs"."state" = $1 OR "jobs"."type" = $2 AND "jobs"."state" = 'running') LIMIT $3  [["state", "waiting_to_start"], ["type", "InfraConversionJob"], ["LIMIT", 11]]
  Job Inst Including Associations (0.0ms - 0rows)
=> #<ActiveRecord::Relation []

[ghost]

Changed the query and added specs. Thanks @agrare

[agrare]

I agree this isn't great but might be the best we can do for now. @fdupont-redhat in addition to the query change can you add a test covering this case?

[miq-bot]

Checked commits fabiendupont/manageiq@ea17a7b~...5d483da with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

[agrare]

👍 thanks @fdupont-redhat

[simaishi]

Ivanchuk backport details:

$ git log -1
commit eb6b2d647937029237c88f448e42459b955fbc8c
Author: Adam Grare <agrare@redhat.com>
Date:   Thu Sep 12 08:21:56 2019 -0400

    Merge pull request #19277 from fdupont-redhat/v2v_allow_dispatching_running_infraconversionjob
    
    [V2V] Allow active InfraConversionJob to be throttled
    
    (cherry picked from commit 1ec27c37b21eb91013e20bfd437f608481c876d7)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1768518

[simaishi]

@fdupont-redhat Travis is failing in ivanchuk branch. Are we missing some PR(s) in ivanchuk branch or do the specs added in this PR need to be modified in ivanchuk?

  1) JobProxyDispatcher.waiting? returns true if VmScan state is fake_state and InfraConversionJob state is restoring_vm_attributes
     Failure/Error:
       def self.create_job(process_type, options = {})
         klass = Object.const_get(process_type)
         ar_options = options.dup.delete_if { |k, _v| !Job.column_names.include?(k.to_s) }
         job = klass.new(ar_options)
         job.options = options
         job.initialize_attributes
         job.save
         job.create_miq_task(job.attributes_for_task)
         $log.info("Job created: #{job.attributes_log}")
         job.signal(:initializing)
     ArgumentError:
       wrong number of arguments (given 0, expected 1..2)
     # ./app/models/job.rb:19:in `create_job'
     # ./spec/models/job_proxy_dispatcher_spec.rb:22:in `block (3 levels) in <top (required)>'
     # ./spec/models/job_proxy_dispatcher_spec.rb:38:in `block (3 levels) in <top (required)>'

[djberg96]

#19461

[simaishi]

Thanks Dan!