andykimpe / rpms / 389-ds-base

Forked from rpms/389-ds-base 5 months ago
Clone
Blob Blame History Raw
From 4f722eff41b69dbfc17266ef6545c680b9f7c544 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@redhat.com>
Date: Tue, 9 Jul 2013 11:38:39 -0600
Subject: [PATCH 67/99] Ticket #47392 - ldbm errors when
 adding/modifying/deleting entries

https://fedorahosted.org/389/ticket/47392
Reviewed by: lkrispenz (Thanks!)
Branch: 389-ds-base-1.2.11
Fix Description: The problem is caused by cache consistency issues with the
RUV entry.  Before the txn starts, we grab a pointer to the RUV entry in
the cache.  When DNA (or any betxnpreop plugin) updates the database, it
will also grab a pointer to the cached RUV entry and modify it, out from
under the parent txn.  This can also cause the max CSN in the RUV to go
backwards - the nested txn will have a later CSN which will be
overwritten by the earlier CSN from the parent txn.
The fix is to move the ldbm_ruv_txn code inside the transaction loop after
the betxnpreop plugins have been run.  Also have to add modify_term inside
the retry logic to cancel the modify ruv txn stuff in order to retry.
The other part of the fix is to tell the code that updates the max CSN
in the RUV to skip changes that would cause the RUV to go backwards,
and return a code to the caller that tells the caller that the CSN is
already covered.  The code that updates the RUV for the txn will skip
the modify operations in that case.
Platforms tested: RHEL6 x86_64
Flag Day: no
Doc impact: no
(cherry picked from commit 9bf5ac83289dcadc922a628c652aebabc4725231)
(cherry picked from commit 42abece92028193530296b5d96a6cb5261d9aa61)
---
 ldap/servers/plugins/replication/repl5.h           |  4 +-
 ldap/servers/plugins/replication/repl5_plugins.c   | 29 ++++++--
 ldap/servers/plugins/replication/repl5_replica.c   | 84 ++++++++++++++++------
 .../plugins/replication/repl5_replica_config.c     |  4 +-
 ldap/servers/plugins/replication/repl5_ruv.c       | 55 ++++++++------
 ldap/servers/plugins/replication/repl5_ruv.h       |  1 +
 ldap/servers/slapd/back-ldbm/ldbm_add.c            | 31 ++++----
 ldap/servers/slapd/back-ldbm/ldbm_delete.c         | 32 +++++----
 ldap/servers/slapd/back-ldbm/ldbm_modify.c         | 32 +++++----
 ldap/servers/slapd/back-ldbm/ldbm_modrdn.c         | 31 ++++----
 ldap/servers/slapd/back-ldbm/misc.c                |  1 +
 11 files changed, 205 insertions(+), 99 deletions(-)

diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h
index bd582bc..780b198 100644
--- a/ldap/servers/plugins/replication/repl5.h
+++ b/ldap/servers/plugins/replication/repl5.h
@@ -538,7 +538,7 @@ void   replica_replace_flags (Replica *r, PRUint32 flags);
 void replica_dump(Replica *r);
 void replica_set_enabled (Replica *r, PRBool enable);
 Object *replica_get_replica_from_dn (const Slapi_DN *dn);
-void replica_update_ruv(Replica *replica, const CSN *csn, const char *replica_purl);
+int replica_update_ruv(Replica *replica, const CSN *csn, const char *replica_purl);
 Object *replica_get_replica_for_op (Slapi_PBlock *pb);
 /* the functions below manipulate replica hash */
 int replica_init_name_hash ();
@@ -570,7 +570,7 @@ void replica_set_purge_delay (Replica *r, PRUint32 purge_delay);
 void replica_set_tombstone_reap_interval (Replica *r, long interval);
 void replica_update_ruv_consumer (Replica *r, RUV *supplier_ruv);
 void replica_set_ruv_dirty (Replica *r);
-void replica_write_ruv (Replica *r);
+int replica_write_ruv (Replica *r);
 char *replica_get_dn(Replica *r);
 void replica_check_for_tasks(Replica*r, Slapi_Entry *e);
 
