You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Ilia Shakitko <i....@tech.leaseweb.com> on 2014/11/13 10:48:28 UTC
Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/
-----------------------------------------------------------
Review request for cloudstack, Rohit Yadav and Wei Zhou.
Bugs: CLOUDSTACK-7903
https://issues.apache.org/jira/browse/CLOUDSTACK-7903
Repository: cloudstack-git
Description
-------
When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes usage samples instead of 10 (which is minimum allowed in ACS at the moment).
This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter.
Will be happy if patch can be included to the 4.4.2 tag.
Diffs
-----
usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
Diff: https://reviews.apache.org/r/27979/diff/
Testing
-------
Tested on both 4.4 and master branches.
Thanks,
Ilia Shakitko
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review62753
-----------------------------------------------------------
@Kishan can you review this?
- Rohit Yadav
On Nov. 18, 2014, 1:44 p.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2014, 1:44 p.m.)
>
>
> Review request for cloudstack, Kishan Kavala, Rohit Yadav, Simon Fiddaman, Stephen Hoogendijk, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes (or 1-minute) usage samples instead of 10 (which is minimum allowed in ACS at the moment). Let admin to control it.
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter, making it minimum possible positive integer. It is also removes declaration of the same constant in 3 places.
>
> Will be happy if patch can be:
> - cherry-picked to the 4.4 and 4.5 branches
> - included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 23ddfb1
> server/src/com/cloud/server/StatsCollector.java 1fcaa62
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on master, 4.4 and 4.5 branches. But then during the review time I had to re-prepare the patch and it become compatible only for master branch.
>
> If you accept the patch, could you also cherry-pick my commit to 4.4 and 4.5 branches? (there will be 1 one-line conflict to resolve)
>
>
> Thanks,
>
> Ilia Shakitko
>
>
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Simon Fiddaman <si...@fiddaman.nl>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review61917
-----------------------------------------------------------
Ship it!
Ship It!
- Simon Fiddaman
On Nov. 18, 2014, 1:44 p.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2014, 1:44 p.m.)
>
>
> Review request for cloudstack, Kishan Kavala, Rohit Yadav, Simon Fiddaman, Stephen Hoogendijk, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes (or 1-minute) usage samples instead of 10 (which is minimum allowed in ACS at the moment). Let admin to control it.
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter, making it minimum possible positive integer. It is also removes declaration of the same constant in 3 places.
>
> Will be happy if patch can be:
> - cherry-picked to the 4.4 and 4.5 branches
> - included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 23ddfb1
> server/src/com/cloud/server/StatsCollector.java 1fcaa62
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on master, 4.4 and 4.5 branches. But then during the review time I had to re-prepare the patch and it become compatible only for master branch.
>
> If you accept the patch, could you also cherry-pick my commit to 4.4 and 4.5 branches? (there will be 1 one-line conflict to resolve)
>
>
> Thanks,
>
> Ilia Shakitko
>
>
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review63348
-----------------------------------------------------------
Ship it!
I'm going ahead with the merge on master, 4.5 and 4.4 and 4.3.
This is a very simple patch that lowers the minimum threshold from 10 mins to 1 mins for usage interval, I was only waiting for Kishan's review but we've waited for several weeks on this.
- Rohit Yadav
On Nov. 25, 2014, 7:25 a.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 25, 2014, 7:25 a.m.)
>
>
> Review request for cloudstack, Chip Childers, Kishan Kavala, Rohit Yadav, Simon Fiddaman, Stephen Hoogendijk, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes (or 1-minute) usage samples instead of 10 (which is minimum allowed in ACS at the moment). Let admin to control it.
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter, making it minimum possible positive integer. It is also removes declaration of the same constant in 3 places.
>
> Will be happy if patch can be:
> - cherry-picked to the 4.4 and 4.5 branches
> - included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 23ddfb1
> server/src/com/cloud/server/StatsCollector.java 1fcaa62
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on master, 4.4 and 4.5 branches. But then during the review time I had to re-prepare the patch and it become compatible only for master branch.
>
> If you accept the patch, could you also cherry-pick my commit to 4.4 and 4.5 branches? (there will be 1 one-line conflict to resolve)
>
>
> Thanks,
>
> Ilia Shakitko
>
>
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Ilia Shakitko <i....@tech.leaseweb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/
-----------------------------------------------------------
(Updated Nov. 25, 2014, 7:25 a.m.)
Review request for cloudstack, Chip Childers, Kishan Kavala, Rohit Yadav, Simon Fiddaman, Stephen Hoogendijk, and Wei Zhou.
Bugs: CLOUDSTACK-7903
https://issues.apache.org/jira/browse/CLOUDSTACK-7903
Repository: cloudstack-git
Description
-------
When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes (or 1-minute) usage samples instead of 10 (which is minimum allowed in ACS at the moment). Let admin to control it.
This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter, making it minimum possible positive integer. It is also removes declaration of the same constant in 3 places.
Will be happy if patch can be:
- cherry-picked to the 4.4 and 4.5 branches
- included to the 4.4.2 tag.
Diffs
-----
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 23ddfb1
server/src/com/cloud/server/StatsCollector.java 1fcaa62
usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
Diff: https://reviews.apache.org/r/27979/diff/
Testing
-------
Tested on master, 4.4 and 4.5 branches. But then during the review time I had to re-prepare the patch and it become compatible only for master branch.
If you accept the patch, could you also cherry-pick my commit to 4.4 and 4.5 branches? (there will be 1 one-line conflict to resolve)
Thanks,
Ilia Shakitko
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Ilia Shakitko <i....@tech.leaseweb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/
-----------------------------------------------------------
(Updated Nov. 18, 2014, 1:44 p.m.)
Review request for cloudstack, Kishan Kavala, Rohit Yadav, Simon Fiddaman, Stephen Hoogendijk, and Wei Zhou.
Changes
-------
1. Maintained latest changed in master.
2. Removed constant declaration duplication, and decreased the limit (as suggested in feedback) to 1.
Bugs: CLOUDSTACK-7903
https://issues.apache.org/jira/browse/CLOUDSTACK-7903
Repository: cloudstack-git
Description (updated)
-------
When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes (or 1-minute) usage samples instead of 10 (which is minimum allowed in ACS at the moment). Let admin to control it.
This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter, making it minimum possible positive integer. It is also removes declaration of the same constant in 3 places.
Will be happy if patch can be:
- cherry-picked to the 4.4 and 4.5 branches
- included to the 4.4.2 tag.
Diffs (updated)
-----
server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 23ddfb1
server/src/com/cloud/server/StatsCollector.java 1fcaa62
usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
Diff: https://reviews.apache.org/r/27979/diff/
Testing (updated)
-------
Tested on master, 4.4 and 4.5 branches. But then during the review time I had to re-prepare the patch and it become compatible only for master branch.
If you accept the patch, could you also cherry-pick my commit to 4.4 and 4.5 branches? (there will be 1 one-line conflict to resolve)
Thanks,
Ilia Shakitko
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Simon Fiddaman <si...@fiddaman.nl>.
> On Nov. 13, 2014, 10:11 a.m., Rohit Yadav wrote:
> > I've added Kishan to this as well. Though it looks good and we can surely accept it. I think it makes sense to either set it to 1 or have a global configuration added which makes it easy for sysadmins to set a value they want instead of hard coding it.
This request is to change the minimum value that another (already configurable) value can be set to. Why not simply enforce USAGE_AGGREGATION_RANGE to integer type and abolish the USAGE_AGGREGATION_RANGE_MIN parameter (thus the minimum value for USAGE_AGGREGATION_RANGE is 1)? A note could even be added to the USAGE_AGGREGATION_RANGE configuration entry point stating this will result in 1440/USAGE_AGGREGATION_RANGE usage records per day entered in the Usage DB for each Network.
- Simon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review61224
-----------------------------------------------------------
On Nov. 13, 2014, 10:09 a.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2014, 10:09 a.m.)
>
>
> Review request for cloudstack, Kishan Kavala, Rohit Yadav, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes usage samples instead of 10 (which is minimum allowed in ACS at the moment).
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter.
>
> Will be happy if patch can be included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on both 4.4 and master branches.
>
>
> Thanks,
>
> Ilia Shakitko
>
>
RE: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Ilia Shakitko <i....@tech.leaseweb.com>.
True that,
I was thinking about setting that to 1, but then wasn't sure my colleagues from ACS will accept that. Because having min value=1 will bring hell a lot more usage records as a result :)
But if it will make sense, I'll re-prepare the patch.
Let's see what Kishan's opinion is ...
Kind regards,
Ilia Shakitko
Innovation Engineer
LeaseWeb Technologies B.V.
T: +31 20 316 0235
M:
E: i.shakitko@tech.leaseweb.com
W: http://www.leaseweb.com
Luttenbergweg 8, 1101 EC Amsterdam, Netherlands
-----Original Message-----
From: Rohit Yadav [mailto:noreply@reviews.apache.org] On Behalf Of Rohit Yadav
Sent: donderdag, 13 november, 2014 11:11
To: Kishan Kavala; Wei Zhou; Rohit Yadav
Cc: Ilia Shakitko; cloudstack
Subject: Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage aggregation range value
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review61224
-----------------------------------------------------------
I've added Kishan to this as well. Though it looks good and we can surely accept it. I think it makes sense to either set it to 1 or have a global configuration added which makes it easy for sysadmins to set a value they want instead of hard coding it.
- Rohit Yadav
On Nov. 13, 2014, 10:09 a.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2014, 10:09 a.m.)
>
>
> Review request for cloudstack, Kishan Kavala, Rohit Yadav, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes usage samples instead of 10 (which is minimum allowed in ACS at the moment).
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter.
>
> Will be happy if patch can be included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on both 4.4 and master branches.
>
>
> Thanks,
>
> Ilia Shakitko
>
>
RE: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Ilia Shakitko <i....@tech.leaseweb.com>.
Hi Rohit,
It's a week almost w/o an answer from Kishan. Meanwhile, I've redone the patch, removing some duplications (found 2 more places with same constant declared). And decreased the limit to 1 (minimal logical range limit). So it was a good feedback from you and Simon to actually let admins to decide. Maybe I don't need less than 5, but someone will need 1 or 2.
Could you please review it? Would be also cool to apply it to 4.4 and 4.5 (I wrote a remark in the review).
Kind regards,
Ilia Shakitko
Innovation Engineer
LeaseWeb Technologies B.V.
T: +31 20 316 0235
M:
E: i.shakitko@tech.leaseweb.com
W: http://www.leaseweb.com
Luttenbergweg 8, 1101 EC Amsterdam, Netherlands
-----Original Message-----
From: Rohit Yadav [mailto:noreply@reviews.apache.org] On Behalf Of Rohit Yadav
Sent: donderdag, 13 november, 2014 11:11
To: Kishan Kavala; Wei Zhou; Rohit Yadav
Cc: Ilia Shakitko; cloudstack
Subject: Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage aggregation range value
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review61224
-----------------------------------------------------------
I've added Kishan to this as well. Though it looks good and we can surely accept it. I think it makes sense to either set it to 1 or have a global configuration added which makes it easy for sysadmins to set a value they want instead of hard coding it.
- Rohit Yadav
On Nov. 13, 2014, 10:09 a.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2014, 10:09 a.m.)
>
>
> Review request for cloudstack, Kishan Kavala, Rohit Yadav, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes usage samples instead of 10 (which is minimum allowed in ACS at the moment).
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter.
>
> Will be happy if patch can be included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on both 4.4 and master branches.
>
>
> Thanks,
>
> Ilia Shakitko
>
>
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Rohit Yadav <bh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review61224
-----------------------------------------------------------
I've added Kishan to this as well. Though it looks good and we can surely accept it. I think it makes sense to either set it to 1 or have a global configuration added which makes it easy for sysadmins to set a value they want instead of hard coding it.
- Rohit Yadav
On Nov. 13, 2014, 10:09 a.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2014, 10:09 a.m.)
>
>
> Review request for cloudstack, Kishan Kavala, Rohit Yadav, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes usage samples instead of 10 (which is minimum allowed in ACS at the moment).
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter.
>
> Will be happy if patch can be included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on both 4.4 and master branches.
>
>
> Thanks,
>
> Ilia Shakitko
>
>
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Ilia Shakitko <i....@tech.leaseweb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/#review61226
-----------------------------------------------------------
Ship it!
Ship It!
- Ilia Shakitko
On Nov. 13, 2014, 10:09 a.m., Ilia Shakitko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27979/
> -----------------------------------------------------------
>
> (Updated Nov. 13, 2014, 10:09 a.m.)
>
>
> Review request for cloudstack, Kishan Kavala, Rohit Yadav, and Wei Zhou.
>
>
> Bugs: CLOUDSTACK-7903
> https://issues.apache.org/jira/browse/CLOUDSTACK-7903
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes usage samples instead of 10 (which is minimum allowed in ACS at the moment).
> This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter.
>
> Will be happy if patch can be included to the 4.4.2 tag.
>
>
> Diffs
> -----
>
> usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
>
> Diff: https://reviews.apache.org/r/27979/diff/
>
>
> Testing
> -------
>
> Tested on both 4.4 and master branches.
>
>
> Thanks,
>
> Ilia Shakitko
>
>
Re: Review Request 27979: CLOUDSTACK-7903: Decrease minimal usage
aggregation range value
Posted by Ilia Shakitko <i....@tech.leaseweb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27979/
-----------------------------------------------------------
(Updated Nov. 13, 2014, 10:09 a.m.)
Review request for cloudstack, Kishan Kavala, Rohit Yadav, and Wei Zhou.
Bugs: CLOUDSTACK-7903
https://issues.apache.org/jira/browse/CLOUDSTACK-7903
Repository: cloudstack-git
Description
-------
When you want to build 95-percentile billing model for the network traffic usage, it is good to have 5-minutes usage samples instead of 10 (which is minimum allowed in ACS at the moment).
This change decreases the lower limit for USAGE_AGGREGATION_RANGE_MIN parameter.
Will be happy if patch can be included to the 4.4.2 tag.
Diffs
-----
usage/src/com/cloud/usage/UsageManagerImpl.java fe7ee1e
Diff: https://reviews.apache.org/r/27979/diff/
Testing
-------
Tested on both 4.4 and master branches.
Thanks,
Ilia Shakitko