blueprint: consistency groups

blueprint: consistency groups

[Victor Denisov]

Hi Ceph developers,

With this email I would like to start a discussion about the design of
the consistency groups feature.

I created a wiki page for this blueprint:
http://tracker.ceph.com/projects/ceph/wiki/consistency_groups

So far it's only filled with what I know for sure about the
consistency groups feature.

First of all this feature is going to require a new entity in rbd -
consistency group.

This entity will be represented in the object storage in the way
similar to how it's done for images.

cg_id.<group name> object has the id of the consistency group.

cg_header.<group id> object has the list of all images in the
consistency group.

Snapshot information will be stored in the omap attributes of
cg_header.

Every description of a snapshot will contain the list of snapshot ids
of the images that comprise the cg's snapshot.

Potentially, I assume, we want to allow images to be members of
several consistency groups. However it can add complexity to the
implementation, see the explanation down the text.

The operations that we should be able to perform on consistency groups are:

   * Create a consistency group

   * Show a consistency group

   * List all available consistency groups

   * Add an image to a consistency group

   * Create a snapshot of all the members of a consistency group
     - Created snapshot of a consistency group only has references to
       snapshots that appear under the corresponding images

   * List all snapshots of a consistency group

   * Delete consistency group's snapshot

   * Modify consistency group (delete, add images to the consistency group)

   * Enable journaling feature for the whole consistency group.

Enabling journaling feature for the whole consistency group will
create one journal for all images in the group. All io operations to
the images in the group will go to this journal. Every operation will
have a tag identifying what image this operation is for. This feature
is required for rbd mirroring.

In order to make it compatible with cinder consistency groups feature
the functionality should cover Cinder Driver API:

def create_consistencygroup(self, context, consistencygroup, volumes)
def delete_consistencygroup(self, context, consistencygroup)
def create_cgsnapshot(self, context, cgsnapshot)
def delete_cgsnapshot(self, context, cgsnapshot)

https://github.com/openstack/cinder-specs/blob/master/specs/juno/consistency-groups.rst

The biggest question regarding the implementation comes from the live
snapshot feature. We believe constistent snapshot is not possible
without cooperation with rbd client (VM, application), because only
application knows if it is in constistent state for snapshotting.  In
the first iteration we would like to make it simple, so CG snapshot
operation just creates snapshots for all images and stores the
snapshot list in its metadata. The consistensy should be provided by
the user, who should ensure that application is in consistent point
(completed all transactions, stopped writting and flushed) during
snapshotting.

Re: blueprint: consistency groups

[Gregory Farnum]

On Thu, Mar 24, 2016 at 8:50 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Hi Ceph developers,
>
> With this email I would like to start a discussion about the design of
> the consistency groups feature.
>
> I created a wiki page for this blueprint:
> http://tracker.ceph.com/projects/ceph/wiki/consistency_groups
>
> So far it's only filled with what I know for sure about the
> consistency groups feature.
>
> First of all this feature is going to require a new entity in rbd -
> consistency group.
>
> This entity will be represented in the object storage in the way
> similar to how it's done for images.
>
> cg_id.<group name> object has the id of the consistency group.
>
> cg_header.<group id> object has the list of all images in the
> consistency group.
>
> Snapshot information will be stored in the omap attributes of
> cg_header.
>
> Every description of a snapshot will contain the list of snapshot ids
> of the images that comprise the cg's snapshot.
>
> Potentially, I assume, we want to allow images to be members of
> several consistency groups. However it can add complexity to the
> implementation, see the explanation down the text.

I think your design is going to be driven more by the needs and
choices of the rbd journaling, but given that, there's very little
reason you can't allow an image to be in more than one CG. Presumably
clients need to set a watch on each CG their image is part of, and to
apply every new snap to the image header. That's about it and not
terribly complicated?

<snip>

> The biggest question regarding the implementation comes from the live
> snapshot feature. We believe constistent snapshot is not possible
> without cooperation with rbd client (VM, application), because only
> application knows if it is in constistent state for snapshotting.  In
> the first iteration we would like to make it simple, so CG snapshot
> operation just creates snapshots for all images and stores the
> snapshot list in its metadata.

Just to be clear, you know that you can use the same snapid for all
the images, right? (...although not if you're trying to allow CGs
across pools. Those will be a little harder.)
-Greg

Re: blueprint: consistency groups

[Mykola Golub]

On Thu, Mar 24, 2016 at 03:38:25PM -0700, Gregory Farnum wrote:

> Just to be clear, you know that you can use the same snapid for all
> the images, right? (...although not if you're trying to allow CGs
> across pools. Those will be a little harder.)

I think not allowing CGs across pools would be annoying limitation for
users. If a VM has several volumes for different purposes (e.g. small
but fast volume for applicatin data, and large but slow one for
backups) it is logical to have different pools for those.

-- 
Mykola Golub

Re: blueprint: consistency groups

[Gregory Farnum]

On Fri, Mar 25, 2016 at 12:19 AM, Mykola Golub <mgolub@mirantis.com> wrote:
> On Thu, Mar 24, 2016 at 03:38:25PM -0700, Gregory Farnum wrote:
>
>> Just to be clear, you know that you can use the same snapid for all
>> the images, right? (...although not if you're trying to allow CGs
>> across pools. Those will be a little harder.)
>
> I think not allowing CGs across pools would be annoying limitation for
> users. If a VM has several volumes for different purposes (e.g. small
> but fast volume for applicatin data, and large but slow one for
> backups) it is logical to have different pools for those.

This isn't impossible either. We do it with cache pools and backing
pools sharing a set of snapids, for instance. But the default rules
won't work, since a snapid is just a 64-bit counter and they're
per-pool rather than cluster-global.
-Greg

Re: blueprint: consistency groups

[Victor Denisov]

Hi,

I have a question regarding implementation of the 'add image to
consistency group' operation.
Since it's a multi object operation while I'm adding an image
reference to a consistency group the image itself can be deleted.
By the time I start adding consistency group reference to the image,
the image will be gone.
So, I have to keep track of my current state of the operation in the
consistency group.
However if I the librbd client looses connection to the ceph cluster
it all will end up in this partially updated state.
It's not clear to who is responsible for picking up this state and
cleaning it all up.

Should it be a special operation that allows us to check consistency
of all consistency groups?
Is there any agent that is supposed to verify consistency of all rbd
objects regularly?

Please advise.

Thanks in advance,
Victor.

P.S. Here is a link to the ether pad with the updated blue print:
http://pad.ceph.com/p/consistency_groups

