From 3aac2e421bb6c2543b2ff2795664ba50a7ceeec9 Mon Sep 17 00:00:00 2001

From: Rich Megginson <rmeggins@redhat.com>

Date: Wed, 17 Jul 2013 17:51:36 -0600

Subject: [PATCH 80/99] Ticket #47424 - Replication problem with add-delete

 requests on single-valued attributes

https://fedorahosted.org/389/ticket/47424

Reviewed by: lkrispenz (Thanks!)

Branch: 389-ds-base-1.2.11

Fix Description: Change the single master resolve attr code to handle the

specific case of doing

add: newvalue

delete: oldvalue

Had to add a new API function - csn_compare_ext - to be able to compare CSNs

without the subsequence number.

Platforms tested: RHEL6 x86_64

Flag Day: no

Doc impact: no

(cherry picked from commit bf53a29af973648c520b005ad37f7a83db4b905b)

---

 ldap/servers/slapd/csn.c           |  12 +++-

 ldap/servers/slapd/entrywsi.c      | 122 +++++++++++++++++++++----------------

 ldap/servers/slapd/slapi-private.h |   2 +

 3 files changed, 83 insertions(+), 53 deletions(-)

diff --git a/ldap/servers/slapd/csn.c b/ldap/servers/slapd/csn.c

index 9fb5b4a..5c31689 100644

--- a/ldap/servers/slapd/csn.c

+++ b/ldap/servers/slapd/csn.c

@@ -298,7 +298,7 @@ csn_as_attr_option_string(CSNType t,const CSN *csn,char *ss)

 }

 int 

-csn_compare(const CSN *csn1, const CSN *csn2)

+csn_compare_ext(const CSN *csn1, const CSN *csn2, unsigned int flags)

 {

     PRInt32 retVal;

 	if(csn1!=NULL && csn2!=NULL)

@@ -321,7 +321,7 @@ csn_compare(const CSN *csn1, const CSN *csn2)

                     retVal = -1;

                 else if (csn1->rid > csn2->rid)

                     retVal = 1;

-                else

+                else if (!(flags & CSN_COMPARE_SKIP_SUBSEQ))

                 {

                     if (csn1->subseqnum < csn2->subseqnum)

                         retVal = -1;

@@ -330,6 +330,8 @@ csn_compare(const CSN *csn1, const CSN *csn2)

                     else

                         retVal = 0;

                 }

+                else

+                    retVal = 0;

             }

         }

@@ -350,6 +352,12 @@ csn_compare(const CSN *csn1, const CSN *csn2)

     return(retVal);

 }

+int

+csn_compare(const CSN *csn1, const CSN *csn2)

+{

+	return csn_compare_ext(csn1, csn2, 0);

+}