diff --git a/ldap/servers/plugins/replication/repl5_plugins.c b/ldap/servers/plugins/replication/repl5_plugins.c
index e3c3083..7aa2e2c 100644
--- a/ldap/servers/plugins/replication/repl5_plugins.c
+++ b/ldap/servers/plugins/replication/repl5_plugins.c
@@ -932,14 +932,15 @@ copy_operation_parameters(Slapi_PBlock *pb)
  * locally-processed update. This is called for both replicated
  * and non-replicated operations.
  */
-static void
+static int
 update_ruv_component(Replica *replica, CSN *opcsn, Slapi_PBlock *pb)
 {
     PRBool legacy;
     char *purl;
+    int rc = RUV_NOTFOUND;
 
 	if (!replica || !opcsn)
-		return;
+		return rc;
 
 	/* Replica configured, so update its ruv */
 	legacy = replica_is_legacy_consumer (replica);
@@ -948,12 +949,13 @@ update_ruv_component(Replica *replica, CSN *opcsn, Slapi_PBlock *pb)
 	else
 		purl = (char*)replica_get_purl_for_op (replica, pb, opcsn);
 
-	replica_update_ruv(replica, opcsn, purl);
+	rc = replica_update_ruv(replica, opcsn, purl);
 
 	if (legacy)
 	{
 		slapi_ch_free ((void**)&purl);
 	}
+	return rc;
 }
 
 /*
@@ -1115,11 +1117,30 @@ write_changelog_and_ruv (Slapi_PBlock *pb)
 	  just read from the changelog in either the supplier or consumer ruv
 	*/
 	if (0 == return_value) {
+		char csn_str[CSN_STRSIZE];
 		CSN *opcsn;
+		int rc;
 
 		slapi_pblock_get( pb, SLAPI_OPERATION, &op );
 		opcsn = operation_get_csn(op);
-		update_ruv_component(r, opcsn, pb);
+		rc = update_ruv_component(r, opcsn, pb);
+		if (RUV_COVERS_CSN == rc) {
+        		slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
+					"write_changelog_and_ruv: RUV already covers csn for "
+					"%s (uniqid: %s, optype: %lu) csn %s\n",
+					REPL_GET_DN(&op_params->target_address),
+					op_params->target_address.uniqueid,
+					op_params->operation_type,
+					csn_as_string(op_params->csn, PR_FALSE, csn_str));
+		} else if (rc != RUV_SUCCESS) {
+        		slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name,
+					"write_changelog_and_ruv: failed to update RUV for "
+					"%s (uniqid: %s, optype: %lu) to changelog csn %s\n",
+					REPL_GET_DN(&op_params->target_address),
+					op_params->target_address.uniqueid,
+					op_params->operation_type,
+					csn_as_string(op_params->csn, PR_FALSE, csn_str));
+		}
 	}
 
 	object_release (repl_obj);
diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c
index b3df831..8187be9 100644
--- a/ldap/servers/plugins/replication/repl5_replica.c
+++ b/ldap/servers/plugins/replication/repl5_replica.c
@@ -657,10 +657,11 @@ replica_set_ruv (Replica *r, RUV *ruv)
  * inbound replication session operation, and needs to update its
  * local RUV.
  */
-void
+int
 replica_update_ruv(Replica *r, const CSN *updated_csn, const char *replica_purl)
 {
 	char csn_str[CSN_STRSIZE];
+	int rc = RUV_SUCCESS;
 	
 	PR_ASSERT(NULL != r);
 	PR_ASSERT(NULL != updated_csn);
@@ -673,11 +674,13 @@ replica_update_ruv(Replica *r, const CSN *updated_csn, const char *replica_purl)
 	{
 		slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "replica_update_ruv: replica "
 			"is NULL\n");
+		rc = RUV_BAD_DATA;
 	}
 	else if (NULL == updated_csn)
 	{
 		slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "replica_update_ruv: csn "
 			"is NULL when updating replica %s\n", slapi_sdn_get_dn(r->repl_root));
+		rc = RUV_BAD_DATA;
 	}
 	else
 	{
@@ -710,8 +713,17 @@ replica_update_ruv(Replica *r, const CSN *updated_csn, const char *replica_purl)
 					}
 				}
 				/* Update max csn for local and remote replicas */
