[CLOUDSTACK-10039] Adding IOPS/GB offering by syed · Pull Request #2231 · apache/cloudstack

syed · 2017-08-08T20:55:00Z

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

We want to add a disk offering where we can specify the Min/Max IOPS as a function of size. The idea is the larger volume you create, the greater your IOPS will be (this is similar to the way GCE handles IOPS). We also have added limits to the IOPS (min and max) so that once the volume goes beyond a certain size, the IOPS won't change and are capped to the given values.

The following parameters are added to the createDiskOffering API:

`miniopspergb' : Minimum IOPS/GB for the offering
maxiopspergb: Maximum IOPS/GB for the offering
highestminiops: The highest miniops value that is allowed for this offering
highestmaxiops: The highest maxiops value that is allowed for this offering

wido · 2017-08-10T09:41:32Z

Could you resolve the conflict @syed ?

syed · 2017-08-15T17:11:59Z

@wido I've resolved the conflicts. I am still trying to see if I can get the DB changes to be idempotently added. However the way Cloudstack reads the db script is really weird. Right now its a WIP

cloudmonger · 2017-08-22T10:45:59Z

ACS CI BVT Run

Sumarry:
Build Number 1126
Hypervisor xenserver
NetworkType Advanced
Passed=109
Failed=5
Skipped=12

Link to logs Folder (search by build_no): https://www.dropbox.com/sh/r2si930m8xxzavs/AAAzNrnoF1fC3auFrvsKo_8-a?dl=0

Failed tests:

test_router_dnsservice.py
test_router_dns_guestipquery Failed
test_routers_network_ops.py
test_01_isolate_network_FW_PF_default_routes_egress_true Failing since 124 runs
test_02_isolate_network_FW_PF_default_routes_egress_false Failing since 124 runs
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true Failing since 120 runs
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false Failing since 120 runs

Skipped tests:
test_vm_nic_adapter_vmxnet3
test_01_verify_libvirt
test_02_verify_libvirt_after_restart
test_03_verify_libvirt_attach_disk
test_04_verify_guest_lspci
test_05_change_vm_ostype_restart
test_06_verify_guest_lspci_again
test_static_role_account_acls
test_11_ss_nfs_version_on_ssvm
test_nested_virtualization_vmware
test_3d_gpu_support
test_deploy_vgpu_enabled_vm

Passed test suits:
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_vm_snapshots.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_loadbalance.py
test_routers.py
test_reset_vm_on_reboot.py
test_deploy_vms_with_varied_deploymentplanners.py
test_network.py
test_router_dns.py
test_non_contigiousvlan.py
test_login.py
test_deploy_vm_iso.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_metrics_api.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_volumes.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_vm_life_cycle.py
test_disk_offerings.py

yadvr · 2017-08-31T07:47:47Z

@syed can you fix the conflicts?

yadvr · 2017-10-04T04:30:12Z

@syed ping

syed · 2017-10-04T17:30:01Z

@rhtyd Sorry Rohit, looks like this slipped out of my radar. I've rebased and resolved the conflicts.

yadvr · 2017-10-12T12:54:56Z

@syed looks like we have new conflicts

syed · 2017-10-12T14:11:49Z

darn it! I'll resolve it now

syed · 2017-10-12T15:41:53Z

@rhtyd Done!

bwsw · 2017-11-13T06:57:35Z

Cool feature. Recently have thought about it.

yadvr · 2018-01-03T08:27:45Z

@syed can you fix the conflicts?
Additional code review is also requested.

rafaelweingartner · 2018-02-22T23:12:52Z

Ping @syed

syed · 2018-02-23T16:51:50Z

@rafaelweingartner I've resolved the conflicts. I'd really like to get this in because I think this is the 3rd time rebasing it. If you could do a code review, that would be sublime :)

rafaelweingartner

@syed thanks for the PR, but we need to work on it.
Can you check some of my inquiries?

rafaelweingartner · 2018-02-23T17:08:47Z

+DROP PROCEDURE IF EXISTS `cloud`.`ADD_COLUMN_TO_TABLE_IDEMPOTENT`;
+
+DELIMITER //
+CREATE PROCEDURE `cloud`.`ADD_COLUMN_TO_TABLE_IDEMPOTENT`(


Isn't this being introduced by some other PR?

No. This is only for this PR

Hmm.. Then, I think it is better to coordinate with PR #2449.

rafaelweingartner · 2018-02-23T17:11:02Z

+               throw new InvalidParameterValueException("Cannot set Min/Max IOPS/GB for a fixed size disk offering");
+            }
+
+            if ((isCustomizedIops != null && isCustomizedIops) || minIops != null || maxIops != null) {


Can you use BooleanUtils here?

rafaelweingartner · 2018-02-23T17:11:47Z

+                        "custom IOPS or fixed IOPS");
+            }
+
+            if (minIopsPerGb != null && maxIopsPerGb != null) {


Could you extract this block to a specific method? Document it, and write unit tests for it?

rafaelweingartner · 2018-02-23T17:12:11Z

+            }
+        }
+
+        if (highestMinIops != null && highestMaxIops != null) {


This whole validation block can also be extracted to a method, documented and unit tested.

rafaelweingartner · 2018-02-23T17:16:56Z

                newSize = volume.getSize();
            }
-
+            newSizeInGb = newSize >> 30;


Let's not use bit shift here. It makes things harder to read and understand.

The code is already convoluted enough

rafaelweingartner · 2018-02-23T17:17:26Z

                newSize = newDiskOffering.getDiskSize();
            }

+            newSizeInGb = newSize >> 30;


bit shift...

Have you checked if there is a library to convert between these bases? If there is none, what about we create an utils method to execute this job?

rafaelweingartner · 2018-02-23T17:19:00Z

@@ -0,0 +1,324 @@
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements.  See the NOTICE file


these scripts should not be here, right?

bwsw · 2018-11-29T19:02:33Z

@syed Hi, do you continue to manage this PR? I think it's very beneficial for 4.12.

svenvogel · 2018-12-06T20:46:30Z

@syed is it useful for solidfire? Is there any progress in this pr?

syed · 2018-12-12T18:28:42Z

This is indeed useful for solidfire. No progress yet because we are waiting on the hardware on our lab to be setup and ready to use

…

On Thu, Dec 6, 2018, 12:46 PM Sven Vogel, ***@***.***> wrote: @syed <https://github.com/syed> is it useful for solidfire? Is there any progress in this pr? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2231 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADaLlT8S9ckTd1WE12pOXPf3mtZVxyaks5u2YIrgaJpZM4OxR9y> .

rafaelweingartner · 2019-01-15T18:57:48Z

@syed I will move this to 5.0.0. The freeze date is coming, and it does not seem that we will be able to add it to 4.12.0.

GabrielBrascher · 2020-01-29T15:27:28Z

@rhtyd @syed @DaanHoogland moving this to 4.15. Still pending review and fixing conflicting files.

GutoVeronezi · 2021-06-25T13:47:55Z

@syed this is an interesting feature; Do you intend to continue managing this PR?

GabrielBrascher · 2021-06-25T13:59:16Z

I think that this PR is no longer necessary due to PR adding support for IOPS burst #3133.

nvazquez · 2021-06-28T23:15:05Z

Hi @syed can you please confirm if this PR is still needed or can be closed by PR3133? In case it is, is it ready for review?

syed · 2021-07-14T12:55:01Z

Hi @nvazquez Sorry for the delayed response. #3313 is different from this PR. The idea here is to give the operator the ability to expose storage offerings in the style AWS gp2/gp3 where IOPS scales with size. I currently don't have a setup to work on this but would be happy to support if someone picks it up

nvazquez · 2021-07-16T15:28:47Z

Thanks @syed this will need some effort and resolving many conflicts. Do you think you can pick this up sometime soon? Otherwise we can move it for the next milestone

syed · 2021-07-16T18:24:04Z

Probably good idea to move to next milestone

…

On Fri, Jul 16, 2021 at 11:28 AM Nicolas Vazquez ***@***.***> wrote: Thanks @syed <https://github.com/syed> this will need some effort and resolving many conflicts. Do you think you can pick this up sometime soon? Otherwise we can move it for the next milestone — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2231 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANULU6WE5GA5WEDAFC66DTYBF3VANCNFSM4DWFD5ZA> .

yadvr · 2022-02-21T10:47:09Z

Hi @syed thank you for your PR, given the inactivity in the last 4+ years, I'm closing this. If you happen to work on this feature and can fix conflicts pl re-open or re-raise the PR.

kiwiflyer added the type:enhancement label Aug 8, 2017

syed force-pushed the disk-offering-iops-per-gig branch from d4ea9d1 to 4b94afb Compare August 15, 2017 17:07

syed force-pushed the disk-offering-iops-per-gig branch from 971decb to 2de3b1b Compare August 21, 2017 20:41

syed force-pushed the disk-offering-iops-per-gig branch from 2de3b1b to 12bd5e8 Compare October 4, 2017 17:22

syed force-pushed the disk-offering-iops-per-gig branch from 12bd5e8 to 047e83b Compare October 4, 2017 17:30

syed force-pushed the disk-offering-iops-per-gig branch from 047e83b to 455ecf1 Compare October 12, 2017 15:41

yadvr added this to the 4.11 milestone Dec 10, 2017

yadvr modified the milestones: 4.11, 4.12 Jan 6, 2018

[CLOUDSTACK-10039] Adding IOPS/GB offering

3c95160

syed force-pushed the disk-offering-iops-per-gig branch from 455ecf1 to 3c95160 Compare February 23, 2018 16:39

rafaelweingartner requested changes Feb 23, 2018

View reviewed changes

rafaelweingartner removed this from the 4.12.0.0 milestone Jan 15, 2019

rafaelweingartner added this to the 5.0.0.0 milestone Jan 15, 2019

yadvr removed this from the 4.13.0.0 milestone May 27, 2019

GabrielBrascher added this to the 4.15.0.0 milestone Jan 29, 2020

DaanHoogland marked this pull request as draft May 7, 2020 08:57

yadvr modified the milestones: 4.15.0.0, 4.16.0.0 Jul 30, 2020

ACSGitBot added status:wip age:2years_plus labels Nov 3, 2020

nvazquez modified the milestones: 4.16.0.0, 4.17.0.0, unplanned Jul 17, 2021

yadvr closed this Feb 21, 2022

		@@ -0,0 +1,324 @@
		-- Licensed to the Apache Software Foundation (ASF) under one
		-- or more contributor license agreements. See the NOTICE file

Conversation

syed commented Aug 8, 2017 • edited by PaulAngus Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

Uh oh!

wido commented Aug 10, 2017

Uh oh!

syed commented Aug 15, 2017

Uh oh!

cloudmonger commented Aug 22, 2017

ACS CI BVT Run

Uh oh!

yadvr commented Aug 31, 2017

Uh oh!

yadvr commented Oct 4, 2017

Uh oh!

syed commented Oct 4, 2017

Uh oh!

yadvr commented Oct 12, 2017

Uh oh!

syed commented Oct 12, 2017

Uh oh!

syed commented Oct 12, 2017

Uh oh!

bwsw commented Nov 13, 2017

Uh oh!

yadvr commented Jan 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rafaelweingartner commented Feb 22, 2018

Uh oh!

syed commented Feb 23, 2018

Uh oh!

rafaelweingartner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwsw commented Nov 29, 2018

Uh oh!

svenvogel commented Dec 6, 2018

Uh oh!

syed commented Dec 12, 2018 via email

Uh oh!

rafaelweingartner commented Jan 15, 2019

Uh oh!

GabrielBrascher commented Jan 29, 2020

Uh oh!

GutoVeronezi commented Jun 25, 2021

Uh oh!

GabrielBrascher commented Jun 25, 2021

Uh oh!

nvazquez commented Jun 28, 2021

Uh oh!

syed commented Jul 14, 2021

Uh oh!

nvazquez commented Jul 16, 2021

Uh oh!

syed commented Jul 16, 2021 via email

Uh oh!

yadvr commented Feb 21, 2022

Uh oh!

Reviewers

syed commented Aug 8, 2017 •

edited by PaulAngus

Loading

yadvr commented Jan 3, 2018 •

edited

Loading