+

 time_t csn_time_difference(const CSN *csn1, const CSN *csn2)

 {

 	return csn_get_time(csn1) - csn_get_time(csn2);

diff --git a/ldap/servers/slapd/entrywsi.c b/ldap/servers/slapd/entrywsi.c

index 971e9e3..6c37f45 100644

--- a/ldap/servers/slapd/entrywsi.c

+++ b/ldap/servers/slapd/entrywsi.c

@@ -423,8 +423,8 @@ static int

 entry_add_present_values_wsi(Slapi_Entry *e, const char *type, struct berval **bervals, const CSN *csn, int urp, long flags)

 {

 	int retVal= LDAP_SUCCESS;

-    Slapi_Value **valuestoadd = NULL;

-    valuearray_init_bervalarray(bervals,&valuestoadd); /* JCM SLOW FUNCTION */

+	Slapi_Value **valuestoadd = NULL;

+	valuearray_init_bervalarray(bervals,&valuestoadd); /* JCM SLOW FUNCTION */

 	if(!valuearray_isempty(valuestoadd))

 	{

 		Slapi_Attr *a= NULL;

@@ -437,7 +437,7 @@ entry_add_present_values_wsi(Slapi_Entry *e, const char *type, struct berval **b

 			slapi_attr_init(a, type);

 			attrlist_add(&e->e_attrs, a);

 		}

-        a_flags_orig = a->a_flags;

+		a_flags_orig = a->a_flags;

 		a->a_flags |= flags;

 		/* Check if the type of the to-be-added values has DN syntax or not. */

 		if (slapi_attr_is_dn_syntax_attr(a)) {

@@ -558,8 +558,8 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval

 		else

 		{

 			/* delete some specific values */

-		    Slapi_Value **valuestodelete= NULL;

-		    valuearray_init_bervalarray(vals,&valuestodelete); /* JCM SLOW FUNCTION */

+			Slapi_Value **valuestodelete= NULL;

+			valuearray_init_bervalarray(vals,&valuestodelete); /* JCM SLOW FUNCTION */

 			/* Check if the type of the to-be-deleted values has DN syntax 

 			 * or not. */

 			if (slapi_attr_is_dn_syntax_attr(a)) {

@@ -575,8 +575,7 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval

 				   there are present values with a later CSN - otherwise, even though

 				   the value will be updated with a VDCSN which is later than the VUCSN,

 				   the attribute will not be deleted */

-				if(slapi_attr_flag_is_set(a,SLAPI_ATTR_FLAG_SINGLE) && valuesupdated &&

-				   *valuesupdated)

+				if(slapi_attr_flag_is_set(a,SLAPI_ATTR_FLAG_SINGLE) && valueset_isempty(&a->a_present_values))

 				{

 					attr_set_deletion_csn(a,csn);			

 				}

@@ -626,8 +625,8 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval

 					if ( retVal==LDAP_OPERATIONS_ERROR )

 					{

 						LDAPDebug( LDAP_DEBUG_ANY, "Possible existing duplicate "

-							"value for attribute type %s found in "

-							"entry %s\n", a->a_type, slapi_entry_get_dn_const(e), 0 );

+						           "value for attribute type %s found in "

+						           "entry %s\n", a->a_type, slapi_entry_get_dn_const(e), 0 );

 					}

 				}

 				valuearray_free(&valuestodelete);

@@ -683,7 +682,7 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval

 			   will add it back to the present list in the non urp case,

 			   or determine if the attribute needs to be added

 			   or not in the urp case

-			*/

+			 */

 			entry_add_deleted_attribute_wsi(e, a);

 		}

 	}

@@ -1103,6 +1102,7 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu

 	Slapi_Value *new_value= NULL;

 	const CSN *current_value_vucsn;

 	const CSN *pending_value_vucsn;

+	const CSN *pending_value_vdcsn;

 	const CSN *adcsn;

 	int i;

@@ -1116,27 +1116,47 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu

 		slapi_attr_next_value(a,i,&new_value);

 	}

 	attr_first_deleted_value(a,&pending_value);

-

 	/* purge_attribute_state_single_valued */

 	adcsn= attr_get_deletion_csn(a);

 	current_value_vucsn= value_get_csn(current_value, CSN_TYPE_VALUE_UPDATED);

 	pending_value_vucsn= value_get_csn(pending_value, CSN_TYPE_VALUE_UPDATED);

-    if((pending_value!=NULL && (csn_compare(adcsn, pending_value_vucsn)<0)) || 

-        (pending_value==NULL && (csn_compare(adcsn, current_value_vucsn)<0)))

+	pending_value_vdcsn= value_get_csn(pending_value, CSN_TYPE_VALUE_DELETED);

+	if((pending_value!=NULL && (csn_compare(adcsn, pending_value_vucsn)<0)) ||

+	   (pending_value==NULL && (csn_compare(adcsn, current_value_vucsn)<0)))

 	{

 		attr_set_deletion_csn(a,NULL);

 		adcsn= NULL;

-    }

+	}

-	if(new_value==NULL)

+	/* in the case of the following:

+	 * add: value2

+	 * delete: value1

+	 * we will have current_value with VUCSN CSN1

+	 * and pending_value with VDCSN CSN2

+	 * and new_value == NULL

+	 * current_value != pending_value

+	 * and

+	 * VUCSN == VDCSN (ignoring subseq)

+	 * even though value1.VDCSN > value2.VUCSN

+	 * value2 should still win because the value is _different_

+	 */

+	if (current_value && pending_value && !new_value && !adcsn &&

+	    (0 != slapi_value_compare(a, current_value, pending_value)) &&

+	    (0 == csn_compare_ext(current_value_vucsn, pending_value_vdcsn, CSN_COMPARE_SKIP_SUBSEQ)))

 	{

-        /* check if the pending value should become the current value */ 

-        if(pending_value!=NULL)

+		/* just remove the deleted value */

+		entry_deleted_value_to_zapped_value(a,pending_value);

+		pending_value = NULL;

+	}

+	else if(new_value==NULL)