-				if (ruv_update_ruv (ruv, updated_csn, replica_purl, rid == r->repl_rid) 
-                    != RUV_SUCCESS)
+				rc = ruv_update_ruv (ruv, updated_csn, replica_purl, rid == r->repl_rid);
+				if (RUV_COVERS_CSN == rc)
+				{
+					slapi_log_error(SLAPI_LOG_REPL,
+						repl_plugin_name, "replica_update_ruv: RUV "
+						"for replica %s already covers max_csn = %s\n",
+						slapi_sdn_get_dn(r->repl_root),
+						csn_as_string(updated_csn, PR_FALSE, csn_str));
+					/* RUV is not dirty - no write needed */
+				}
+				else if (RUV_SUCCESS != rc)
 				{
 					slapi_log_error(SLAPI_LOG_FATAL,
 						repl_plugin_name, "replica_update_ruv: unable "
@@ -719,14 +731,18 @@ replica_update_ruv(Replica *r, const CSN *updated_csn, const char *replica_purl)
 						slapi_sdn_get_dn(r->repl_root),
 						csn_as_string(updated_csn, PR_FALSE, csn_str));
 				}
-			
-				r->repl_ruv_dirty = PR_TRUE;
+				else
+				{
+					/* RUV updated - mark as dirty */
+					r->repl_ruv_dirty = PR_TRUE;
+				}
 			}
 			else
 			{
 				slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name,
 					"replica_update_ruv: unable to get RUV object for replica "
 					"%s\n", slapi_sdn_get_dn(r->repl_root));
+				rc = RUV_NOTFOUND;
 			}
 		}
 		else
@@ -734,9 +750,11 @@ replica_update_ruv(Replica *r, const CSN *updated_csn, const char *replica_purl)
 			slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "replica_update_ruv: "
 				"unable to initialize RUV for replica %s\n",
 				slapi_sdn_get_dn(r->repl_root));
+			rc = RUV_NOTFOUND;
 		}
 		PR_Unlock(r->repl_lock);
 	}
+	return rc;
 }
 
 /* 
@@ -2400,7 +2418,11 @@ _replica_update_state (time_t when, void *arg)
 	{
 		/* EY: the consumer needs to flush ruv to disk. */
 		PR_Unlock(r->repl_lock);
-		replica_write_ruv(r);
+		if (replica_write_ruv(r)) {
+			slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
+				"_replica_update_state: failed write RUV for %s\n",
+				slapi_sdn_get_dn (r->repl_root));
+		}
 		goto done;
 	}
 	
@@ -2471,7 +2493,11 @@ _replica_update_state (time_t when, void *arg)
 	}
 
 	/* update RUV - performs its own locking */
-	replica_write_ruv (r);
+	if (replica_write_ruv(r)) {
+		slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
+			"_replica_update_state: failed write RUV for %s\n",
+			slapi_sdn_get_dn (r->repl_root));
+	}
 
 	/* since this is the only place this value is changed and we are 
 	   guaranteed that only one thread enters the function, its ok
@@ -2487,10 +2513,10 @@ done:
 		object_release (replica_object);
 }
 
-void
+int
 replica_write_ruv (Replica *r)
 {	
-	int rc;
+	int rc = LDAP_SUCCESS;
 	Slapi_Mod smod;
 	Slapi_Mod smod_last_modified;
 	LDAPMod *mods [3];	 
@@ -2503,7 +2529,7 @@ replica_write_ruv (Replica *r)
     if (!r->repl_ruv_dirty)
     {
         PR_Unlock(r->repl_lock);
-        return;
+        return rc;
     }
 
 	PR_ASSERT (r->repl_ruv);
@@ -2560,6 +2586,8 @@ replica_write_ruv (Replica *r)
 	slapi_mod_done (&smod);
 	slapi_mod_done (&smod_last_modified);
 	slapi_pblock_destroy (pb);
+
+	return rc;
 }
 
 
@@ -2580,6 +2608,7 @@ replica_ruv_smods_for_op( Slapi_PBlock *pb, char **uniqueid, Slapi_Mods **smods
     Slapi_Mod smod_last_modified;
     Slapi_Operation *op;
     Slapi_Entry *target_entry = NULL;
+    int rc = 0;
 
     slapi_pblock_get(pb, SLAPI_ENTRY_PRE_OP, &target_entry);
     if (target_entry && is_ruv_tombstone_entry(target_entry)) {
@@ -2618,19 +2647,32 @@ replica_ruv_smods_for_op( Slapi_PBlock *pb, char **uniqueid, Slapi_Mods **smods
     object_release (ruv_obj);
     object_release (replica_obj);
 
-    ruv_set_max_csn( ruv_copy, opcsn, NULL );
-
-    ruv_to_smod( ruv_copy, &smod );
-    ruv_last_modified_to_smod( ruv_copy, &smod_last_modified );
+    rc = ruv_set_max_csn_ext( ruv_copy, opcsn, NULL, PR_TRUE );
+    if (rc == RUV_COVERS_CSN) { /* change would "revert" RUV - ignored */
+        rc = 0; /* tell caller to ignore */
+    } else if (rc == RUV_SUCCESS) {
+        rc = 1; /* tell caller success */
+    } else { /* error */
+        rc = -1; /* tell caller error */
+    }
 
