From dace0f15c0b19efbd6af3ef2d1a8f7992b90c140 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Jul 10 2013 17:12:48 +0000 Subject: 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 --- diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h index 8da2d3b..0fa7ee4 100644 --- a/ldap/servers/plugins/replication/repl5.h +++ b/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); diff --git a/ldap/servers/plugins/replication/repl5_plugins.c b/ldap/servers/plugins/replication/repl5_plugins.c index 5981d0c..5d1a776 100644 --- a/ldap/servers/plugins/replication/repl5_plugins.c +++ b/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); diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c index 8121901..5aa58f0 100644 --- a/ldap/servers/plugins/replication/repl5_replica.c +++ b/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); diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c index 1aa69f0..b6a2d72 100644 --- a/ldap/servers/plugins/replication/repl5_replica_config.c +++ b/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 */ diff --git a/ldap/servers/plugins/replication/repl5_ruv.c b/ldap/servers/plugins/replication/repl5_ruv.c index d882326..b68223b 100644 --- a/ldap/servers/plugins/replication/repl5_ruv.c +++ b/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; @@ -716,9 +724,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; } @@ -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) { diff --git a/ldap/servers/plugins/replication/repl5_ruv.h b/ldap/servers/plugins/replication/repl5_ruv.h index 4d84662..a5a0c19 100644 --- a/ldap/servers/plugins/replication/repl5_ruv.h +++ b/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); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index c7d1f62..0cd9432 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/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) { diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index 0478a12..61af3a6 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/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) { diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modify.c b/ldap/servers/slapd/back-ldbm/ldbm_modify.c index c00194b..d2302fe 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modify.c +++ b/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) { diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index fe53554..416b928 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/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) { diff --git a/ldap/servers/slapd/back-ldbm/misc.c b/ldap/servers/slapd/back-ldbm/misc.c index 87262c5..00d48b3 100644 --- a/ldap/servers/slapd/back-ldbm/misc.c +++ b/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); }