#47424 Replication problem with add-delete requests on single-valued attributes
Closed: Fixed None Opened 6 years ago by nkinder.

One of our customer has an application which generates this kind of requests:

dn: uid=myuser,ou=people,o=myroot
changetype: modify
add: employeenumber
employeenumber: new_value
-
delete: employeenumber
employeenumber: old_value

Note: according to the schema, the employeenumber attribute is NOT multivalued.

The result is that the entry myuser is modified as expected on the master
server, but not on the replicas.

Master server: the employeenumber attribute is updated, "old_value" is replaced
by "new_value".

Replicas: although the audit log is exactly the same as the one of the master
server, the result is different. The employeenumber attribute is deleted from
the entry, "new_value" is ignored.

For both master and replica servers, access logs show only one MOD request with
error code = 0 (success).

We suppose the problem is due to the fact that the add & delete operations are
parts of the same modify request, so they share the same CSN and the
replication module doesn't process them correctly. The add operation seems to
be ignored by the replica.

Here are some additional tests we made, and the results we obtained.

  1. The same modify request with the two operations inverted (delete "old_value"
    is placed before add "new_value").

dn: uid=myuser,ou=people,o=myroot
changetype: modify
delete: employeenumber
employeenumber: old_value
-
add: employeenumber
employeenumber: new_value

-> Result is OK on both master & replicas.

  1. Splitting the original request into 2 separate modify requests, keeping the
    same order as the original.

dn: uid=myuser,ou=people,o=myroot
changetype: modify
add: employeenumber
employeenumber: new_value

dn: uid=myuser,ou=people,o=myroot
changetype: modify
delete: employeenumber
employeenumber: old_value

-> The first request returns an error code 65 (object class violation), what is
logical since the employeenumber attribute is defined as single-valued in the
schema. The second request deletes the employeenumber attribute on both the
master & the replica servers.

  1. Splitting the original request into 2 separate modify requests, with order
    inversion (first delete then add operation).

dn: uid=myuser,ou=people,o=myroot
changetype: modify
delete: employeenumber
employeenumber: old_value

dn: uid=myuser,ou=people,o=myroot
changetype: modify
add: employeenumber
employeenumber: new_value

-> Result is OK on both master & replicas.

  1. Executing a request similar to the original except it modifies a multivalued
    attribute, such as title, instead of employeenumber.

dn: uid=myuser,ou=people,o=myroot
changetype: modify
add: title
title: new_value
-
delete: title
title: old_value

-> Result is OK on both master & replicas: "old_value" is deleted and replaced
by "new_value".

As a workaround we modified the source code of the application so that it
executes the 2 operations separately (cf. test 3). However it means there is
some bug in the replication, because the modify request should not have a
return code 0 on replicas while it doesn't produce the same result as on the
master server.


0001-Ticket-47424-Replication-problem-with-add-delete-req.patch
0001-Ticket-47424-Replication-problem-with-add-delete-req.patch

Looks ok to me, what I am wondering is if it also handles the cases where more than on e value is involved,eg

add: newval1
add: newval2
add: newval3
-
delete: oldval
delete: newval1
delete: newval3

it should then have newval2. Or

add: newval1
delete:oldval
add: newval2
delete: newval1

And if you port the fix to master, could you add the following change
index 0335358..3e3c2c4 100644
--- a/ldap/servers/slapd/entrywsi.c
+++ b/ldap/servers/slapd/entrywsi.c
@@ -757,7 +757,9 @@ entry_delete_present_values_wsi_single_valued(Slapi_Entry e, const char type,
{
valuearray_free(&deletedvalues);
/ The attribute is single valued and the value was successful deleted /
- entry_present_attribute_to_deleted_attribute(e, a);
+ / but there could have been an add in the same operation, so double check /
+ if (valueset_isempty(&a->a_present_values))
+ entry_present_attribute_to_deleted_attribute(e, a);
}
else if (retVal != LDAP_SUCCESS)
{
--

or do we need a new ticket for this ?

Replying to [comment:4 lkrispen]:

Looks ok to me, what I am wondering is if it also handles the cases where more than on e value is involved,eg

add: newval1
add: newval2
add: newval3
-
delete: oldval
delete: newval1
delete: newval3

it should then have newval2. Or

This is rejected as an attempt to add more than one value.

add: newval1
delete:oldval
add: newval2
delete: newval1

This works fine.

And if you port the fix to master, could you add the following change
index 0335358..3e3c2c4 100644
--- a/ldap/servers/slapd/entrywsi.c
+++ b/ldap/servers/slapd/entrywsi.c
@@ -757,7 +757,9 @@ entry_delete_present_values_wsi_single_valued(Slapi_Entry e, const char type,
{
valuearray_free(&deletedvalues);
/ The attribute is single valued and the value was successful deleted /
- entry_present_attribute_to_deleted_attribute(e, a);
+ / but there could have been an add in the same operation, so double check /
+ if (valueset_isempty(&a->a_present_values))
+ entry_present_attribute_to_deleted_attribute(e, a);
}
else if (retVal != LDAP_SUCCESS)
{
--

or do we need a new ticket for this ?

What about ticket 569? My fix is only in the URP code - 569 breaks without urp

yes, the suggested fix is only related to 569. I just was not sure if we would reopen this or create a new ticket as a regression or just here.

Regarding my first test example it finally should have only one value, the original example temporarily also violates the schema (which is ok)

Replying to [comment:7 lkrispen]:

yes, the suggested fix is only related to 569. I just was not sure if we would reopen this or create a new ticket as a regression or just here.

569 is open, so any fixes related to 569 should go there. ticket/47424 is for the urp part, so I will port that part to master, 1.3.1. But I don't think my fix for ticket/47424 will fix 569.

Regarding my first test example it finally should have only one value, the original example temporarily also violates the schema (which is ok)

Ok. Not sure what I was doing wrong. Yes, this case works too, with my patch.

52a0cc5..bf53a29 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit bf53a29
Author: Rich Megginson rmeggins@redhat.com
Date: Wed Jul 17 17:51:36 2013 -0600
6b87414..90f170c 389-ds-base-1.3.0 -> 389-ds-base-1.3.0
commit 90f170c
Author: Rich Megginson rmeggins@redhat.com
Date: Wed Jul 17 17:51:36 2013 -0600
ad0a2ca..ea333f2 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
commit ea333f2
Author: Rich Megginson rmeggins@redhat.com
Date: Wed Jul 17 17:51:36 2013 -0600

To ssh://git.fedorahosted.org/git/389/ds.git
90c04c0..56e4fac master -> master
commit 56e4fac
Author: Rich Megginson rmeggins@redhat.com
Date: Wed Jul 17 17:51:36 2013 -0600

Metadata Update from @rmeggins:
- Issue assigned to rmeggins
- Issue set to the milestone: 1.2.11.22

2 years ago

Login to comment on this ticket.

Metadata