From bf53a29af973648c520b005ad37f7a83db4b905b Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Jul 19 2013 20:20:31 +0000 Subject: 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 --- 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 9648b4c..8184843 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);