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.

Sign up for GitHub

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

Jump to bottom

[WIP]Use create_with vs use blocks for miq_queue updates #14569

Closed
wants to merge 1 commit into from
Closed

[WIP]Use create_with vs use blocks for miq_queue updates #14569

wants to merge 1 commit into from

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Mar 29, 2017 •
edited
Loading

Blocked on

The goal is to have fewer put_or_update calls and move over to put_unless_exists, which are easier to express as a single query.

The high-level premise is we are using put_or_update because we didn't know how to use put_unless_exists properly.

related: #14676

@kbrock kbrock requested a review from Fryguy March 29, 2017 23:40
@kbrock kbrock mentioned this pull request Apr 3, 2017
Merged
@kbrock kbrock force-pushed the miq_queue_updates_2 branch from 3d3536a to d61a2e5 Compare April 3, 2017 21:54
@kbrock
Copy link
Member Author

kbrock commented Apr 3, 2017

Regarding the failure, it looks like it was manipulating the values going into the db. So this is a change in behavior

I'm ok changing this behavior:

  1. We should be going to a simple put. Almost every pruning method is done in 100% sql and is quick. It is more expensive to check a duplicate in the queue than to delete non-existing records.
  2. If the queue is backed up 15 minutes, the timestamps will be within 15 minutes of each other. Can't imagine that many more records will be deleted with a 15 minute created_at timestamp difference.

@kbrock
Copy link
Member Author

kbrock commented Apr 4, 2017

Oh fun, so put defaults priorityif it is not passed in. Using create_with() only currently works for attributes that do not have a default value.

@kbrock kbrock changed the title Leave queue entries as is vs updating them [WIP] Leave queue entries as is vs updating them Apr 4, 2017
@kbrock kbrock added the wip label Apr 4, 2017
@kbrock
Copy link
Member Author

kbrock commented Apr 4, 2017

WIP:
pull out purges (as puts)
ensure widget and message logs are working well
The defaulting of options can botch us up here.

@kbrock kbrock force-pushed the miq_queue_updates_2 branch 3 times, most recently from d6eb116 to ece66a2 Compare April 7, 2017 19:50
@kbrock
Copy link
Member Author

kbrock commented Apr 7, 2017 •
edited
Loading

Non WIP
cops fixed - fixed in other PR

just 1 commit

@kbrock kbrock changed the title [WIP] Leave queue entries as is vs updating them [WIP] Use create_with vs use blocks for miq_queue updates Apr 7, 2017
@kbrock kbrock force-pushed the miq_queue_updates_2 branch 3 times, most recently from 62f045c to c69bb53 Compare April 10, 2017 17:43
@kbrock kbrock changed the title [WIP] Use create_with vs use blocks for miq_queue updates Use create_with vs use blocks for miq_queue updates Apr 14, 2017
@kbrock kbrock removed the wip label Apr 14, 2017
@kbrock kbrock force-pushed the miq_queue_updates_2 branch 3 times, most recently from 34439bd to a1e8b74 Compare April 18, 2017 16:42
@kbrock
Copy link
Member Author

kbrock commented Apr 20, 2017

Removed last rubo-cop warning.

@kbrock kbrock mentioned this pull request Apr 20, 2017
Merged
@miq-bot
Copy link
Member

miq-bot commented Apr 21, 2017

This pull request is not mergeable. Please rebase and repush.

@kbrock kbrock force-pushed the miq_queue_updates_2 branch from a1e8b74 to 7c22332 Compare April 24, 2017 15:07
@kbrock
Copy link
Member Author

kbrock commented Apr 24, 2017

pulled out the kubernetes fixes.

@miq-bot
Copy link
Member

miq-bot commented May 15, 2017

This pull request is not mergeable. Please rebase and repush.

@kbrock
MiqQueue.create_with() instead of block manipulation
331c289 
Getting away from manipulating the put_unless_exists hash
and instead

AuthenticationMixin requires ManageIQ#14562 which allows deliver_on
to work in miq_queue.create_with
@kbrock kbrock force-pushed the miq_queue_updates_2 branch from 7c22332 to 331c289 Compare June 6, 2017 15:10
@kbrock kbrock changed the title Use create_with vs use blocks for miq_queue updates [WIP]Use create_with vs use blocks for miq_queue updates Jun 6, 2017
@kbrock kbrock mentioned this pull request Jun 6, 2017
Merged
@kbrock kbrock mentioned this pull request Jun 6, 2017
Merged
@miq-bot
Copy link
Member

miq-bot commented Jun 6, 2017

Checked commit kbrock@331c289 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks fine. 🍪

@miq-bot miq-bot added the wip label Jun 6, 2017
@miq-bot
Copy link
Member

miq-bot commented Jun 12, 2017

This pull request is not mergeable. Please rebase and repush.

@Fryguy
Copy link
Member

Fryguy commented Jun 14, 2017

@kbrock Is there anything left for this, or have these been replaced by other PRs?

@kbrock
Copy link
Member Author

kbrock commented Jun 19, 2017

merged in individual requests

@kbrock kbrock closed this Jun 19, 2017
@kbrock kbrock deleted the miq_queue_updates_2 branch June 19, 2017 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fryguy Fryguy Awaiting requested review from Fryguy Fryguy is a code owner

Assignees

@Fryguy Fryguy

Labels
core refactoring unmergeable wip
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kbrock @miq-bot @Fryguy