cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Tutkowski <mike.tutkow...@solidfire.com>
Subject Re: [MERGE] disk_io_throttling to MASTER
Date Wed, 12 Jun 2013 16:43:29 GMT
No problem, John.

I still want to re-review it by myself before coming up with a new patch
file.

Also, maybe I should first wait for Wei's changes to be checked in and
merge those into mine before generating a new patch file?


On Wed, Jun 12, 2013 at 10:40 AM, John Burwell <jburwell@basho.com> wrote:

> Mike,
>
> I just realized that I forgot to publish my review.  I am offline ATM,
> but I will publish it in the next couple of hours.
>
> Do you plan to update your the patch in Review Board?
>
> Sorry for the oversight,
> -John
>
>
>
>
> On Jun 12, 2013, at 2:26 AM, Mike Tutkowski
> <mike.tutkowski@solidfire.com> wrote:
>
> > Hi Edison, John, and Wei (and whoever else is reading this :) ),
> >
> > Just an FYI that I believe I have implemented all the areas we wanted
> > addressed.
> >
> > I plan to review the code again tomorrow morning or afternoon, then send
> in
> > another patch.
> >
> > Thanks for all the work on this everyone!
> >
> >
> > On Tue, Jun 11, 2013 at 12:29 PM, Mike Tutkowski <
> > mike.tutkowski@solidfire.com> wrote:
> >
> >> Sure, that sounds good.
> >>
> >>
> >> On Tue, Jun 11, 2013 at 12:11 PM, Wei ZHOU <ustcweizhou@gmail.com>
> wrote:
> >>
> >>> Hi Mike,
> >>>
> >>> It looks the two feature do not have many conflicts in Java code,
> except
> >>> the cloudstack UI.
> >>> If you do not mind, I will merge disk_io_throttling branch into master
> >>> this
> >>> week, so that you can develop based on it.
> >>>
> >>> -Wei
> >>>
> >>>
> >>> 2013/6/11 Mike Tutkowski <mike.tutkowski@solidfire.com>
> >>>
> >>>> Hey John,
> >>>>
> >>>> The SolidFire patch does not depend on the object_store branch, but
-
> as
> >>>> Edison mentioned - it might be easier if we merge the SolidFire branch
> >>> into
> >>>> the object_store branch before object_store goes into master.
> >>>>
> >>>> I'm not sure how the disk_io_throttling fits into this merge strategy.
> >>>> Perhaps Wei can chime in on that.
> >>>>
> >>>>
> >>>> On Tue, Jun 11, 2013 at 11:07 AM, John Burwell <jburwell@basho.com>
> >>> wrote:
> >>>>
> >>>>> Mike,
> >>>>>
> >>>>> We have a delicate merge dance to perform.  The disk_io_throttling,
> >>>>> solidfire, and object_store appear to have a number of overlapping
> >>>>> elements.  I understand the dependencies between the patches to
be as
> >>>>> follows:
> >>>>>
> >>>>>        object_store <- solidfire -> disk_io_throttling
> >>>>>
> >>>>> Am I correct that the device management aspects of SolidFire are
> >>> additive
> >>>>> to the object_store branch or there are circular dependency between
> >>> the
> >>>>> branches?  Once we understand the dependency graph, we can determine
> >>> the
> >>>>> best approach to land the changes in master.
> >>>>>
> >>>>> Thanks,
> >>>>> -John
> >>>>>
> >>>>>
> >>>>> On Jun 10, 2013, at 11:10 PM, Mike Tutkowski <
> >>>> mike.tutkowski@solidfire.com>
> >>>>> wrote:
> >>>>>
> >>>>>> Also, if we are good with Edison merging my code into his branch
> >>> before
> >>>>>> going into master, I am good with that.
> >>>>>>
> >>>>>> We can remove the StoragePoolType.Dynamic code after his merge
and
> >>> we
> >>>> can
> >>>>>> deal with Burst IOPS then, as well.
> >>>>>>
> >>>>>>
> >>>>>> On Mon, Jun 10, 2013 at 9:08 PM, Mike Tutkowski <
> >>>>>> mike.tutkowski@solidfire.com> wrote:
> >>>>>>
> >>>>>>> Let me make sure I follow where we're going here:
> >>>>>>>
> >>>>>>> 1) There should be NO references to hypervisor code in the
storage
> >>>>>>> plug-ins code (this includes the default storage plug-in,
which
> >>>>> currently
> >>>>>>> sends several commands to the hypervisor in use (although
it does
> >>> not
> >>>>> know
> >>>>>>> which hypervisor (XenServer, ESX(i), etc.) is actually in
use))
> >>>>>>>
> >>>>>>> 2) managed=true or managed=false can be placed in the url
field (if
> >>>> not
> >>>>>>> present, we default to false). This info is stored in the
> >>>>>>> storage_pool_details table.
> >>>>>>>
> >>>>>>> 3) When the "attach" command is sent to the hypervisor in
> >>> question, we
> >>>>>>> pass the managed property along (this takes the place of
the
> >>>>>>> StoragePoolType.Dynamic check).
> >>>>>>>
> >>>>>>> 4) execute(AttachVolumeCommand) in the hypervisor checks
for the
> >>>> managed
> >>>>>>> property. If true for an attach, the necessary hypervisor
data
> >>>>> structure is
> >>>>>>> created and the rest of the attach command executes to attach
the
> >>>>> volume.
> >>>>>>>
> >>>>>>> 5) When execute(AttachVolumeCommand) is invoked to detach
a volume,
> >>>> the
> >>>>>>> same check is made. If managed, the hypervisor data structure
is
> >>>>> removed.
> >>>>>>>
> >>>>>>> 6) I do not see an clear way to support Burst IOPS in 4.2
unless
> >>> it is
> >>>>>>> stored in the volumes and disk_offerings table. If we have
some
> >>> idea,
> >>>>>>> that'd be cool.
> >>>>>>>
> >>>>>>> Thanks!
> >>>>>>>
> >>>>>>>
> >>>>>>> On Mon, Jun 10, 2013 at 8:58 PM, Mike Tutkowski <
> >>>>>>> mike.tutkowski@solidfire.com> wrote:
> >>>>>>>
> >>>>>>>> "+1 -- Burst IOPS can be implemented while avoiding
implementation
> >>>>>>>> attributes.  I always wondered about the details field.
 I think