On Tue, Mar 29, 2016 at 10:54 AM, Gregory Farnum <gfarnum@redhat.com> wrote:
> On Fri, Mar 25, 2016 at 12:19 AM, Mykola Golub <mgolub@mirantis.com> wrote:
>> On Thu, Mar 24, 2016 at 03:38:25PM -0700, Gregory Farnum wrote:
>>
>>> Just to be clear, you know that you can use the same snapid for all
>>> the images, right? (...although not if you're trying to allow CGs
>>> across pools. Those will be a little harder.)
>>
>> I think not allowing CGs across pools would be annoying limitation for
>> users. If a VM has several volumes for different purposes (e.g. small
>> but fast volume for applicatin data, and large but slow one for
>> backups) it is logical to have different pools for those.
>
> This isn't impossible either. We do it with cache pools and backing
> pools sharing a set of snapids, for instance. But the default rules
> won't work, since a snapid is just a 64-bit counter and they're
> per-pool rather than cluster-global.
> -Greg

Re: blueprint: consistency groups

[Victor Denisov]

The problem that I see with this approach is: let's say we start
adding an image to a cg and the cg is in adding image state.
It either means that somebody is already adding an image to this cg or
somebody died while adding this image.
And we can't really understand it.

So, my suggestion is: normally the command fails if it finds the cg in
an unexpected state.
If the user adds flag --force then it picks up this unfinished
operation and completes it and then adds the new image.
How does that sound?

On Mon, Apr 25, 2016 at 7:23 AM, Jason Dillaman <jdillama@redhat.com> wrote:
> I would hope any necessary state tracking would be enough to allow the code
> to intelligently decide if an image is attached or not to the consistency
> group without the need for a special "check consistency" function.  In your
> example above, I should be able to remove the orphaned image from the CG
> without error.
>
> As an alternative, if you set an "attaching to CG" state in the image before
> adding the image link to the CG, you could prevent the deletion of the image
> but the CG could be deleted if an error occurred.  In this case, the image
> remove logic could be updated to permit deletion of orphaned CG images.
>
> On Fri, Apr 22, 2016 at 6:49 PM, Victor Denisov <vdenisov@mirantis.com>
> wrote:
>>
>> Hi,
>>
>> I have a question regarding implementation of the 'add image to
>> consistency group' operation.
>> Since it's a multi object operation while I'm adding an image
>> reference to a consistency group the image itself can be deleted.
>> By the time I start adding consistency group reference to the image,
>> the image will be gone.
>> So, I have to keep track of my current state of the operation in the
>> consistency group.
>> However if I the librbd client looses connection to the ceph cluster
>> it all will end up in this partially updated state.
>> It's not clear to who is responsible for picking up this state and
>> cleaning it all up.
>>
>> Should it be a special operation that allows us to check consistency
>> of all consistency groups?
>> Is there any agent that is supposed to verify consistency of all rbd
>> objects regularly?
>>
>> Please advise.
>>
>> Thanks in advance,
>> Victor.
>>
>> P.S. Here is a link to the ether pad with the updated blue print:
>> http://pad.ceph.com/p/consistency_groups
>>
>> On Tue, Mar 29, 2016 at 10:54 AM, Gregory Farnum <gfarnum@redhat.com>
>> wrote:
>> > On Fri, Mar 25, 2016 at 12:19 AM, Mykola Golub <mgolub@mirantis.com>
>> > wrote:
>> >> On Thu, Mar 24, 2016 at 03:38:25PM -0700, Gregory Farnum wrote:
>> >>
>> >>> Just to be clear, you know that you can use the same snapid for all
>> >>> the images, right? (...although not if you're trying to allow CGs
>> >>> across pools. Those will be a little harder.)
>> >>
>> >> I think not allowing CGs across pools would be annoying limitation for
>> >> users. If a VM has several volumes for different purposes (e.g. small
>> >> but fast volume for applicatin data, and large but slow one for
>> >> backups) it is logical to have different pools for those.
>> >
>> > This isn't impossible either. We do it with cache pools and backing
>> > pools sharing a set of snapids, for instance. But the default rules
>> > won't work, since a snapid is just a 64-bit counter and they're
>> > per-pool rather than cluster-global.
>> > -Greg
>
>
>
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

> The problem that I see with this approach is: let's say we start
> adding an image to a cg and the cg is in adding image state.
> It either means that somebody is already adding an image to this cg or
> somebody died while adding this image.
> And we can't really understand it.

The metadata link from CG -> image should be in the "adding" state,
not the entire CG.  There shouldn't be anything preventing
concurrently adding two different images to a CG.

>
> So, my suggestion is: normally the command fails if it finds the cg in
> an unexpected state.
> If the user adds flag --force then it picks up this unfinished
> operation and completes it and then adds the new image.
> How does that sound?

My concern w/ adding a "--force" command for this is that it can be
abused.  Personally, I'd rather see that the API is robust enough to
handle these partial states.  If an "add" fails, I should be able to
repeat the "add" and have it work or run "remove" to have it clean up
any intermediate state.


-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

I understand. I can do it.
One quick question: is image id unique across all pools? I'm
referencing image using 'image_<image id>' key and then keeping pool
id in the omap value.
Is it safe?

On Tue, Apr 26, 2016 at 9:09 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>> The problem that I see with this approach is: let's say we start
>> adding an image to a cg and the cg is in adding image state.
>> It either means that somebody is already adding an image to this cg or
>> somebody died while adding this image.
>> And we can't really understand it.
>
> The metadata link from CG -> image should be in the "adding" state,
> not the entire CG.  There shouldn't be anything preventing
> concurrently adding two different images to a CG.
>
>>
>> So, my suggestion is: normally the command fails if it finds the cg in
>> an unexpected state.
>> If the user adds flag --force then it picks up this unfinished
>> operation and completes it and then adds the new image.
>> How does that sound?
>
> My concern w/ adding a "--force" command for this is that it can be
> abused.  Personally, I'd rather see that the API is robust enough to
> handle these partial states.  If an "add" fails, I should be able to
> repeat the "add" and have it work or run "remove" to have it clean up
> any intermediate state.
>
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

On Tue, Apr 26, 2016 at 6:33 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> One quick question: is image id unique across all pools? I'm
> referencing image using 'image_<image id>' key and then keeping pool
> id in the omap value.
> Is it safe?


No -- the image id is only guaranteed to be unique within the pool
that owns the image.  You can use the pool id in combination with the
image id to form a unique key.

-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

I created a pull request for preliminary review:
https://github.com/ceph/ceph/pull/8816
Please leave your comments if you have any. Currently it only has the
following operations implemented:

- create consistency group
- remove consistency group
- add image to consistency group
- remove image from consistency group
- list consistency groups


