[Engine-devel] REST-API: Problem with additional DELETE action at collection level

Mon Nov 11 11:03:53 UTC 2013

On 11/11/2013 04:08 PM, Ori Liel wrote:
>> ----- Original Message -----
>> From: "Shubhendu Tripathi" <shtripat at redhat.com>
>> To: oliel at redhat.com, "Michael Pasternak" <mpastern at redhat.com>, "Sahina Bose" <sabose at redhat.com>
>> Cc: "Dusmant Pati" <dpati at redhat.com>, "engine-devel" <engine-devel at ovirt.org>
>> Sent: Monday, November 11, 2013 12:00:34 PM
>> Subject: Re: [Engine-devel] REST-API: Problem with additional DELETE action at	collection level
>>
>> Self and Ori had a detailed discussion on the topic.
>>
>> Points discussed -
>>
>> 1. Ori mentioned that Michael and Ori did not mean to have a separate
>> DELETE action for the purpose of commit in previous discussions.
> That's right - no need for a new delete signature; existing signatures are enough.
>
>> 2. Ori tried to understand the intention behind the separate commit
>> command in gluster and suggested that ideally gluster should remember
>> that a migration of data is started on the set of bricks, and if there
>> is a delete fired on the same bricks it should perform a commit action
>> because commit is nothing but a delete action
>> 3. Ori suggested that in an ideal situation APIs needed would be
>>      - start migrate
>>      - stop migrate
>>      - delete bricks
>>      - retain bricks
>> 4. As suggested by Ori, the remove bricks action should internally
>> decide if there was a data migration, started on the set of bricks. If
>> so, it should effectively fire a commit for the set of bricks. And if
>> there was not occurrence of data migration it should behave like a
>> normal force remove action.
>>
>> Ori, please add your comments if I have missed out anything here.
> I agree with what you wrote, let me sort of say it again in my words:
>
> I don't like requiring two consecutive commands from the API user, to perform migrate.
> If the user only does 'migrate', and doesn't do the second step, we are in a state of
> corruption. But Shubhendu explained to me that there's no way around this; the
> administrator *must* take a look and decide; it is simply not possible to make this
> decision in advance. So I accepted this heavy-heartedly.
>
> Then we were left with the decision of how to model this two step operation. I tried to
> look at it from the point of view of the user; what would be the simplest way to 'tell him
> the story?'
>
> - If you want to migrate a brick, run 'migrate' on it.
> - If you want to stop migration of the brick, run 'stopMigrate' on it.
> - If you want to resume the migration of the brick, simply run 'migrate' on it again.
> After migration is done:
> - if you want to remove the brick, DELETE it (regular delete).
> - if you want to keep using the brick, run 'reactivate' on it.
>
> And that's the whole story.
>
> The advantages are:
>
> * In simple English, not bound to Gluster terms, the administrator has to decide whether to
> get rid of the brick, remove it, when migration is done. So what would be more natural than
> to delete it? It makes sense, and there's no need for a new 'action'. On the Gluster side,
> if user tried to delete a brick which is undergoing migration - simply don't allow it. This
> is something you have to do anyway; a user might try to delete a brick which is undergoing
> migration and you have to stop him from doing so. So 0 extra work here.
>
> * Instead of stopMigrate meaning two different things in two different contexts, we now
> have stopMigration mean exactly what its name suggests: it stops the migration, and 'reactive'
> mean exactly what it suggests - mark this brick as active again, allow it to be used.
Thanks Ori for the detailed mail.
So now the final set of APIs are -
     - migrate
     - stopmigrate
     - delete (existing delete to be enabled for commit)
     - reactivate

Will go ahead with this and raise the patches accordingly.

Thanks and Regards,
Shubhendu
>> Sahina, request your comments on the same.
>>
>> Thanks and Regards,
>> Shubhendu
> On 11/11/2013 10:30 AM, Shubhendu Tripathi wrote:
>> On 11/10/2013 11:06 PM, Moti Asayag wrote:
>>> ----- Original Message -----
>>>> From: "Shubhendu Tripathi" <shtripat at redhat.com>
>>>> To: "engine-devel" <engine-devel at ovirt.org>, "Michael Pasternak"
>>>> <mpastern at redhat.com>, oliel at redhat.com
>>>> Sent: Friday, November 8, 2013 8:37:30 AM
>>>> Subject: [Engine-devel] REST-API: Problem with additional DELETE
>>>> action at    collection level
>>>>
>>>> Hi All,
>>>>
>>>> There is a DELETE action defined at collection level for Gluster
>>>> Bricks with
>>>> signature -
>>>>
>>>> @DELETE
>>>> public Response remove ( GlusterBricks bricks );
>>>>
>>>> Recently we had needed a commit action to remove migrated bricks.
>>>> After multiple rounds of discussion on introducing a commit action
>>>> to remove
>>>> migrated bricks we introduced a DELETE action [1] which accepts a
>>>> boolean
>>>> parameter isForce.
>>>> If the parameter is set to true , forced deletion of bricks happens
>>>> without
>>>> any data migration.
>>>> And if the parameter is not set or set to false, the deletion is
>>>> meant for a
>>>> brick on which migration has already taken place.
>>>>
>>>> To achieve the above functionality we introduced another DELETE
>>>> action with
>>>> new signature as below and also marked the first action as deprecated -
>>>>
>>>> @DELETE
>>>> public Response remove ( Action action );
>>>>
>>>> The problem arises now as the new api works fine for all possible
>>>> scenarios
>>>> with below input structure -
>>>>
>>>> <action>
>>>> <force>true/false</force>
>>>> <bricks>
>>>> <brick>
>>>> <name> brick1-name</name>
>>>> <name>brick2-name</name>
>>>> </brick>
>>>> </bricks>
>>>> </action>
>>>>
>>>> BUT after these change backward compatibility is broken and the old
>>>> api does
>>>> not work.
>>>> If we try invoking old DELETE with bricks as input parameter as
>>>> below, it
>>>> still invokes the new api and gives an error saying " Invalid
>>>> parameter ".
>>>>
>>> Maybe I've missed it some where, but i wasn't able to find the new
>>> 'force' parameter
>>> in the rsdl, and specifically not in optionalArguments list of:
>>>
>>> - name:
>>> /api/clusters/{cluster:id}/glustervolumes/{glustervolume:id}/bricks|rel=delete
>>>
>>> and perhaps the correct approach will be to deprecate this signature
>>> and introduce a new
>>> one with the 'force' in the 'mandatoryArguments' section.
>> Moti, I have introduced another methods remove of DELETE type which
>> takes Action as input.
>> I pass list of bricks and force as parameter in the same Action.
>>
>> Also, I tried marking the old method deprecated and introduced the new
>> one BUT as mentioned above, after introduction of new DELETE with
>> Action parameter old one STOPS working.
>> Is it OK to stop working for a method if its deprecated?? I don't feel
>> so. Please comment.
>>>> <bricks>
>>>> <brick id="brick1-id"/>
>>>> <brick id="brick2-id"/>
>>>> </bricks>
>>>>
>>>> Kindly suggest a solution around the same.
>>>>
>>>> PS: Both the actions are defined at collection level (
>>>> /api/clusters/<cluster-id>/glustervolumes/<volume-id>/bricks )
>>>>
>>>> [1]: http://gerrit.ovirt.org/#/c/21043/
>>>>
>>>> Thanks and Regards,
>>>> Shubhendu
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Engine-devel mailing list
>>>> Engine-devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>
>> _______________________________________________
>> Engine-devel mailing list
>> Engine-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>
>>
>
>