+    if (rc == 1) {
+        ruv_to_smod( ruv_copy, &smod );
+        ruv_last_modified_to_smod( ruv_copy, &smod_last_modified );
+    }
     ruv_destroy( &ruv_copy );
 
-    *smods = slapi_mods_new();
-    slapi_mods_add_smod(*smods, &smod);
-    slapi_mods_add_smod(*smods, &smod_last_modified);
-    *uniqueid = slapi_ch_strdup( RUV_STORAGE_ENTRY_UNIQUEID );
+    if (rc == 1) {
+        *smods = slapi_mods_new();
+        slapi_mods_add_smod(*smods, &smod);
+        slapi_mods_add_smod(*smods, &smod_last_modified);
+        *uniqueid = slapi_ch_strdup( RUV_STORAGE_ENTRY_UNIQUEID );
+    } else {
+        *smods = NULL;
+        *uniqueid = NULL;
+    }
 
-    return (1);
+    return rc;
 }
 
 
@@ -3375,7 +3417,9 @@ replica_strip_cleaned_rids(Replica *r)
     while(rid[i] != 0){
         ruv_delete_replica(ruv, rid[i]);
         replica_set_ruv_dirty(r);
-        replica_write_ruv(r);
+        if (replica_write_ruv(r)) {
+		slapi_log_error (SLAPI_LOG_REPL, "replica_strip_cleaned_rids", "failed to write RUV\n");
+        }
         i++;
     }
     object_release(RUVObj);
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c
index bbbe87e..7c625eb 100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -1200,7 +1200,9 @@ replica_execute_cleanruv_task (Object *r, ReplicaId rid, char *returntext /* not
 	}
 	rc = ruv_delete_replica(local_ruv, rid);
 	replica_set_ruv_dirty(replica);
-	replica_write_ruv(replica);
+	if (replica_write_ruv(replica)) {
+		slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name, "cleanruv_task: could not write RUV\n");
+	}
 	object_release(RUVObj);
 
 	/* Update Mapping Tree to reflect RUV changes */
diff --git a/ldap/servers/plugins/replication/repl5_ruv.c b/ldap/servers/plugins/replication/repl5_ruv.c
index 8fbd89c..2808c58 100644
--- a/ldap/servers/plugins/replication/repl5_ruv.c
+++ b/ldap/servers/plugins/replication/repl5_ruv.c
@@ -665,26 +665,34 @@ set_min_csn_nolock(RUV *ruv, const CSN *min_csn, const char *replica_purl)
 }
 
 static int