On Wed, Apr 27, 2016 at 10:50 AM, Jason Dillaman <jdillama@redhat.com> wrote:
> On Tue, Apr 26, 2016 at 6:33 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> One quick question: is image id unique across all pools? I'm
>> referencing image using 'image_<image id>' key and then keeping pool
>> id in the omap value.
>> Is it safe?
>
>
> No -- the image id is only guaranteed to be unique within the pool
> that owns the image.  You can use the pool id in combination with the
> image id to form a unique key.
>
> --
> Jason

Re: blueprint: consistency groups

[Victor Denisov]

I have a question regarding the implementation of list images in a
consistency group.
For referencing images I use their ids and pool ids, since image's
name and pool's name are subject to change.
In order to list the images in a consistency group I have a function:
cg_list_images.
This function only returns ids and pool ids.
I presume normally users would like to see names instead of ids.
Everything in rbd class is written the way that it accepts ioctx.
Which is a handle for accessing pool.
It means(in my opinion) that everything in rbd class should accept
ioctx. As such, I can't open another ioctx when I'm already inside rbd
class.
It leaves me no choice, but to implement image's name resolution in
tools/rbd/action/ConsGrp.cc.
In this case only command line users can have images' names, but not
other services that use librbd library.
Is it fine?

On Thu, Apr 28, 2016 at 11:06 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
> I created a pull request for preliminary review:
> https://github.com/ceph/ceph/pull/8816
> Please leave your comments if you have any. Currently it only has the
> following operations implemented:
>
> - create consistency group
> - remove consistency group
> - add image to consistency group
> - remove image from consistency group
> - list consistency groups
>
>
> On Wed, Apr 27, 2016 at 10:50 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>> On Tue, Apr 26, 2016 at 6:33 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>> One quick question: is image id unique across all pools? I'm
>>> referencing image using 'image_<image id>' key and then keeping pool
>>> id in the omap value.
>>> Is it safe?
>>
>>
>> No -- the image id is only guaranteed to be unique within the pool
>> that owns the image.  You can use the pool id in combination with the
>> image id to form a unique key.
>>
>> --
>> Jason

Re: blueprint: consistency groups

[Mykola Golub]

On Mon, May 02, 2016 at 05:26:04PM -0700, Victor Denisov wrote:
> I have a question regarding the implementation of list images in a
> consistency group.
> For referencing images I use their ids and pool ids, since image's
> name and pool's name are subject to change.
> In order to list the images in a consistency group I have a function:
> cg_list_images.
> This function only returns ids and pool ids.
> I presume normally users would like to see names instead of ids.
> Everything in rbd class is written the way that it accepts ioctx.
> Which is a handle for accessing pool.
> It means(in my opinion) that everything in rbd class should accept
> ioctx. As such, I can't open another ioctx when I'm already inside rbd
> class.
> It leaves me no choice, but to implement image's name resolution in
> tools/rbd/action/ConsGrp.cc.
> In this case only command line users can have images' names, but not
> other services that use librbd library.
> Is it fine?

You need to add RBD API functions. These functions operate only with
image names (image ID is unknown entity for RBD API). And use these
functions in tools/rbd/action/ConsGrp.cc. See for example the recently
added rbd_mirror_image_status_list(). Internally it uses
cls_client::mirror_image_status_list(), which returns image IDs; the
list is converted to image names, when returning to the user.

-- 
Mykola Golub

Re: blueprint: consistency groups

[Jason Dillaman]

On Mon, May 2, 2016 at 8:26 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> As such, I can't open another ioctx when I'm already inside rbd
> class.

You actually can open a new IoCtx against a new pool -- we do this
internally for journaling handling, scanning pools for children during
the snapshot unprotect operation, opening a parent image of a clone,
etc.   You can extract the "librados::Rados" object from the
"librados::IoCtx" and then use "librados::Rados::ioctx_create2" to
create a new IoCtx by pool id.

Also agree with Mykola in that the API should return resolved image names.

-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

Jason,

Can you point me to a specific line of code where you reuse Rados?
I see that IoCtx only has reference to RadosClient not to Rados itself.

On Tue, May 3, 2016 at 5:05 AM, Jason Dillaman <jdillama@redhat.com> wrote:
> On Mon, May 2, 2016 at 8:26 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> As such, I can't open another ioctx when I'm already inside rbd
>> class.
>
> You actually can open a new IoCtx against a new pool -- we do this
> internally for journaling handling, scanning pools for children during
> the snapshot unprotect operation, opening a parent image of a clone,
> etc.   You can extract the "librados::Rados" object from the
> "librados::IoCtx" and then use "librados::Rados::ioctx_create2" to
> create a new IoCtx by pool id.
>
> Also agree with Mykola in that the API should return resolved image names.
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

You can use the explicit constructor of librados::Rados to convert
from a librados::IoCtx [1].  Once you have the Rados object, use
librados::Rados::ioctx_create2 to create a new librados::IoCtx [2].
Here is the example from the snap unprotect state machine [3].

[1] https://github.com/ceph/ceph/blob/master/src/include/rados/librados.hpp#L1145
[2] https://github.com/ceph/ceph/blob/master/src/include/rados/librados.hpp#L1187
[3] https://github.com/ceph/ceph/blob/master/src/librbd/operation/SnapshotUnprotectRequest.cc#L73

On Tue, May 3, 2016 at 5:48 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Jason,
>
> Can you point me to a specific line of code where you reuse Rados?
> I see that IoCtx only has reference to RadosClient not to Rados itself.
>
> On Tue, May 3, 2016 at 5:05 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>> On Mon, May 2, 2016 at 8:26 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>> As such, I can't open another ioctx when I'm already inside rbd
>>> class.
>>
>> You actually can open a new IoCtx against a new pool -- we do this
>> internally for journaling handling, scanning pools for children during
>> the snapshot unprotect operation, opening a parent image of a clone,
>> etc.   You can extract the "librados::Rados" object from the
>> "librados::IoCtx" and then use "librados::Rados::ioctx_create2" to
>> create a new IoCtx by pool id.
>>
>> Also agree with Mykola in that the API should return resolved image names.
>>
>> --
>> Jason



-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

I was writing unit tests for create_cg function when I realized that
it wouldn't recover if we lost connection between
cls_client::dir_add_cg and create_cg object. If create_cg doesn't
happen then we are unable to repeat dir_add_cg.
However since I implemented create_cg operation after create_image
operation. create_image operation has the same issue.

So, my question is. Should I reimplement create_cg so that it can
recover from connection loss to the cluster or should I keep it the
same way create_image is implemented?