> >>> we
> >>>>> should
> >>>>>>>> beef up the description in the documentation regarding
the
> >>> expected
> >>>>> format
> >>>>>>>> of the field.  In 4.1, I noticed that the details are
not
> >>> returned on
> >>>>> the
> >>>>>>>> createStoratePool updateStoragePool, or listStoragePool
response.
> >>>> Why
> >>>>>>>> don't we return it?  It seems like it would be useful
for clients
> >>> to
> >>>> be
> >>>>>>>> able to inspect the contents of the details field."
> >>>>>>>>
> >>>>>>>> Not sure how this would work storing Burst IOPS here.
> >>>>>>>>
> >>>>>>>> Burst IOPS need to be variable on a Disk Offering-by-Disk
Offering
> >>>>>>>> basis. For each Disk Offering created, you have to be
able to
> >>>> associate
> >>>>>>>> unique Burst IOPS. There is a disk_offering_details
table. Maybe
> >>> it
> >>>>> could
> >>>>>>>> go there?
> >>>>>>>>
> >>>>>>>> I'm also not sure how you would accept the Burst IOPS
in the GUI
> >>> if
> >>>>> it's
> >>>>>>>> not stored like the Min and Max fields are in the DB.
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> *Mike Tutkowski*
> >>>>>>> *Senior CloudStack Developer, SolidFire Inc.*
> >>>>>>> e: mike.tutkowski@solidfire.com
> >>>>>>> o: 303.746.7302
> >>>>>>> Advancing the way the world uses the cloud<
> >>>>> http://solidfire.com/solution/overview/?video=play>
> >>>>>>> *™*
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> *Mike Tutkowski*
> >>>>>> *Senior CloudStack Developer, SolidFire Inc.*
> >>>>>> e: mike.tutkowski@solidfire.com
> >>>>>> o: 303.746.7302
> >>>>>> Advancing the way the world uses the
> >>>>>> cloud<http://solidfire.com/solution/overview/?video=play>
> >>>>>> *™*
> >>>>
> >>>>
> >>>> --
> >>>> *Mike Tutkowski*
> >>>> *Senior CloudStack Developer, SolidFire Inc.*
> >>>> e: mike.tutkowski@solidfire.com
> >>>> o: 303.746.7302
> >>>> Advancing the way the world uses the
> >>>> cloud<http://solidfire.com/solution/overview/?video=play>
> >>>> *™*
> >>
> >>
> >>
> >> --
> >> *Mike Tutkowski*
> >> *Senior CloudStack Developer, SolidFire Inc.*
> >> e: mike.tutkowski@solidfire.com
> >> o: 303.746.7302
> >> Advancing the way the world uses the cloud<
> http://solidfire.com/solution/overview/?video=play>
> >> *™*
> >
> >
> >
> > --
> > *Mike Tutkowski*
> > *Senior CloudStack Developer, SolidFire Inc.*
> > e: mike.tutkowski@solidfire.com
> > o: 303.746.7302
> > Advancing the way the world uses the
> > cloud<http://solidfire.com/solution/overview/?video=play>
> > *™*
>



-- 
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*™*

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message