-set_max_csn_nolock(RUV *ruv, const CSN *max_csn, const char *replica_purl)
+set_max_csn_nolock_ext(RUV *ruv, const CSN *max_csn, const char *replica_purl, PRBool must_be_greater)
 {
-	int return_value;
+	int return_value = RUV_SUCCESS;
 	ReplicaId rid = csn_get_replicaid (max_csn);
 	RUVElement *replica = ruvGetReplica (ruv, rid);
-    if (NULL == replica)
-    {
-	    replica = ruvAddReplica (ruv, max_csn, replica_purl);
-        if (replica)
-            return_value = RUV_SUCCESS;
-        else
-            return_value = RUV_MEMORY_ERROR;
-	}
-	else
-	{
-        if (replica_purl && replica->replica_purl == NULL)
-            replica->replica_purl = slapi_ch_strdup (replica_purl);    
-		csn_free(&replica->csn);
-		replica->csn = csn_dup(max_csn);
-		replica->last_modified = current_time();
+	if (NULL == replica) {
+		replica = ruvAddReplica (ruv, max_csn, replica_purl);
+		if (replica)
+			return_value = RUV_SUCCESS;
+		else
+			return_value = RUV_MEMORY_ERROR;
+	} else {
+		if (replica_purl && replica->replica_purl == NULL)
+			replica->replica_purl = slapi_ch_strdup (replica_purl);
+		if (!must_be_greater || (csn_compare(replica->csn, max_csn) < 0)) {
+			csn_free(&replica->csn);
+			replica->csn = csn_dup(max_csn);
+			replica->last_modified = current_time();
+		} else {
+			char csn1[CSN_STRSIZE+1];
+			char csn2[CSN_STRSIZE+1];
+			slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
+			                "set_max_csn_nolock_ext: new CSN [%s] for replica ID [%d] "
+			                "is less than the existing max CSN [%s] - ignoring\n",
+			                csn_as_string(max_csn, PR_FALSE, csn1), rid,
+			                csn_as_string(replica->csn, PR_FALSE, csn2));
+			return_value = RUV_COVERS_CSN;
+		}
 		return_value = RUV_SUCCESS;
 	}
 	return return_value;
@@ -704,9 +712,15 @@ ruv_set_min_csn(RUV *ruv, const CSN *min_csn, const char *replica_purl)
 int
 ruv_set_max_csn(RUV *ruv, const CSN *max_csn, const char *replica_purl)
 {
+	return ruv_set_max_csn_ext(ruv, max_csn, replica_purl, PR_FALSE);
+}
+
+int
+ruv_set_max_csn_ext(RUV *ruv, const CSN *max_csn, const char *replica_purl, PRBool must_be_greater)
+{
 	int return_value;
 	slapi_rwlock_wrlock (ruv->lock);
-	return_value = set_max_csn_nolock(ruv, max_csn, replica_purl);
+	return_value = set_max_csn_nolock_ext(ruv, max_csn, replica_purl, must_be_greater);
 	slapi_rwlock_unlock (ruv->lock);
 	return return_value;
 }
@@ -1654,8 +1668,9 @@ int ruv_update_ruv (RUV *ruv, const CSN *csn, const char *replica_purl, PRBool i
 		   * generated by this replica, we need to set the first_csn as the min csn in the
 		   * ruv */
 		  set_min_csn_nolock(ruv, first_csn, replica_purl);
-		} 
-		set_max_csn_nolock(ruv, max_csn, replica_purl);
+		}
+		/* only update the max_csn in the RUV if it is greater than the existing one */
+		rc = set_max_csn_nolock_ext(ruv, max_csn, replica_purl, PR_TRUE /* must be greater */);
 		/* It is possible that first_csn points to max_csn.
 		   We need to free it once */
 		if (max_csn != first_csn) {
diff --git a/ldap/servers/plugins/replication/repl5_ruv.h b/ldap/servers/plugins/replication/repl5_ruv.h
index 944f5ed..fdc8af2 100644
--- a/ldap/servers/plugins/replication/repl5_ruv.h
+++ b/ldap/servers/plugins/replication/repl5_ruv.h
@@ -108,6 +108,7 @@ int ruv_get_smallest_csn_for_replica(const RUV *ruv, ReplicaId rid, CSN **csn);
 int ruv_set_csns(RUV *ruv, const CSN *csn, const char *replica_purl);  
 int ruv_set_csns_keep_smallest(RUV *ruv, const CSN *csn);  
 int ruv_set_max_csn(RUV *ruv, const CSN *max_csn, const char *replica_purl);
+int ruv_set_max_csn_ext(RUV *ruv, const CSN *max_csn, const char *replica_purl, PRBool must_be_greater);
 int ruv_set_min_csn(RUV *ruv, const CSN *min_csn, const char *replica_purl);
 const char *ruv_get_purl_for_replica(const RUV *ruv, ReplicaId rid);
 char *ruv_get_replica_generation (const RUV *ruv);
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c
index 78ca565..554822c 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_add.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c
@@ -668,19 +668,6 @@ ldbm_back_add( Slapi_PBlock *pb )
 		parententry = NULL;
 	}
 
