Ticket #47392 - ldbm errors when adding/modifying/deleting entries

https://fedorahosted.org/389/ticket/47392
Reviewed by: lkrispenz (Thanks!)
Branch: master
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

ldap/servers/plugins/replication/repl5.h

@@ -558,7 +558,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 ();
@@ -591,8 +591,8 @@ 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);
 Slapi_Entry *get_in_memory_ruv(Slapi_DN *suffix_sdn);
+int replica_write_ruv (Replica *r);
 char *replica_get_dn(Replica *r);
 void replica_check_for_tasks(Replica*r, Slapi_Entry *e);
 void replica_update_state (time_t when, void *arg);

ldap/servers/plugins/replication/repl5_plugins.c

@@ -1114,14 +1114,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);
@@ -1130,12 +1131,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;
 }
 
 /*
@@ -1297,11 +1299,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);

ldap/servers/plugins/replication/repl5_replica.c

@@ -665,10 +665,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);
@@ -681,11 +682,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
 	{
@@ -718,8 +721,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 "
@@ -727,14 +739,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
@@ -742,9 +758,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;
 }
 
 /* 
@@ -2462,7 +2480,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;
 	}
 	
@@ -2533,7 +2555,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
@@ -2549,10 +2575,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];	 
@@ -2565,7 +2591,7 @@ replica_write_ruv (Replica *r)
     if (!r->repl_ruv_dirty)
     {
         PR_Unlock(r->repl_lock);
-        return;
+        return rc;
     }
 
 	PR_ASSERT (r->repl_ruv);
@@ -2622,6 +2648,8 @@ replica_write_ruv (Replica *r)
 	slapi_mod_done (&smod);
 	slapi_mod_done (&smod_last_modified);
 	slapi_pblock_destroy (pb);
+
+	return rc;
 }
 
 
@@ -2642,6 +2670,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)) {
@@ -2680,19 +2709,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;
 }
 
 
@@ -3437,7 +3479,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);

ldap/servers/plugins/replication/repl5_replica_config.c

@@ -1304,7 +1304,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 */

ldap/servers/plugins/replication/repl5_ruv.c

@@ -677,26 +677,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;
@@ -715,10 +723,16 @@ 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;
 }
@@ -1742,8 +1756,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) {

ldap/servers/plugins/replication/repl5_ruv.h

@@ -113,6 +113,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);

ldap/servers/slapd/back-ldbm/ldbm_add.c

@@ -344,6 +344,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");
@@ -759,19 +764,6 @@ ldbm_back_add( Slapi_PBlock *pb )
 				parententry = NULL;
 			}
 		
-			if (!is_ruv && !is_fixup_operation && !NO_RUV_UPDATE(li)) {
-				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;
@@ -952,6 +944,19 @@ ldbm_back_add( Slapi_PBlock *pb )
 			}
 		}
 
+		if (!is_ruv && !is_fixup_operation && !NO_RUV_UPDATE(li)) {
+			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) {

ldap/servers/slapd/back-ldbm/ldbm_delete.c

@@ -235,6 +235,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");
@@ -550,19 +556,6 @@ ldbm_back_delete( Slapi_PBlock *pb )
 					goto error_return;
 				}
 			}
-
-			if (!is_ruv && !is_fixup_operation && !delete_tombstone_entry && !NO_RUV_UPDATE(li)) {
-				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 (0 == retry_count) just once */
 		else {
 			/* call the transaction pre delete plugins not just once
@@ -1028,6 +1021,19 @@ ldbm_back_delete( Slapi_PBlock *pb )
 			}
 		}
 
+		if (!is_ruv && !is_fixup_operation && !delete_tombstone_entry && !NO_RUV_UPDATE(li)) {
+			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) {

ldap/servers/slapd/back-ldbm/ldbm_modify.c

@@ -445,6 +445,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
@@ -560,19 +566,6 @@ ldbm_back_modify( Slapi_PBlock *pb )
 				goto error_return;
 			}
 		
-			if (!is_ruv && !is_fixup_operation && !NO_RUV_UPDATE(li)) {
-				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
@@ -679,6 +672,19 @@ ldbm_back_modify( Slapi_PBlock *pb )
 
 		}
 
+		if (!is_ruv && !is_fixup_operation && !NO_RUV_UPDATE(li)) {
+			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) {

ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

@@ -309,6 +309,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");
@@ -846,19 +851,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 && !NO_RUV_UPDATE(li)) {
-                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
@@ -1065,6 +1057,19 @@ ldbm_back_modrdn( Slapi_PBlock *pb )
             goto error_return;
         }
 
+	if (!is_ruv && !is_fixup_operation && !NO_RUV_UPDATE(li)) {
+            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) {

ldap/servers/slapd/back-ldbm/misc.c

@@ -426,6 +426,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);
     }