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

Refactor MiqTask.delete_older to queue condition instead of array of IDs #15415

Merged

Conversation

yrudman
Copy link
Contributor

@yrudman yrudman commented Jun 20, 2017

Deleting old tasks records executed in 2 steps: Execute SQL to get list of IDs which need to be deleted (based on passed condition and date) and queue request to MiqTask.destroy with array of IDs as argument.

Issue: when executed on schedule (deleting performance rollup tasks) there is possibility that second request to delete old task could be added to MiqQueue before first request executed. So, in the second request some IDs will be from the first request and attempt to destroy record by id second time will raise error.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1397247

Solution: Instead of queuing list of IDs to destroy - queue actual condition. Also, It would be better from performance perspective.

@miq-bot add-label bug, core

@yrudman yrudman force-pushed the added-delete-by-condition-to-miq-task branch 2 times, most recently from aae9c4b to dce3744 Compare June 21, 2017 13:43
…ueue destroy for those id). If queue is busy then there are could be several request to delete task with the same id. Instead queue actual condition to filter record and execute destroy_all.

https://bugzilla.redhat.com/show_bug.cgi?id=1397247
@yrudman yrudman force-pushed the added-delete-by-condition-to-miq-task branch from dce3744 to d1435a5 Compare June 21, 2017 13:57
@yrudman yrudman changed the title Refactor MiqTask.delete_older Refactor MiqTask.delete_older to queue condition instead of arrays of IDs Jun 21, 2017
@miq-bot
Copy link
Member

miq-bot commented Jun 21, 2017

Checked commits yrudman/manageiq@ad3525f~...d1435a5 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

@yrudman yrudman changed the title Refactor MiqTask.delete_older to queue condition instead of arrays of IDs Refactor MiqTask.delete_older to queue condition instead of array of IDs Jun 21, 2017
@yrudman
Copy link
Contributor Author

yrudman commented Jun 21, 2017

@miq-bot assign @gtanzillo

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@gtanzillo gtanzillo added this to the Sprint 64 Ending Jul 3, 2017 milestone Jun 21, 2017
@gtanzillo gtanzillo merged commit ed1af7c into ManageIQ:master Jun 21, 2017
@yrudman yrudman deleted the added-delete-by-condition-to-miq-task branch June 21, 2017 21:05
@yrudman
Copy link
Contributor Author

yrudman commented Feb 15, 2018

@miq-bot add-label fine/yes

@simaishi could you not backport spec/models/metric_spec.rb which was modified in last commit.

simaishi pushed a commit that referenced this pull request Feb 28, 2018
…iq-task

Refactor MiqTask.delete_older to queue condition instead of array of IDs
(cherry picked from commit ed1af7c)

https://bugzilla.redhat.com/show_bug.cgi?id=1550276
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit f39c617ed64e8b6dff2d67ab58b5f265301781ae
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Jun 21 17:03:06 2017 -0400

    Merge pull request #15415 from yrudman/added-delete-by-condition-to-miq-task
    
    Refactor MiqTask.delete_older to queue condition instead of array of IDs
    (cherry picked from commit ed1af7cff52cf00c418a1f2549cbd98e9739898a)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1550276

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…ion-to-miq-task

Refactor MiqTask.delete_older to queue condition instead of array of IDs
(cherry picked from commit ed1af7c)

https://bugzilla.redhat.com/show_bug.cgi?id=1550276
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.

4 participants