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

Fix tier selection when using different units. #13593

Merged

Conversation

isimluk
Copy link
Member

@isimluk isimluk commented Jan 20, 2017

The problem was

Rates are tiered. Tier is selected based on the metrics' value.

When metric value and tier boundaries were in different units (metric in bytes, tiers in gigabytes), the tier selection was broken. We used / operator instead of *.

Cause

This problem existed since the day 0. It became just more visible after recent refactoring in #13331. I noticed that on one ocasion we had value * rate_adjustment and on another occasion we had value / rate_adjustment. That begged for investigation.

@miq-bot add_label chargeback, wip, bug
@miq-bot assign @gtanzillo

@miq-bot
Copy link
Member

miq-bot commented Jan 23, 2017

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

@isimluk isimluk force-pushed the fix-rate-adjustment-rounding-bug-2-wip branch from dfaa42f to 4656fac Compare January 23, 2017 04:26
@isimluk isimluk changed the title [WIP] Fix tier selection when using different units. Fix tier selection when using different units. Jan 23, 2017
@isimluk
Copy link
Member Author

isimluk commented Jan 23, 2017

@miq-bot remove_label wip, unmergeable

@isimluk isimluk force-pushed the fix-rate-adjustment-rounding-bug-2-wip branch from 4656fac to bb2a2cf Compare January 30, 2017 12:35
@isimluk isimluk force-pushed the fix-rate-adjustment-rounding-bug-2-wip branch from bb2a2cf to 84fa4e5 Compare February 14, 2017 08:43
@isimluk
Copy link
Member Author

isimluk commented Feb 14, 2017

Ping, any idea?

@miq-bot miq-bot added the wip label Feb 14, 2017
@miq-bot miq-bot changed the title Fix tier selection when using different units. [WIP] Fix tier selection when using different units. Feb 14, 2017
@miq-bot miq-bot changed the title [WIP] Fix tier selection when using different units. Fix tier selection when using different units. Feb 14, 2017
@miq-bot miq-bot removed the wip label Feb 14, 2017
@isimluk
Copy link
Member Author

isimluk commented Feb 14, 2017

@miq-bot what's up?

Let's have it really strict and check interval boundaries.
Interestingly, the math for tiers was broken from day zero. In most
cases however, rate_adjustoment=1, so this came unnoticed.

This makes the new test green again.
@isimluk isimluk force-pushed the fix-rate-adjustment-rounding-bug-2-wip branch from 84fa4e5 to 671628d Compare February 15, 2017 08:11
@miq-bot
Copy link
Member

miq-bot commented Feb 15, 2017

Checked commits isimluk/manageiq@9e8094e~...671628d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. ⭐

Copy link
Contributor

@lpichler lpichler left a comment

Choose a reason for hiding this comment

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

👍

@martinpovolny martinpovolny merged commit 7913926 into ManageIQ:master Feb 15, 2017
@isimluk
Copy link
Member Author

isimluk commented Feb 15, 2017

Thanks!!!

@martinpovolny martinpovolny added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 15, 2017
@isimluk isimluk deleted the fix-rate-adjustment-rounding-bug-2-wip branch February 15, 2017 10:11
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