[Freeipa-devel] [PATCH 0032] prevent duplicate IDs when setting up multiple replicas against single master

[thierry bordaz]

On 05/07/2015 01:05 PM, Martin Babinsky wrote:
> On 05/06/2015 07:41 PM, thierry bordaz wrote:
>> On 05/06/2015 05:56 PM, Martin Babinsky wrote:
>>> On 05/06/2015 04:25 PM, thierry bordaz wrote:
>>>> On 05/06/2015 03:19 PM, Martin Babinsky wrote:
>>>>> Hello Thierry,
>>>>>
>>>>> replies are inline.
>>>>>
>>>>> On 05/06/2015 02:22 PM, thierry bordaz wrote:
>>>>>> On 05/06/2015 01:54 PM, Martin Babinsky wrote:
>>>>>>> The attached patch tries to fix
>>>>>>> https://fedorahosted.org/freeipa/ticket/4378
>>>>>>>
>>>>>>> After discussion with Thierry we concluded that while this issue is
>>>>>>> more complex than it seems, the transition from REPLACE to DEL/ADD
>>>>>>> operations when updating nsDS5ReplicaId should suffice for this
>>>>>>> ticket.
>>>>>>>
>>>>>> Hello Martin,
>>>>>>
>>>>>> Few comments, you are using MOD_DEL 'replicaID' with None value. So
>>>>>> this
>>>>>> is going to delete all previous values and it should be equivalent
>>>>>> to a
>>>>>> MOD_REPL.
>>>>>> I was thinking you wanted to retrieve the id_value and call MOD_DEL
>>>>>> 'replicaID' <current_value>. So that if by the time you fetched the
>>>>>> replicaId, an other replica updated the replicaId, the 
>>>>>> MOD_DEL/MOD_ADD
>>>>>> would fail and you need a new iteration.
>>>>>>
>>>>> Sorry I didn't know you can MOD_DEL a particular value (I'm LDAP noob
>>>>> I know). Will fix this.
>>>>>> If replicaId was multi-valued and you want to make it single
>>>>>> valued, you
>>>>>> may want to do create a more complex MOD (e.g. (ldap.MOD_DELETE,
>>>>>> 'nsDS5ReplicaId', str(value1), (ldap.MOD_DELETE, 'nsDS5ReplicaId',
>>>>>> str(value2)...)
>>>>>>
>>>>> AFAIK ReplicaId is single-valued (looking at the schema right now) so
>>>>> this shouldn't be problem.
>>>>>> If it is updating successfully do you want to return 'retval' or
>>>>>> 'retval+1' ?
>>>>>>
>>>>>> If several replicas try to update the replicaId of the master and 
>>>>>> the
>>>>>> current replicaId is 1000.
>>>>>> Replica1 successfully updates the replicaId and gets 1001 as the new
>>>>>> value.
>>>>>> Replica2 successfully updates the replicaId and gets 1002.
>>>>>> The final value on master will be 1002, but replica1 will assum 
>>>>>> it is
>>>>>> 1001. Is it a problem ?
>>>>>>
>>>>> I studied the code in the master branch and IIUC (and please correct
>>>>> me if I got this wrong) nsDS5ReplicaId attribute in
>>>>> 'cn=replication,cn=etc,$SUFFIX' represents replicaID of the _next_
>>>>> replica that will be installed.
>>>>>
>>>>> So if a replica is installed, it sets the current value of
>>>>> nsDS5ReplicaId as its replica ID (the function returns 'retval') and
>>>>> then increments it in 'cn=replication,cn=etc,$SUFFIX' entry 
>>>>> ('retval +
>>>>> 1' is written to master) so that the next installed replica fetches
>>>>> this updated value.
>>>>>
>>>>> So the case you described should be the expected behavior. To change
>>>>> it would require different patch IMHO.
>>>>
>>>> Thank for your precious explanations, in fact the value in
>>>> 'cn=replication,cn=etc,SUFFIX' is the next available replicaId 
>>>> value and
>>>> is the centralized mechanism that assign unique replicaID.
>>>>
>>>> The risk was that several replica pick the same value. So yes it is
>>>> important that the MOD_DEL specifies the previously read value so that
>>>> the test/set will be atomic. If several replicas read the same value,
>>>> only the faster one will use it to install the replica.
>>>>
>>>> thanks
>>>> thierry
>>>>>> thanks
>>>>>> thierry
>>>>>
>>>>>
>>>>
>>>
>>> Attaching updated patch with fixed MOD_DELETE operation.
>>>
>>
>> Hi Martin,
>>
>> The fix looks good to me except I think you need to do (ldap.MOD_DELETE,
>> 'nsDS5ReplicaId', *str(*retval*)*)
>>
>> thanks
>> thierry
>
> Attaching updated patch.
>
Thanks Martin,

The fix is good for me. Ack.

thierry
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20150507/86d09831/attachment.htm>