>From 6319251696baccdbd8ab42390c0802996f3c701e Mon Sep 17 00:00:00 2001 From: Nathan Kinder Date: Mon, 16 Nov 2009 12:03:26 -0800 Subject: [PATCH] Bug 515329 - Correct attribute value inconsistency on replica When performing operations with multiple mods to the same multi-valued attribute on a single modify operation, a replica was not resolving the attribute values correctly. This would lead to an inconsistency between the master the change was initially performed against and the replicas. The problem would occur with a modify operation such as this: dn: uid=testuser,dc=example,dc=com changetype: modify add: cn cn: 2 - replace: cn cn: 3 The problem is that we use the CSNs from the attribute state data to determine which values should remain after the operation (this is done to merge with later occuring changes from other masters). The CSN for all mods within the same modify operation is exactly the same. The old code was looking for attributes older than the deletion that occurs as a part of the replace, then deleting those values. This would cause the value of "2" in the above example to remain. Simply changing this comparision to look for values with the same or older CSN to delete would cause the new value of "3" to be removed as well when we get around to resolving the attribute after the second half of the replace operation. The fix is to use a different CSN comparison when we are removing all values of an attribute during attribute resolution (remove values with the same or older CSN). This is safe becuse the only present values at this time are older values or values added in a previous mod in the same modify operation. When processing other mods that are not removing all values of an attribute, we only want to remove values with a CSN older that that of the current modify operation. This prevents us from removing a newly added value, such as "3" in the example above. This is safe since we resolve the attribute after each mod in the modify operation. --- ldap/servers/slapd/entrywsi.c | 30 +++++++++++++++++++++--------- 1 files changed, 21 insertions(+), 9 deletions(-) diff --git a/ldap/servers/slapd/entrywsi.c b/ldap/servers/slapd/entrywsi.c index 9923f57..579d044 100644 --- a/ldap/servers/slapd/entrywsi.c +++ b/ldap/servers/slapd/entrywsi.c @@ -46,7 +46,7 @@ #include "slap.h" #include "slapi-plugin.h" -static void resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state); +static void resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state, int delete_priority); static int entry_present_value_to_deleted_value(Slapi_Attr *a, Slapi_Value *v) @@ -477,7 +477,7 @@ entry_add_present_values_wsi(Slapi_Entry *e, const char *type, struct berval **b * Now delete non-RDN values from a->a_present_values; and * restore possible RDN values from a->a_deleted_values */ - resolve_attribute_state(e, a, attr_state); + resolve_attribute_state(e, a, attr_state, 0); retVal= LDAP_SUCCESS; } else @@ -547,7 +547,7 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval attr_set_deletion_csn(a,csn); if(urp) { - resolve_attribute_state(e, a, attr_state); /* ABSOLVED */ + resolve_attribute_state(e, a, attr_state, 1 /* set delete priority */); /* ABSOLVED */ } else { @@ -589,7 +589,7 @@ entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval * should free valuestodelete only (don't call valuearray_free) * [622023] */ slapi_ch_free((void **)&valuestodelete); - resolve_attribute_state(e, a, attr_state); + resolve_attribute_state(e, a, attr_state, 0); retVal= LDAP_SUCCESS; } else @@ -955,7 +955,7 @@ value_distinguished_at_csn(const Slapi_Entry *e, const Slapi_Attr *original_attr } static void -resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribute_state) +resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribute_state, int delete_priority) { int i; const CSN *adcsn= attr_get_deletion_csn(a); @@ -976,7 +976,17 @@ resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribut vdcsn= value_get_csn(v, CSN_TYPE_VALUE_DELETED); vucsn= value_get_csn(v, CSN_TYPE_VALUE_UPDATED); deletedcsn= csn_max(vdcsn, adcsn); - if(csn_compare(vucsn,deletedcsn)<0) /* check if the attribute or value was deleted after the value was last updated */ + + /* Check if the attribute or value was deleted after the value was + * last updated. If the value update CSN and the deleted CSN are + * the same (meaning they are separate mods from the same exact + * operation), we should only delete the value if delete priority + * is set. Delete priority should only be set when we are deleting + * all value of an attribute. This prevents us from leaving a value + * that was added as a previous mod in the same exact modify + * operation as the subsequent delete.*/ + if((csn_compare(vucsn,deletedcsn)<0) || + (delete_priority && (csn_compare(vucsn,deletedcsn) == 0))) { if(!value_distinguished_at_csn(e, a, v, deletedcsn)) { @@ -1000,7 +1010,9 @@ resolve_attribute_state_multi_valued(Slapi_Entry *e, Slapi_Attr *a, int attribut vdcsn= value_get_csn(v, CSN_TYPE_VALUE_DELETED); vucsn= value_get_csn(v, CSN_TYPE_VALUE_UPDATED); deletedcsn= csn_max(vdcsn, adcsn); - if((csn_compare(vucsn,deletedcsn)>=0) || /* check if the attribute or value was deleted after the value was last updated */ + + /* check if the attribute or value was deleted after the value was last updated */ + if((csn_compare(vucsn,deletedcsn)>0) || value_distinguished_at_csn(e, a, v, deletedcsn)) { entry_deleted_value_to_present_value(a,v); @@ -1178,7 +1190,7 @@ resolve_attribute_state_single_valued(Slapi_Entry *e, Slapi_Attr *a, int attribu } static void -resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state) +resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state, int delete_priority) { if(slapi_attr_flag_is_set(a,SLAPI_ATTR_FLAG_SINGLE)) { @@ -1186,6 +1198,6 @@ resolve_attribute_state(Slapi_Entry *e, Slapi_Attr *a, int attribute_state) } else { - resolve_attribute_state_multi_valued(e,a,attribute_state); + resolve_attribute_state_multi_valued(e,a,attribute_state, delete_priority); } } -- 1.6.2.5