Change the way active_provisions are calculated. #16831
Conversation
tinaafitz
force-pushed
the
active_provisions_using_requests
branch
2 times, most recently
from
b1f835d
to
220978e
Compare
|
@simaishi Note, this is currently a blocker on the |
instead of MiqQueue entries. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1489507
tinaafitz
force-pushed
the
active_provisions_using_requests
branch
from
220978e
to
34a83cf
Compare
|
Checked commit tinaafitz@34a83cf with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
tinaafitz
changed the title
|
@miq_bot add_labels bug, fine/yes |
| :request_state => %w(active queued pending), | ||
| :status => 'Ok', | ||
| :process => true | ||
| ).where.not(:id => id) |
There was a problem hiding this comment.
@tinaafitz This is a much clearer solution and almost completely gets the quota logic away from doing lookups in MiqQueue. (One call left in this file)
My one concern is the possibility for false-positive MiqRequest instances being returned. We had that issue before, but it is slightly higher now because in the past clearing a queue would have removed the request from being returned from this method. Now clearing the queue or some other error could leave the request in what looks like a valid state.
Wondering if it makes sense to log a count and list of IDs that are being returned here. Or maybe we create a small script in the tools directory to help users identify what MiqRequest objects are in play for quota.
Thoughts?
There was a problem hiding this comment.
I agree that this is a much clearer solution than the previous MiqQueue lookup logic. Your false-positive concern is valid. While I was testing this changes, I did exactly that and added logging for the count and request ID's being included in the active counts. I'm in favor of the tools/script approach to help users identify the MiqRequest objects in play for quota.
There was a problem hiding this comment.
A major difference in this approach is that requests will be more likely to be denied based on the active requests, whereas before, the requests would be much more likely to be approved and cause quota to go negative.
There was a problem hiding this comment.
Sound good. Please open a git issue for creating that tools script with a reference back to this PR. I think that is a task we can assign to @billfitzgerald0120.
|
@tinaafitz Can you please create a PR for Fine branch? 3 out of 4 files are conflicting and the diff is not small, and not easy to see which line(s) are actually causing the conflicts... Or if I should just take the change from master without worrying about conflicts, let me know! |
…ests Change the way active_provisions are calculated. (cherry picked from commit ca66301) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1536677
|
Gaprindashvili backport details: |
|
Backported to Fine via #16859 |
The current quota implementation has some limitations regarding the active provision calculations.
Changed the quota_find_active_prov_requests method to use MiqRequests instead of MiqQueue entries for for a more accurate count of active provisions.
We now calculate active provisions based on the following MiqRequests:
approval_state: = approved
request_state = active/queued/pending
type = MiqProvisionRequest/ServiceTemplateProvisionRequest
status = 'Ok'
process = true
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1456819