You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Harikrishna Patnala <ha...@citrix.com> on 2013/07/10 12:14:08 UTC

Re: Review Request 12427: Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/
-----------------------------------------------------------

(Updated July 10, 2013, 10:14 a.m.)


Review request for cloudstack, Abhinandan Prateek and Koushik Das.


Changes
-------

Added fix for CLOUDSTACK-2536


Summary (updated)
-----------------

Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536


Bugs: CLOUDSTACK-2536 and CLOUDSTACK-3365


Repository: cloudstack-git


Description (updated)
-------

CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

CLOUDSTACK-2536:  parameters (cpu/memory)overcommit ratio and (cpu/memory).overprosioning.factor are redundant(cluster level)


Diffs (updated)
-----

  server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
  server/src/com/cloud/configuration/Config.java 1a2c620 
  server/src/com/cloud/vm/VirtualMachineManagerImpl.java 30ee2d7 
  server/test/com/cloud/vm/VirtualMachineManagerImplTest.java 44e22e2 

Diff: https://reviews.apache.org/r/12427/diff/


Testing
-------

Tested locally both with actual usage and over committed values for memory. 


Thanks,

Harikrishna Patnala


Re: Review Request 12427: Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/#review23492
-----------------------------------------------------------


The changes related to CLOUDSTACK-2536 is not required. updateCluster needs to be fixed as updateConfiguration is a generic approach and shouldn't be broken for handling a special scenario.

- Koushik Das


On July 17, 2013, 7:37 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12427/
> -----------------------------------------------------------
> 
> (Updated July 17, 2013, 7:37 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.
> 
> 
> Bugs: CLOUDSTACK-2536 and CLOUDSTACK-3365
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value
> 
> CLOUDSTACK-2536:  parameters (cpu/memory)overcommit ratio and (cpu/memory).overprosioning.factor are redundant(cluster level)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
>   server/src/com/cloud/configuration/Config.java e77f6ce 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java 827e233 
>   server/test/com/cloud/vm/VirtualMachineManagerImplTest.java 44e22e2 
> 
> Diff: https://reviews.apache.org/r/12427/diff/
> 
> 
> Testing
> -------
> 
> Tested locally both with actual usage and over committed values for memory. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request 12427: Fix for CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/#review23574
-----------------------------------------------------------


Commit 78f1092df12928f06646b3bc42d69cc45dd3b1d3 in branch refs/heads/master from Harikrishna Patnala
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=78f1092 ]

CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value


- ASF Subversion and Git Services