Also, what is more preferable big pull request for all components of
the feature - CRUD for consistency groups and snapshots, or split it
into smaller pull requests - first merge CRUD operations then
implement snapshots?

Thanks,
V.

On Tue, May 3, 2016 at 5:29 PM, Jason Dillaman <jdillama@redhat.com> wrote:
> You can use the explicit constructor of librados::Rados to convert
> from a librados::IoCtx [1].  Once you have the Rados object, use
> librados::Rados::ioctx_create2 to create a new librados::IoCtx [2].
> Here is the example from the snap unprotect state machine [3].
>
> [1] https://github.com/ceph/ceph/blob/master/src/include/rados/librados.hpp#L1145
> [2] https://github.com/ceph/ceph/blob/master/src/include/rados/librados.hpp#L1187
> [3] https://github.com/ceph/ceph/blob/master/src/librbd/operation/SnapshotUnprotectRequest.cc#L73
>
> On Tue, May 3, 2016 at 5:48 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> Jason,
>>
>> Can you point me to a specific line of code where you reuse Rados?
>> I see that IoCtx only has reference to RadosClient not to Rados itself.
>>
>> On Tue, May 3, 2016 at 5:05 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>> On Mon, May 2, 2016 at 8:26 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>> As such, I can't open another ioctx when I'm already inside rbd
>>>> class.
>>>
>>> You actually can open a new IoCtx against a new pool -- we do this
>>> internally for journaling handling, scanning pools for children during
>>> the snapshot unprotect operation, opening a parent image of a clone,
>>> etc.   You can extract the "librados::Rados" object from the
>>> "librados::IoCtx" and then use "librados::Rados::ioctx_create2" to
>>> create a new IoCtx by pool id.
>>>
>>> Also agree with Mykola in that the API should return resolved image names.
>>>
>>> --
>>> Jason
>
>
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

On Tue, May 10, 2016 at 12:25 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> I was writing unit tests for create_cg function when I realized that
> it wouldn't recover if we lost connection between
> cls_client::dir_add_cg and create_cg object. If create_cg doesn't
> happen then we are unable to repeat dir_add_cg.
> However since I implemented create_cg operation after create_image
> operation. create_image operation has the same issue.

Correct - the recovery is to remove the image and re-create if you
have a failure creating the image.  I think that is a valid solution
to the consistency groups as well.

> So, my question is. Should I reimplement create_cg so that it can
> recover from connection loss to the cluster or should I keep it the
> same way create_image is implemented?

There will not be an issue if the connection is lost -- there will
only be an issue if the rbd CLI (or whatever else is using the librbd
API) crashes / is stopped mid-operation.  I am fine with matching the
create_image behavior because there is a recovery path.

> Also, what is more preferable big pull request for all components of
> the feature - CRUD for consistency groups and snapshots, or split it
> into smaller pull requests - first merge CRUD operations then
> implement snapshots?

Smaller patchsets are easier to review, test, and merge (especially as
review comments are addressed).

-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

Thanks, Jason. At the first glance at create_image and remove_image I
frankly wasn't sure it would be able to recover from a mid-operation
failure.
Now, I tested it and reread the code - it works like a charm. If we
remove a partially added image then all artifacts will be clean up.
I just need to make consistency groups behave the same way.

One more question. I see that remove operation for images can report
progress. Is it necessary for consistency groups?


On Wed, May 11, 2016 at 8:12 AM, Jason Dillaman <jdillama@redhat.com> wrote:
> On Tue, May 10, 2016 at 12:25 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> I was writing unit tests for create_cg function when I realized that
>> it wouldn't recover if we lost connection between
>> cls_client::dir_add_cg and create_cg object. If create_cg doesn't
>> happen then we are unable to repeat dir_add_cg.
>> However since I implemented create_cg operation after create_image
>> operation. create_image operation has the same issue.
>
> Correct - the recovery is to remove the image and re-create if you
> have a failure creating the image.  I think that is a valid solution
> to the consistency groups as well.
>
>> So, my question is. Should I reimplement create_cg so that it can
>> recover from connection loss to the cluster or should I keep it the
>> same way create_image is implemented?
>
> There will not be an issue if the connection is lost -- there will
> only be an issue if the rbd CLI (or whatever else is using the librbd
> API) crashes / is stopped mid-operation.  I am fine with matching the
> create_image behavior because there is a recovery path.
>
>> Also, what is more preferable big pull request for all components of
>> the feature - CRUD for consistency groups and snapshots, or split it
>> into smaller pull requests - first merge CRUD operations then
>> implement snapshots?
>
> Smaller patchsets are easier to review, test, and merge (especially as
> review comments are addressed).
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

On Wed, May 11, 2016 at 2:44 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> One more question. I see that remove operation for images can report
> progress. Is it necessary for consistency groups?

It shouldn't be necessary for consistency groups.  The rationale for
the progress callback for images is because it might require deleting
tens of thousands of backing objects.  When you remove a consistency
groups, we should ensure that all associated images have already been
removed -- so it should be a quick (algorithmically speaking)
operation.

-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

Jason,

Do you have any opinion regarding deleting images that are in a
consistency group?

Should we delete them as well as the references in the consistency
group they belong to or should we prohibit deleting images that are in
a consistency group?

V.

On Wed, May 11, 2016 at 1:24 PM, Jason Dillaman <jdillama@redhat.com> wrote:
> On Wed, May 11, 2016 at 2:44 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> One more question. I see that remove operation for images can report
>> progress. Is it necessary for consistency groups?
>
> It shouldn't be necessary for consistency groups.  The rationale for
> the progress callback for images is because it might require deleting
> tens of thousands of backing objects.  When you remove a consistency
> groups, we should ensure that all associated images have already been
> removed -- so it should be a quick (algorithmically speaking)
> operation.
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Jason,
>
> Do you have any opinion regarding deleting images that are in a
> consistency group?
>
> Should we delete them as well as the references in the consistency
> group they belong to or should we prohibit deleting images that are in
> a consistency group?
>
> V.
>

Right now, if an image has a snapshot we required you to remove all
snapshots before removing the image.  Along those lines, if an image
is in a consistency group and the consistency group has snapshots, the
user wouldn't be able to remove the image since it has snapshots nor
should the user be able to remove the snapshots associated with the
consistency group. In this case, the user would be forced to
dissociate the image from the group before attempting to delete it.
Therefore, just to keep the actions consistent, you might as well
force the user to dissociate an image from the consistency group even
if the image doesn't have snapshots.


-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

Jason,

Do you prefer pull requests to be rebased on top of the latest master
or should I keep it where I started the development?

Thanks,
V.