-	if (!is_ruv && !is_fixup_operation) {
-		ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
-		if (-1 == ruv_c_init) {
-			LDAPDebug( LDAP_DEBUG_ANY,
-				"ldbm_back_add: ldbm_txn_ruv_modify_context "
-				"failed to construct RUV modify context\n",
-				0, 0, 0);
-			ldap_result_code= LDAP_OPERATIONS_ERROR;
-			retval = 0;
-			goto error_return;
-		}
-	}
-
 	if ( (originalentry = backentry_dup(addingentry )) == NULL ) {
 		ldap_result_code= LDAP_OPERATIONS_ERROR;
 		goto error_return;
@@ -718,6 +705,11 @@ ldbm_back_add( Slapi_PBlock *pb )
 					goto error_return;
 				}
 			}
+			if (ruv_c_init) {
+				/* reset the ruv txn stuff */
+				modify_term(&ruv_c, be);
+				ruv_c_init = 0;
+			}
 
 			/* We're re-trying */
 			LDAPDebug0Args(LDAP_DEBUG_BACKLDBM, "Add Retrying Transaction\n");
@@ -910,6 +902,19 @@ ldbm_back_add( Slapi_PBlock *pb )
 			}
 		}
 
+		if (!is_ruv && !is_fixup_operation) {
+			ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
+			if (-1 == ruv_c_init) {
+				LDAPDebug( LDAP_DEBUG_ANY,
+					"ldbm_back_add: ldbm_txn_ruv_modify_context "
+					"failed to construct RUV modify context\n",
+					0, 0, 0);
+				ldap_result_code= LDAP_OPERATIONS_ERROR;
+				retval = 0;
+				goto error_return;
+			}
+		}
+
 		if (ruv_c_init) {
 			retval = modify_update_all( be, pb, &ruv_c, &txn );
 			if (DB_LOCK_DEADLOCK == retval) {
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
index d80c54e..3cf58f4 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c
@@ -451,19 +451,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
 		}
 	}
 
-	if (!is_ruv && !is_fixup_operation && !delete_tombstone_entry) {
-		ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
-		if (-1 == ruv_c_init) {
-			LDAPDebug( LDAP_DEBUG_ANY,
-				"ldbm_back_delete: ldbm_txn_ruv_modify_context "
-				"failed to construct RUV modify context\n",
-				0, 0, 0);
-			ldap_result_code= LDAP_OPERATIONS_ERROR;
-			retval = 0;
-			goto error_return;
-		}
-	}
-
 	/*
 	 * So, we believe that no code up till here actually added anything
 	 * to the persistent store. From now on, we're transacted
@@ -511,6 +498,12 @@ ldbm_back_delete( Slapi_PBlock *pb )
 				e_in_cache = 1;
 			}
 
+			if (ruv_c_init) {
+				/* reset the ruv txn stuff */
+				modify_term(&ruv_c, be);
+				ruv_c_init = 0;
+			}
+
 			/* We're re-trying */
 			LDAPDebug0Args(LDAP_DEBUG_BACKLDBM,
 			               "Delete Retrying Transaction\n");
@@ -991,6 +984,19 @@ ldbm_back_delete( Slapi_PBlock *pb )
 			}
 		}
 
+		if (!is_ruv && !is_fixup_operation && !delete_tombstone_entry) {
+			ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
+			if (-1 == ruv_c_init) {
+				LDAPDebug( LDAP_DEBUG_ANY,
+					"ldbm_back_delete: ldbm_txn_ruv_modify_context "
+					"failed to construct RUV modify context\n",
+					0, 0, 0);
+				ldap_result_code= LDAP_OPERATIONS_ERROR;
+				retval = 0;
+				goto error_return;
+			}
+		}
+
 		if (ruv_c_init) {
 			retval = modify_update_all( be, pb, &ruv_c, &txn );
 			if (DB_LOCK_DEADLOCK == retval) {
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
index 66b8ab8..32510ab 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modify.c
@@ -476,19 +476,6 @@ ldbm_back_modify( Slapi_PBlock *pb )
 		goto error_return;
 	}
 
-	if (!is_ruv && !is_fixup_operation) {
-		ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
-		if (-1 == ruv_c_init) {
-			LDAPDebug( LDAP_DEBUG_ANY,
-				"ldbm_back_modify: ldbm_txn_ruv_modify_context "
-				"failed to construct RUV modify context\n",
-				0, 0, 0);
-			ldap_result_code= LDAP_OPERATIONS_ERROR;
-			retval = 0;
-			goto error_return;
-		}
-	}
-
 	/*
 	 * Grab a copy of the mods and the entry in case the be_txn_preop changes
 	 * the them.  If we have a failure, then we need to reset the mods to their
@@ -526,6 +513,12 @@ ldbm_back_modify( Slapi_PBlock *pb )
 				goto error_return;
 			}
 
+			if (ruv_c_init) {
+				/* reset the ruv txn stuff */
+				modify_term(&ruv_c, be);
+				ruv_c_init = 0;
+			}
+
 			LDAPDebug0Args(LDAP_DEBUG_BACKLDBM,
 			               "Modify Retrying Transaction\n");
 #ifndef LDBM_NO_BACKOFF_DELAY
