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

[WIP] Move queue_capture to provider metric_capture_object #19492

Closed
wants to merge 1 commit into from

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Nov 12, 2019

extracted / Blocked:

Overview

  • Cap&U is a single workflow that is used by all providers.
  • It puts too many messages on the queue.
  • It doesn't pacing itself and relies upon put_or_update to avoid some of the duplicate work.
  • It takes hours to run because it hits the provider too many times, many of which are duplicate.
  • The cache in the broker alleviates some of the duplicate calls.

This PR is focused on queuing.

Before

  • Queuing capture was mixed into Vm, Host, and Storage models.
  • The different models and providers work differently, this was handled by if checks on the provider class.

After

  • Queuing capture is part of the provider's MetricCapture object
  • The different models and providers are still handles by if checks, but since there is now a hierarchy, they can use traditional inheritance.
  • zone, queue name, ems are only looked up once. This removes quite a few calls.

@Fryguy
Copy link
Member

Fryguy commented Nov 12, 2019

There's a lot of movement in this PR, so it's hard to see if this is just moving things around or if there are actual code changes along with the moves. Are there specific commits that help the review of this?

@kbrock kbrock changed the title Move queue_capture to provider metric_capture_object [WIP] Move queue_capture to provider metric_capture_object Nov 14, 2019
@kbrock kbrock added the wip label Nov 14, 2019
@kbrock kbrock force-pushed the cu_capture_object branch 2 times, most recently from 1c0175a to 0ea4676 Compare November 20, 2019 04:20
@kbrock kbrock force-pushed the cu_capture_object branch 2 times, most recently from 9a68a15 to 0470e92 Compare November 20, 2019 16:53
@kbrock
Copy link
Member Author

kbrock commented Nov 20, 2019

sigh. let me know if we want to handle these cops. I got it down to 2 which is good.

I just rebased and pushed another version with a lot more comments and organized the commits slightly differently.

Hoping the comments make it easier to merge.
Thought I'm guessing splitting this up may be the best way

@kbrock kbrock mentioned this pull request Nov 20, 2019
@kbrock kbrock changed the title [WIP] Move queue_capture to provider metric_capture_object Move queue_capture to provider metric_capture_object Nov 25, 2019
@kbrock kbrock removed the wip label Nov 25, 2019
@kbrock kbrock changed the title Move queue_capture to provider metric_capture_object [WIP] Move queue_capture to provider metric_capture_object Dec 6, 2019
@kbrock
Copy link
Member Author

kbrock commented Dec 6, 2019

wip: going to focus on tests first then will work on getting the remaining commits in

Instead of getting into the weeds about how granular our messages are,
we are just making sure all the time frames of interest are tested
@miq-bot
Copy link
Member

miq-bot commented Jan 8, 2020

Checked commit kbrock@b30db11 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@miq-bot miq-bot added the stale label Jun 11, 2020
@miq-bot
Copy link
Member

miq-bot commented Jun 11, 2020

This pull request has been automatically marked as stale because it has not been updated for at least 3 months.

If these changes are still valid, please remove the stale label, make any changes requested by reviewers (if any), and ensure that this issue is being looked at by the assigned/reviewer(s)

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the traige process documentation.

@gtanzillo gtanzillo removed the stale label Jul 20, 2020
@miq-bot
Copy link
Member

miq-bot commented Oct 31, 2022

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

@kbrock
Copy link
Member Author

kbrock commented Nov 1, 2022

Did all the work except for combining messages. Doing that in #20071 instead

@kbrock kbrock closed this Nov 1, 2022
@kbrock kbrock deleted the cu_capture_object branch November 1, 2022 14:48
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.

5 participants