On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> Jason,
>>
>> Do you have any opinion regarding deleting images that are in a
>> consistency group?
>>
>> Should we delete them as well as the references in the consistency
>> group they belong to or should we prohibit deleting images that are in
>> a consistency group?
>>
>> V.
>>
>
> Right now, if an image has a snapshot we required you to remove all
> snapshots before removing the image.  Along those lines, if an image
> is in a consistency group and the consistency group has snapshots, the
> user wouldn't be able to remove the image since it has snapshots nor
> should the user be able to remove the snapshots associated with the
> consistency group. In this case, the user would be forced to
> dissociate the image from the group before attempting to delete it.
> Therefore, just to keep the actions consistent, you might as well
> force the user to dissociate an image from the consistency group even
> if the image doesn't have snapshots.
>
>
> --
> Jason

Re: blueprint: consistency groups

[Victor Denisov]

Never mind. I just realized that it will be easier to build it on top
of the latest master in any case.

On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Jason,
>
> Do you prefer pull requests to be rebased on top of the latest master
> or should I keep it where I started the development?
>
> Thanks,
> V.
>
> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>> Jason,
>>>
>>> Do you have any opinion regarding deleting images that are in a
>>> consistency group?
>>>
>>> Should we delete them as well as the references in the consistency
>>> group they belong to or should we prohibit deleting images that are in
>>> a consistency group?
>>>
>>> V.
>>>
>>
>> Right now, if an image has a snapshot we required you to remove all
>> snapshots before removing the image.  Along those lines, if an image
>> is in a consistency group and the consistency group has snapshots, the
>> user wouldn't be able to remove the image since it has snapshots nor
>> should the user be able to remove the snapshots associated with the
>> consistency group. In this case, the user would be forced to
>> dissociate the image from the group before attempting to delete it.
>> Therefore, just to keep the actions consistent, you might as well
>> force the user to dissociate an image from the consistency group even
>> if the image doesn't have snapshots.
>>
>>
>> --
>> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

Definitely want to ensure that it cleanly merges with master.  I would
also request, if at all possible, that you break it into individual
PRs of concrete sub-tasks for implementing consistency groups for ease
of review.  Have individual commits for each step of implementing the
task would also help (i.e. squash related commits, fix style issues or
bugs in the commit that introduced them, etc).

On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Never mind. I just realized that it will be easier to build it on top
> of the latest master in any case.
>
> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> Jason,
>>
>> Do you prefer pull requests to be rebased on top of the latest master
>> or should I keep it where I started the development?
>>
>> Thanks,
>> V.
>>
>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>> Jason,
>>>>
>>>> Do you have any opinion regarding deleting images that are in a
>>>> consistency group?
>>>>
>>>> Should we delete them as well as the references in the consistency
>>>> group they belong to or should we prohibit deleting images that are in
>>>> a consistency group?
>>>>
>>>> V.
>>>>
>>>
>>> Right now, if an image has a snapshot we required you to remove all
>>> snapshots before removing the image.  Along those lines, if an image
>>> is in a consistency group and the consistency group has snapshots, the
>>> user wouldn't be able to remove the image since it has snapshots nor
>>> should the user be able to remove the snapshots associated with the
>>> consistency group. In this case, the user would be forced to
>>> dissociate the image from the group before attempting to delete it.
>>> Therefore, just to keep the actions consistent, you might as well
>>> force the user to dissociate an image from the consistency group even
>>> if the image doesn't have snapshots.
>>>
>>>
>>> --
>>> Jason



-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

Yes, I'm working on splitting it into small logically separated commits.
My current PR is for CRUD operations only(create, remove, add image,
remove image, show info).
Do you want even a smaller PR or this one is small enough?

Thanks,
V.

On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman <jdillama@redhat.com> wrote:
> Definitely want to ensure that it cleanly merges with master.  I would
> also request, if at all possible, that you break it into individual
> PRs of concrete sub-tasks for implementing consistency groups for ease
> of review.  Have individual commits for each step of implementing the
> task would also help (i.e. squash related commits, fix style issues or
> bugs in the commit that introduced them, etc).
>
> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> Never mind. I just realized that it will be easier to build it on top
>> of the latest master in any case.
>>
>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>> Jason,
>>>
>>> Do you prefer pull requests to be rebased on top of the latest master
>>> or should I keep it where I started the development?
>>>
>>> Thanks,
>>> V.
>>>
>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>> Jason,
>>>>>
>>>>> Do you have any opinion regarding deleting images that are in a
>>>>> consistency group?
>>>>>
>>>>> Should we delete them as well as the references in the consistency
>>>>> group they belong to or should we prohibit deleting images that are in
>>>>> a consistency group?
>>>>>
>>>>> V.
>>>>>
>>>>
>>>> Right now, if an image has a snapshot we required you to remove all
>>>> snapshots before removing the image.  Along those lines, if an image
>>>> is in a consistency group and the consistency group has snapshots, the
>>>> user wouldn't be able to remove the image since it has snapshots nor
>>>> should the user be able to remove the snapshots associated with the
>>>> consistency group. In this case, the user would be forced to
>>>> dissociate the image from the group before attempting to delete it.
>>>> Therefore, just to keep the actions consistent, you might as well
>>>> force the user to dissociate an image from the consistency group even
>>>> if the image doesn't have snapshots.
>>>>
>>>>
>>>> --
>>>> Jason
>
>
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

The current PR is at approximately 2500 lines -- it would be nice to
have a PR under 1000 changed lines in a perfect world.  The trouble
with larger PR is that after you address comments, I have to re-read
it all again.  The smaller the PR, the faster it can be reviewed and
merged.  If changes depend on another PR, you can always
non-fast-forward merge the other PR branch into your dependent PR
branch so that it's clear what needs to be reviewed.