On July 19, 2013, 10:14 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12427/
> -----------------------------------------------------------
> 
> (Updated July 19, 2013, 10:14 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.
> 
> 
> Bugs: CLOUDSTACK-3365
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
> 
> Diff: https://reviews.apache.org/r/12427/diff/
> 
> 
> Testing
> -------
> 
> Tested locally both with actual usage and over committed values for memory. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request 12427: Fix for CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/#review23576
-----------------------------------------------------------

Ship it!


master -> 78f1092df12928f06646b3bc42d69cc45dd3b1d3
4.2 -> f604e3d9cc9073c8863bbd7dfc47fb6693a0b6f0

Currently in the db. for total capacity of a resource actual value is stored but for used capacity the value stored takes into account over provisioning factor. As a result of this for any comparison the total capacity needs to be re-calibrated in the code everywhere based on over provisioning factor. Please open a new bug for future to store used/total capacity values in db. based on same scale factor.

- Koushik Das


On July 19, 2013, 10:14 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12427/
> -----------------------------------------------------------
> 
> (Updated July 19, 2013, 10:14 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.
> 
> 
> Bugs: CLOUDSTACK-3365
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
> 
> Diff: https://reviews.apache.org/r/12427/diff/
> 
> 
> Testing
> -------
> 
> Tested locally both with actual usage and over committed values for memory. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request 12427: Fix for CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/#review23575
-----------------------------------------------------------


Commit f604e3d9cc9073c8863bbd7dfc47fb6693a0b6f0 in branch refs/heads/4.2 from Harikrishna Patnala
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=f604e3d ]

CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value


- ASF Subversion and Git Services


On July 19, 2013, 10:14 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12427/
> -----------------------------------------------------------
> 
> (Updated July 19, 2013, 10:14 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.
> 
> 
> Bugs: CLOUDSTACK-3365
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
> 
> Diff: https://reviews.apache.org/r/12427/diff/
> 
> 
> Testing
> -------
> 
> Tested locally both with actual usage and over committed values for memory. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request 12427: Fix for CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

Posted by Koushik Das <ko...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/#review23577
-----------------------------------------------------------

Ship it!


Ship It!

- Koushik Das


On July 19, 2013, 10:14 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12427/
> -----------------------------------------------------------
> 
> (Updated July 19, 2013, 10:14 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.
> 
> 
> Bugs: CLOUDSTACK-3365
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
> 
> Diff: https://reviews.apache.org/r/12427/diff/
> 
> 
> Testing
> -------
> 
> Tested locally both with actual usage and over committed values for memory. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request 12427: Fix for CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

Posted by Harikrishna Patnala <ha...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/
-----------------------------------------------------------

(Updated July 19, 2013, 10:14 a.m.)


Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.


Changes
-------

removed the fix for 2536.


Summary (updated)
-----------------

Fix for CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value


Bugs: CLOUDSTACK-3365


Repository: cloudstack-git


Description (updated)
-------

CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value


Diffs (updated)
-----

  server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 

Diff: https://reviews.apache.org/r/12427/diff/


Testing
-------

Tested locally both with actual usage and over committed values for memory. 


Thanks,

Harikrishna Patnala


Re: Review Request 12427: Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536

Posted by Harikrishna Patnala <ha...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/
-----------------------------------------------------------

(Updated July 17, 2013, 7:37 a.m.)


Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.


Changes
-------

Updated patch based on review comments


Bugs: CLOUDSTACK-2536 and CLOUDSTACK-3365


Repository: cloudstack-git


Description
-------

CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

CLOUDSTACK-2536:  parameters (cpu/memory)overcommit ratio and (cpu/memory).overprosioning.factor are redundant(cluster level)


Diffs (updated)
-----

  server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
  server/src/com/cloud/configuration/Config.java e77f6ce 
  server/src/com/cloud/vm/VirtualMachineManagerImpl.java 827e233 
  server/test/com/cloud/vm/VirtualMachineManagerImplTest.java 44e22e2 

Diff: https://reviews.apache.org/r/12427/diff/


Testing
-------

Tested locally both with actual usage and over committed values for memory. 


Thanks,

Harikrishna Patnala


Re: Review Request 12427: Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536

Posted by Harikrishna Patnala <ha...@citrix.com>.

> On July 17, 2013, 5:38 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/configuration/Config.java, line 202
> > <https://reviews.apache.org/r/12427/diff/3/?file=319814#file319814line202>
> >
> >     Please clarify - Are they not at cluster level anymore ?

These are cluster level parameters but can be modified only using UpdateCluster API since this needs some other updates not just DB change that we do in UpdateConfiguration.


> On July 17, 2013, 5:38 a.m., Nitin Mehta wrote:
> > server/src/com/cloud/vm/VirtualMachineManagerImpl.java, line 3243
> > <https://reviews.apache.org/r/12427/diff/3/?file=319815#file319815line3243>
> >
> >     What was wrong here...why not taking it from Config ?

Since we have removed the scope of these parameters from cluster. This method cant be used to get the value.


- Harikrishna


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/#review23239
-----------------------------------------------------------


On July 10, 2013, 10:56 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12427/
> -----------------------------------------------------------
> 
> (Updated July 10, 2013, 10:56 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.
> 
> 
> Bugs: CLOUDSTACK-2536 and CLOUDSTACK-3365
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value
> 
> CLOUDSTACK-2536:  parameters (cpu/memory)overcommit ratio and (cpu/memory).overprosioning.factor are redundant(cluster level)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
>   server/src/com/cloud/configuration/Config.java d3ed718 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java 30ee2d7 
>   server/test/com/cloud/vm/VirtualMachineManagerImplTest.java 44e22e2 
> 
> Diff: https://reviews.apache.org/r/12427/diff/
> 
> 
> Testing
> -------
> 
> Tested locally both with actual usage and over committed values for memory. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request 12427: Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536

Posted by Nitin Mehta <ni...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/#review23239
-----------------------------------------------------------



server/src/com/cloud/alert/AlertManagerImpl.java
<https://reviews.apache.org/r/12427/#comment47131>

    Write a function getOverProvisioningFactor(clusterId, capacityType)



server/src/com/cloud/alert/AlertManagerImpl.java
<https://reviews.apache.org/r/12427/#comment47132>

    capacityValue is misleading ---- call it threshhold



server/src/com/cloud/configuration/Config.java
<https://reviews.apache.org/r/12427/#comment47133>

    Please clarify - Are they not at cluster level anymore ?



server/src/com/cloud/vm/VirtualMachineManagerImpl.java
<https://reviews.apache.org/r/12427/#comment47135>

    What was wrong here...why not taking it from Config ?


- Nitin Mehta


On July 10, 2013, 10:56 a.m., Harikrishna Patnala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12427/
> -----------------------------------------------------------
> 
> (Updated July 10, 2013, 10:56 a.m.)
> 
> 
> Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.
> 
> 
> Bugs: CLOUDSTACK-2536 and CLOUDSTACK-3365
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value
> 
> CLOUDSTACK-2536:  parameters (cpu/memory)overcommit ratio and (cpu/memory).overprosioning.factor are redundant(cluster level)
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
>   server/src/com/cloud/configuration/Config.java d3ed718 
>   server/src/com/cloud/vm/VirtualMachineManagerImpl.java 30ee2d7 
>   server/test/com/cloud/vm/VirtualMachineManagerImplTest.java 44e22e2 
> 
> Diff: https://reviews.apache.org/r/12427/diff/
> 
> 
> Testing
> -------
> 
> Tested locally both with actual usage and over committed values for memory. 
> 
> 
> Thanks,
> 
> Harikrishna Patnala
> 
>


Re: Review Request 12427: Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536

Posted by Harikrishna Patnala <ha...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/
-----------------------------------------------------------

(Updated July 10, 2013, 10:56 a.m.)


Review request for cloudstack, Abhinandan Prateek, Koushik Das, and Nitin Mehta.


Bugs: CLOUDSTACK-2536 and CLOUDSTACK-3365


Repository: cloudstack-git


Description
-------

CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

CLOUDSTACK-2536:  parameters (cpu/memory)overcommit ratio and (cpu/memory).overprosioning.factor are redundant(cluster level)


Diffs
-----

  server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
  server/src/com/cloud/configuration/Config.java d3ed718 
  server/src/com/cloud/vm/VirtualMachineManagerImpl.java 30ee2d7 
  server/test/com/cloud/vm/VirtualMachineManagerImplTest.java 44e22e2 

Diff: https://reviews.apache.org/r/12427/diff/


Testing
-------

Tested locally both with actual usage and over committed values for memory. 


Thanks,

Harikrishna Patnala


Re: Review Request 12427: Fix for CLOUDSTACK-3365 and CLOUDSTACK-2536

Posted by Harikrishna Patnala <ha...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12427/
-----------------------------------------------------------

(Updated July 10, 2013, 10:19 a.m.)


Review request for cloudstack, Abhinandan Prateek and Koushik Das.


Bugs: CLOUDSTACK-2536 and CLOUDSTACK-3365


Repository: cloudstack-git


Description
-------

CLOUDSTACK-3365: cluster level parameters cluster.(cpu/memory).allocated.capacity.notificationthreshold is not considering overcommit value

CLOUDSTACK-2536:  parameters (cpu/memory)overcommit ratio and (cpu/memory).overprosioning.factor are redundant(cluster level)


Diffs (updated)
-----

  server/src/com/cloud/alert/AlertManagerImpl.java 9b7cd27 
  server/src/com/cloud/configuration/Config.java d3ed718 
  server/src/com/cloud/vm/VirtualMachineManagerImpl.java 30ee2d7 
  server/test/com/cloud/vm/VirtualMachineManagerImplTest.java 44e22e2 

Diff: https://reviews.apache.org/r/12427/diff/


Testing
-------

Tested locally both with actual usage and over committed values for memory. 


Thanks,

Harikrishna Patnala