+	{

+		/* check if the pending value should become the current value */

+		if(pending_value!=NULL)

 		{

 			if(!value_distinguished_at_csn(e,a,current_value,pending_value_vucsn))

 			{

-	            /* attribute.current_value = attribute.pending_value; */

-	            /* attribute.pending_value = NULL; */

+				/* attribute.current_value = attribute.pending_value; */

+				/* attribute.pending_value = NULL; */

 				entry_present_value_to_zapped_value(a,current_value);

 				entry_deleted_value_to_present_value(a,pending_value);

 				current_value= pending_value;

@@ -1145,12 +1165,12 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu

 				pending_value_vucsn= NULL;

 			}

 		}

-        /* check if the current value should be deleted */ 

-        if(current_value!=NULL)

+		/* check if the current value should be deleted */

+		if(current_value!=NULL)

 		{

 			if(csn_compare(adcsn,current_value_vucsn)>0) /* check if the attribute was deleted after the value was last updated */

 			{

-	            if(!value_distinguished_at_csn(e,a,current_value,current_value_vucsn))

+				if(!value_distinguished_at_csn(e,a,current_value,current_value_vucsn))

 				{

 					entry_present_value_to_zapped_value(a,current_value);

 					current_value= NULL;

@@ -1162,17 +1182,17 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu

 	else /* addition of a new value */ 

 	{

 		const CSN *new_value_vucsn= value_get_csn(new_value,CSN_TYPE_VALUE_UPDATED);

-        if(csn_compare(new_value_vucsn,current_value_vucsn)<0)

+		if(csn_compare(new_value_vucsn,current_value_vucsn)<0)

 		{

-            /*

-             * if the new value was distinguished at the time the current value was added 

-             * then the new value should become current

-             */ 

-            if(value_distinguished_at_csn(e,a,new_value,current_value_vucsn))

+			/*

+			 * if the new value was distinguished at the time the current value was added

+			 * then the new value should become current

+			 */

+			if(value_distinguished_at_csn(e,a,new_value,current_value_vucsn))

 			{

-                /* attribute.pending_value = attribute.current_value  */

-                /* attribute.current_value = new_value  */

-                if(pending_value==NULL)

+				/* attribute.pending_value = attribute.current_value  */

+				/* attribute.current_value = new_value  */

+				if(pending_value==NULL)

 				{

 					entry_present_value_to_deleted_value(a,current_value);

 				}

@@ -1188,63 +1208,63 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu

 			}

 			else

 			{

-                /* new_value= NULL */

+				/* new_value= NULL */

 				entry_present_value_to_zapped_value(a, new_value);

 				new_value= NULL;

 			}

 		}

-        else    /* new value is after the current value */ 

+		else    /* new value is after the current value */

 		{

-            if(!value_distinguished_at_csn(e, a, current_value, new_value_vucsn))

+			if(!value_distinguished_at_csn(e, a, current_value, new_value_vucsn))

 			{

-                /* attribute.current_value = new_value */

+				/* attribute.current_value = new_value */

 				entry_present_value_to_zapped_value(a, current_value);

 				current_value= new_value;

 				new_value= NULL;

 				current_value_vucsn= new_value_vucsn;

 			}

-            else /* value is distinguished - check if we should replace the current pending value */ 

+			else /* value is distinguished - check if we should replace the current pending value */

 			{

-                if(csn_compare(new_value_vucsn, pending_value_vucsn)>0)

+				if(csn_compare(new_value_vucsn, pending_value_vucsn)>0)

 				{

-                    /* attribute.pending_value = new_value */

+					/* attribute.pending_value = new_value */

 					entry_deleted_value_to_zapped_value(a,pending_value);

 					entry_present_value_to_deleted_value(a,new_value);

 					pending_value= new_value;

 					new_value= NULL;

 					pending_value_vucsn= new_value_vucsn;

-                } 

-            } 

-        } 

-    } 

+				}

+			}

+		}

+	}

-    /*

-     * This call ensures that the attribute does not have a pending_value

+	/*

+	 * This call ensures that the attribute does not have a pending_value

 	 * or a deletion_csn that is earlier than the current_value.

 	 */ 

 	/* purge_attribute_state_single_valued */

-    if((pending_value!=NULL && (csn_compare(adcsn, pending_value_vucsn)<0)) || 

-        (pending_value==NULL && (csn_compare(adcsn, current_value_vucsn)<0)))

+	if((pending_value!=NULL && (csn_compare(adcsn, pending_value_vucsn)<0)) ||

+	   (pending_value==NULL && (csn_compare(adcsn, current_value_vucsn)<0)))

 	{

 		attr_set_deletion_csn(a,NULL);

 		adcsn= NULL;

-    } 

+	}

-    /* set attribute state */ 

-    if(current_value==NULL)

+	/* set attribute state */

+	if(current_value==NULL)

 	{

 		if(attribute_state==ATTRIBUTE_PRESENT)

 		{

 			entry_present_attribute_to_deleted_attribute(e, a);

 		}

 	}

-    else 

+	else

 	{

 		if(attribute_state==ATTRIBUTE_DELETED)

 		{

 			entry_deleted_attribute_to_present_attribute(e, a);

 		}

-    }

+	}

 }

 static void

diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h

index ddeac62..1efbc8b 100644

--- a/ldap/servers/slapd/slapi-private.h

+++ b/ldap/servers/slapd/slapi-private.h

@@ -196,6 +196,8 @@ PRUint16 csn_get_seqnum(const CSN *csn);

 PRUint16 csn_get_subseqnum(const CSN *csn);

 char *csn_as_string(const CSN *csn, PRBool replicaIdOrder, char *ss); /* WARNING: ss must be CSN_STRSIZE bytes, or NULL. */

 int csn_compare(const CSN *csn1, const CSN *csn2);

+int csn_compare_ext(const CSN *csn1, const CSN *csn2, unsigned int flags);

+#define CSN_COMPARE_SKIP_SUBSEQ 0x1

 time_t csn_time_difference(const CSN *csn1, const CSN *csn2);

 size_t csn_string_size();

 char *csn_as_attr_option_string(CSNType t,const CSN *csn,char *ss);

-- 

1.8.1.4