On Tue, May 24, 2016 at 6:42 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Yes, I'm working on splitting it into small logically separated commits.
> My current PR is for CRUD operations only(create, remove, add image,
> remove image, show info).
> Do you want even a smaller PR or this one is small enough?
>
> Thanks,
> V.
>
> On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>> Definitely want to ensure that it cleanly merges with master.  I would
>> also request, if at all possible, that you break it into individual
>> PRs of concrete sub-tasks for implementing consistency groups for ease
>> of review.  Have individual commits for each step of implementing the
>> task would also help (i.e. squash related commits, fix style issues or
>> bugs in the commit that introduced them, etc).
>>
>> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>> Never mind. I just realized that it will be easier to build it on top
>>> of the latest master in any case.
>>>
>>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>> Jason,
>>>>
>>>> Do you prefer pull requests to be rebased on top of the latest master
>>>> or should I keep it where I started the development?
>>>>
>>>> Thanks,
>>>> V.
>>>>
>>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>> Jason,
>>>>>>
>>>>>> Do you have any opinion regarding deleting images that are in a
>>>>>> consistency group?
>>>>>>
>>>>>> Should we delete them as well as the references in the consistency
>>>>>> group they belong to or should we prohibit deleting images that are in
>>>>>> a consistency group?
>>>>>>
>>>>>> V.
>>>>>>
>>>>>
>>>>> Right now, if an image has a snapshot we required you to remove all
>>>>> snapshots before removing the image.  Along those lines, if an image
>>>>> is in a consistency group and the consistency group has snapshots, the
>>>>> user wouldn't be able to remove the image since it has snapshots nor
>>>>> should the user be able to remove the snapshots associated with the
>>>>> consistency group. In this case, the user would be forced to
>>>>> dissociate the image from the group before attempting to delete it.
>>>>> Therefore, just to keep the actions consistent, you might as well
>>>>> force the user to dissociate an image from the consistency group even
>>>>> if the image doesn't have snapshots.
>>>>>
>>>>>
>>>>> --
>>>>> Jason
>>
>>
>>
>> --
>> Jason



-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

I this case I case create a separate pull request for consistency
group's operations only(create, remove, list).
Another pull request will be: add image to cg, remove image, list images.

One more question. How do you prefer comments to be addressed, as an
additional commit to the pull request or amend the corresponding
commit?

V.

On Tue, May 24, 2016 at 10:30 PM, Jason Dillaman <jdillama@redhat.com> wrote:
> The current PR is at approximately 2500 lines -- it would be nice to
> have a PR under 1000 changed lines in a perfect world.  The trouble
> with larger PR is that after you address comments, I have to re-read
> it all again.  The smaller the PR, the faster it can be reviewed and
> merged.  If changes depend on another PR, you can always
> non-fast-forward merge the other PR branch into your dependent PR
> branch so that it's clear what needs to be reviewed.
>
> On Tue, May 24, 2016 at 6:42 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> Yes, I'm working on splitting it into small logically separated commits.
>> My current PR is for CRUD operations only(create, remove, add image,
>> remove image, show info).
>> Do you want even a smaller PR or this one is small enough?
>>
>> Thanks,
>> V.
>>
>> On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>> Definitely want to ensure that it cleanly merges with master.  I would
>>> also request, if at all possible, that you break it into individual
>>> PRs of concrete sub-tasks for implementing consistency groups for ease
>>> of review.  Have individual commits for each step of implementing the
>>> task would also help (i.e. squash related commits, fix style issues or
>>> bugs in the commit that introduced them, etc).
>>>
>>> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>> Never mind. I just realized that it will be easier to build it on top
>>>> of the latest master in any case.
>>>>
>>>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>> Jason,
>>>>>
>>>>> Do you prefer pull requests to be rebased on top of the latest master
>>>>> or should I keep it where I started the development?
>>>>>
>>>>> Thanks,
>>>>> V.
>>>>>
>>>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>> Jason,
>>>>>>>
>>>>>>> Do you have any opinion regarding deleting images that are in a
>>>>>>> consistency group?
>>>>>>>
>>>>>>> Should we delete them as well as the references in the consistency
>>>>>>> group they belong to or should we prohibit deleting images that are in
>>>>>>> a consistency group?
>>>>>>>
>>>>>>> V.
>>>>>>>
>>>>>>
>>>>>> Right now, if an image has a snapshot we required you to remove all
>>>>>> snapshots before removing the image.  Along those lines, if an image
>>>>>> is in a consistency group and the consistency group has snapshots, the
>>>>>> user wouldn't be able to remove the image since it has snapshots nor
>>>>>> should the user be able to remove the snapshots associated with the
>>>>>> consistency group. In this case, the user would be forced to
>>>>>> dissociate the image from the group before attempting to delete it.
>>>>>> Therefore, just to keep the actions consistent, you might as well
>>>>>> force the user to dissociate an image from the consistency group even
>>>>>> if the image doesn't have snapshots.
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Jason
>>>
>>>
>>>
>>> --
>>> Jason
>
>
>
> --
> Jason

Re: blueprint: consistency groups

[Jason Dillaman]

Perfect.  I prefer to have the PR's commits reflect the clean
end-state of the change instead of having multiple "tweak" commits
fixing code review comments.  Therefore, a rebase to address the
comment in its associated comment and force push is the desired
process.

On Wed, May 25, 2016 at 5:34 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> I this case I case create a separate pull request for consistency
> group's operations only(create, remove, list).
> Another pull request will be: add image to cg, remove image, list images.
>
> One more question. How do you prefer comments to be addressed, as an
> additional commit to the pull request or amend the corresponding
> commit?
>
> V.
>
> On Tue, May 24, 2016 at 10:30 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>> The current PR is at approximately 2500 lines -- it would be nice to
>> have a PR under 1000 changed lines in a perfect world.  The trouble
>> with larger PR is that after you address comments, I have to re-read
>> it all again.  The smaller the PR, the faster it can be reviewed and
>> merged.  If changes depend on another PR, you can always
>> non-fast-forward merge the other PR branch into your dependent PR
>> branch so that it's clear what needs to be reviewed.
>>
>> On Tue, May 24, 2016 at 6:42 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>> Yes, I'm working on splitting it into small logically separated commits.
>>> My current PR is for CRUD operations only(create, remove, add image,
>>> remove image, show info).
>>> Do you want even a smaller PR or this one is small enough?
>>>
>>> Thanks,
>>> V.
>>>
>>> On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>> Definitely want to ensure that it cleanly merges with master.  I would
>>>> also request, if at all possible, that you break it into individual
>>>> PRs of concrete sub-tasks for implementing consistency groups for ease
>>>> of review.  Have individual commits for each step of implementing the
>>>> task would also help (i.e. squash related commits, fix style issues or
>>>> bugs in the commit that introduced them, etc).
>>>>
>>>> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>> Never mind. I just realized that it will be easier to build it on top
>>>>> of the latest master in any case.
>>>>>
>>>>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>> Jason,
>>>>>>
>>>>>> Do you prefer pull requests to be rebased on top of the latest master
>>>>>> or should I keep it where I started the development?
>>>>>>
>>>>>> Thanks,
>>>>>> V.
>>>>>>
>>>>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>>> Jason,
>>>>>>>>
>>>>>>>> Do you have any opinion regarding deleting images that are in a
>>>>>>>> consistency group?
>>>>>>>>
>>>>>>>> Should we delete them as well as the references in the consistency
>>>>>>>> group they belong to or should we prohibit deleting images that are in
>>>>>>>> a consistency group?
>>>>>>>>
>>>>>>>> V.
>>>>>>>>
>>>>>>>
>>>>>>> Right now, if an image has a snapshot we required you to remove all
>>>>>>> snapshots before removing the image.  Along those lines, if an image
>>>>>>> is in a consistency group and the consistency group has snapshots, the
>>>>>>> user wouldn't be able to remove the image since it has snapshots nor
>>>>>>> should the user be able to remove the snapshots associated with the
>>>>>>> consistency group. In this case, the user would be forced to
>>>>>>> dissociate the image from the group before attempting to delete it.
>>>>>>> Therefore, just to keep the actions consistent, you might as well
>>>>>>> force the user to dissociate an image from the consistency group even
>>>>>>> if the image doesn't have snapshots.
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Jason
>>>>
>>>>
>>>>
>>>> --
>>>> Jason
>>
>>
>>
>> --
>> Jason