@@ -638,6 +631,19 @@ ldbm_back_modify( Slapi_PBlock *pb )
 
 		}
 
+		if (!is_ruv && !is_fixup_operation) {
+			ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
+			if (-1 == ruv_c_init) {
+				LDAPDebug( LDAP_DEBUG_ANY,
+					"ldbm_back_modify: ldbm_txn_ruv_modify_context "
+					"failed to construct RUV modify context\n",
+					0, 0, 0);
+				ldap_result_code= LDAP_OPERATIONS_ERROR;
+				retval = 0;
+				goto error_return;
+			}
+		}
+
 		if (ruv_c_init) {
 			retval = modify_update_all( be, pb, &ruv_c, &txn );
 			if (DB_LOCK_DEADLOCK == retval) {
diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
index 69fc053..b5cb90b 100644
--- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
+++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c
@@ -712,19 +712,6 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
         /* JCM - A subtree move could break ACIs, static groups, and dynamic groups. */
     }
 
-    if (!is_ruv && !is_fixup_operation) {
-        ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
-        if (-1 == ruv_c_init) {
-            LDAPDebug( LDAP_DEBUG_ANY,
-                "ldbm_back_modrdn: ldbm_txn_ruv_modify_context "
-                "failed to construct RUV modify context\n",
-                0, 0, 0);
-            ldap_result_code = LDAP_OPERATIONS_ERROR;
-            retval = 0;
-            goto error_return;
-        }
-    }
-
     /*
      * make copies of the originals, no need to copy the mods because
      * we have already copied them
@@ -826,6 +813,11 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
                 goto error_return;
             }
 
+	    if (ruv_c_init) {
+		/* reset the ruv txn stuff */
+		modify_term(&ruv_c, be);
+		ruv_c_init = 0;
+	    }
             /* We're re-trying */
             LDAPDebug0Args(LDAP_DEBUG_BACKLDBM,
                            "Modrdn Retrying Transaction\n");
@@ -1028,6 +1020,19 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
             goto error_return;
         }
 
+        if (!is_ruv && !is_fixup_operation) {
+            ruv_c_init = ldbm_txn_ruv_modify_context( pb, &ruv_c );
+            if (-1 == ruv_c_init) {
+                LDAPDebug( LDAP_DEBUG_ANY,
+                    "ldbm_back_modrdn: ldbm_txn_ruv_modify_context "
+                    "failed to construct RUV modify context\n",
+                    0, 0, 0);
+                ldap_result_code = LDAP_OPERATIONS_ERROR;
+                retval = 0;
+                goto error_return;
+            }
+        }
+
         if (ruv_c_init) {
             retval = modify_update_all( be, pb, &ruv_c, &txn );
             if (DB_LOCK_DEADLOCK == retval) {
diff --git a/ldap/servers/slapd/back-ldbm/misc.c b/ldap/servers/slapd/back-ldbm/misc.c
index fd62df9..7a90742 100644
--- a/ldap/servers/slapd/back-ldbm/misc.c
+++ b/ldap/servers/slapd/back-ldbm/misc.c
@@ -420,6 +420,7 @@ ldbm_txn_ruv_modify_context( Slapi_PBlock *pb, modify_context *mc )
     /* Either something went wrong when the RUV callback tried to assemble
      * the updates for us, or there were no updates because the op doesn't
      * target a replica. */
+    /* or, the CSN is already covered by the RUV */
     if (1 != rc || NULL == smods || NULL == uniqueid) {
         return (rc);
     }
-- 
1.8.1.4