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