-- 
Jason

Re: blueprint: consistency groups

[Victor Denisov]

Here is the link to my pull request with operations on consistency
groups only: create cg, remove cg, list cgs.

https://github.com/ceph/ceph/pull/9333

On Wed, May 25, 2016 at 5:53 PM, Jason Dillaman <jdillama@redhat.com> wrote:
> Perfect.  I prefer to have the PR's commits reflect the clean
> end-state of the change instead of having multiple "tweak" commits
> fixing code review comments.  Therefore, a rebase to address the
> comment in its associated comment and force push is the desired
> process.
>
> On Wed, May 25, 2016 at 5:34 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> I this case I case create a separate pull request for consistency
>> group's operations only(create, remove, list).
>> Another pull request will be: add image to cg, remove image, list images.
>>
>> One more question. How do you prefer comments to be addressed, as an
>> additional commit to the pull request or amend the corresponding
>> commit?
>>
>> V.
>>
>> On Tue, May 24, 2016 at 10:30 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>> The current PR is at approximately 2500 lines -- it would be nice to
>>> have a PR under 1000 changed lines in a perfect world.  The trouble
>>> with larger PR is that after you address comments, I have to re-read
>>> it all again.  The smaller the PR, the faster it can be reviewed and
>>> merged.  If changes depend on another PR, you can always
>>> non-fast-forward merge the other PR branch into your dependent PR
>>> branch so that it's clear what needs to be reviewed.
>>>
>>> On Tue, May 24, 2016 at 6:42 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>> Yes, I'm working on splitting it into small logically separated commits.
>>>> My current PR is for CRUD operations only(create, remove, add image,
>>>> remove image, show info).
>>>> Do you want even a smaller PR or this one is small enough?
>>>>
>>>> Thanks,
>>>> V.
>>>>
>>>> On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>> Definitely want to ensure that it cleanly merges with master.  I would
>>>>> also request, if at all possible, that you break it into individual
>>>>> PRs of concrete sub-tasks for implementing consistency groups for ease
>>>>> of review.  Have individual commits for each step of implementing the
>>>>> task would also help (i.e. squash related commits, fix style issues or
>>>>> bugs in the commit that introduced them, etc).
>>>>>
>>>>> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>> Never mind. I just realized that it will be easier to build it on top
>>>>>> of the latest master in any case.
>>>>>>
>>>>>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>> Jason,
>>>>>>>
>>>>>>> Do you prefer pull requests to be rebased on top of the latest master
>>>>>>> or should I keep it where I started the development?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> V.
>>>>>>>
>>>>>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>>>> Jason,
>>>>>>>>>
>>>>>>>>> Do you have any opinion regarding deleting images that are in a
>>>>>>>>> consistency group?
>>>>>>>>>
>>>>>>>>> Should we delete them as well as the references in the consistency
>>>>>>>>> group they belong to or should we prohibit deleting images that are in
>>>>>>>>> a consistency group?
>>>>>>>>>
>>>>>>>>> V.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Right now, if an image has a snapshot we required you to remove all
>>>>>>>> snapshots before removing the image.  Along those lines, if an image
>>>>>>>> is in a consistency group and the consistency group has snapshots, the
>>>>>>>> user wouldn't be able to remove the image since it has snapshots nor
>>>>>>>> should the user be able to remove the snapshots associated with the
>>>>>>>> consistency group. In this case, the user would be forced to
>>>>>>>> dissociate the image from the group before attempting to delete it.
>>>>>>>> Therefore, just to keep the actions consistent, you might as well
>>>>>>>> force the user to dissociate an image from the consistency group even
>>>>>>>> if the image doesn't have snapshots.
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Jason
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Jason
>>>
>>>
>>>
>>> --
>>> Jason
>
>
>
> --
> Jason

Re: blueprint: consistency groups

[Victor Denisov]

Just updated the pull request with all fixes. Please review one more time.

On Wed, May 25, 2016 at 6:01 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Here is the link to my pull request with operations on consistency
> groups only: create cg, remove cg, list cgs.
>
> https://github.com/ceph/ceph/pull/9333
>
> On Wed, May 25, 2016 at 5:53 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>> Perfect.  I prefer to have the PR's commits reflect the clean
>> end-state of the change instead of having multiple "tweak" commits
>> fixing code review comments.  Therefore, a rebase to address the
>> comment in its associated comment and force push is the desired
>> process.
>>
>> On Wed, May 25, 2016 at 5:34 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>> I this case I case create a separate pull request for consistency
>>> group's operations only(create, remove, list).
>>> Another pull request will be: add image to cg, remove image, list images.
>>>
>>> One more question. How do you prefer comments to be addressed, as an
>>> additional commit to the pull request or amend the corresponding
>>> commit?
>>>
>>> V.
>>>
>>> On Tue, May 24, 2016 at 10:30 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>> The current PR is at approximately 2500 lines -- it would be nice to
>>>> have a PR under 1000 changed lines in a perfect world.  The trouble
>>>> with larger PR is that after you address comments, I have to re-read
>>>> it all again.  The smaller the PR, the faster it can be reviewed and
>>>> merged.  If changes depend on another PR, you can always
>>>> non-fast-forward merge the other PR branch into your dependent PR
>>>> branch so that it's clear what needs to be reviewed.
>>>>
>>>> On Tue, May 24, 2016 at 6:42 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>> Yes, I'm working on splitting it into small logically separated commits.
>>>>> My current PR is for CRUD operations only(create, remove, add image,
>>>>> remove image, show info).
>>>>> Do you want even a smaller PR or this one is small enough?
>>>>>
>>>>> Thanks,
>>>>> V.
>>>>>
>>>>> On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>>> Definitely want to ensure that it cleanly merges with master.  I would
>>>>>> also request, if at all possible, that you break it into individual
>>>>>> PRs of concrete sub-tasks for implementing consistency groups for ease
>>>>>> of review.  Have individual commits for each step of implementing the
>>>>>> task would also help (i.e. squash related commits, fix style issues or
>>>>>> bugs in the commit that introduced them, etc).
>>>>>>
>>>>>> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>> Never mind. I just realized that it will be easier to build it on top
>>>>>>> of the latest master in any case.
>>>>>>>
>>>>>>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>>> Jason,
>>>>>>>>
>>>>>>>> Do you prefer pull requests to be rebased on top of the latest master
>>>>>>>> or should I keep it where I started the development?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> V.
>>>>>>>>
>>>>>>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>>>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>>>>> Jason,
>>>>>>>>>>
>>>>>>>>>> Do you have any opinion regarding deleting images that are in a
>>>>>>>>>> consistency group?
>>>>>>>>>>
>>>>>>>>>> Should we delete them as well as the references in the consistency
>>>>>>>>>> group they belong to or should we prohibit deleting images that are in
>>>>>>>>>> a consistency group?
>>>>>>>>>>
>>>>>>>>>> V.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Right now, if an image has a snapshot we required you to remove all
>>>>>>>>> snapshots before removing the image.  Along those lines, if an image
>>>>>>>>> is in a consistency group and the consistency group has snapshots, the
>>>>>>>>> user wouldn't be able to remove the image since it has snapshots nor
>>>>>>>>> should the user be able to remove the snapshots associated with the
>>>>>>>>> consistency group. In this case, the user would be forced to
>>>>>>>>> dissociate the image from the group before attempting to delete it.
>>>>>>>>> Therefore, just to keep the actions consistent, you might as well
>>>>>>>>> force the user to dissociate an image from the consistency group even
>>>>>>>>> if the image doesn't have snapshots.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Jason
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Jason
>>>>
>>>>
>>>>
>>>> --
>>>> Jason
>>
>>
>>
>> --
>> Jason

Re: blueprint: consistency groups

[Victor Denisov]

Another version of PR is available addressing the latest three comments.

On Fri, Jun 3, 2016 at 5:33 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
> Just updated the pull request with all fixes. Please review one more time.
>
> On Wed, May 25, 2016 at 6:01 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>> Here is the link to my pull request with operations on consistency
>> groups only: create cg, remove cg, list cgs.
>>
>> https://github.com/ceph/ceph/pull/9333
>>
>> On Wed, May 25, 2016 at 5:53 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>> Perfect.  I prefer to have the PR's commits reflect the clean
>>> end-state of the change instead of having multiple "tweak" commits
>>> fixing code review comments.  Therefore, a rebase to address the
>>> comment in its associated comment and force push is the desired
>>> process.
>>>
>>> On Wed, May 25, 2016 at 5:34 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>> I this case I case create a separate pull request for consistency
>>>> group's operations only(create, remove, list).
>>>> Another pull request will be: add image to cg, remove image, list images.
>>>>
>>>> One more question. How do you prefer comments to be addressed, as an
>>>> additional commit to the pull request or amend the corresponding
>>>> commit?
>>>>
>>>> V.
>>>>
>>>> On Tue, May 24, 2016 at 10:30 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>> The current PR is at approximately 2500 lines -- it would be nice to
>>>>> have a PR under 1000 changed lines in a perfect world.  The trouble
>>>>> with larger PR is that after you address comments, I have to re-read
>>>>> it all again.  The smaller the PR, the faster it can be reviewed and
>>>>> merged.  If changes depend on another PR, you can always
>>>>> non-fast-forward merge the other PR branch into your dependent PR
>>>>> branch so that it's clear what needs to be reviewed.
>>>>>
>>>>> On Tue, May 24, 2016 at 6:42 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>> Yes, I'm working on splitting it into small logically separated commits.
>>>>>> My current PR is for CRUD operations only(create, remove, add image,
>>>>>> remove image, show info).
>>>>>> Do you want even a smaller PR or this one is small enough?
>>>>>>
>>>>>> Thanks,
>>>>>> V.
>>>>>>
>>>>>> On Mon, May 23, 2016 at 7:39 PM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>>>> Definitely want to ensure that it cleanly merges with master.  I would
>>>>>>> also request, if at all possible, that you break it into individual
>>>>>>> PRs of concrete sub-tasks for implementing consistency groups for ease
>>>>>>> of review.  Have individual commits for each step of implementing the
>>>>>>> task would also help (i.e. squash related commits, fix style issues or
>>>>>>> bugs in the commit that introduced them, etc).
>>>>>>>
>>>>>>> On Mon, May 23, 2016 at 6:43 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>>> Never mind. I just realized that it will be easier to build it on top
>>>>>>>> of the latest master in any case.
>>>>>>>>
>>>>>>>> On Mon, May 23, 2016 at 8:32 AM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>>>> Jason,
>>>>>>>>>
>>>>>>>>> Do you prefer pull requests to be rebased on top of the latest master
>>>>>>>>> or should I keep it where I started the development?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> V.
>>>>>>>>>
>>>>>>>>> On Mon, May 16, 2016 at 8:48 AM, Jason Dillaman <jdillama@redhat.com> wrote:
>>>>>>>>>> On Thu, May 12, 2016 at 10:45 PM, Victor Denisov <vdenisov@mirantis.com> wrote:
>>>>>>>>>>> Jason,
>>>>>>>>>>>
>>>>>>>>>>> Do you have any opinion regarding deleting images that are in a
>>>>>>>>>>> consistency group?
>>>>>>>>>>>
>>>>>>>>>>> Should we delete them as well as the references in the consistency
>>>>>>>>>>> group they belong to or should we prohibit deleting images that are in
>>>>>>>>>>> a consistency group?
>>>>>>>>>>>
>>>>>>>>>>> V.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Right now, if an image has a snapshot we required you to remove all
>>>>>>>>>> snapshots before removing the image.  Along those lines, if an image
>>>>>>>>>> is in a consistency group and the consistency group has snapshots, the
>>>>>>>>>> user wouldn't be able to remove the image since it has snapshots nor
>>>>>>>>>> should the user be able to remove the snapshots associated with the
>>>>>>>>>> consistency group. In this case, the user would be forced to
>>>>>>>>>> dissociate the image from the group before attempting to delete it.
>>>>>>>>>> Therefore, just to keep the actions consistent, you might as well
>>>>>>>>>> force the user to dissociate an image from the consistency group even
>>>>>>>>>> if the image doesn't have snapshots.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> Jason
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Jason
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Jason
>>>
>>>
>>>
>>> --
>>> Jason