ldap/servers/plugins/replication/llist.c | 6 +-- ldap/servers/plugins/replication/urp.c | 57 ++++++++++++++++++++++------- ldap/servers/slapd/back-ldbm/ldbm_add.c | 29 +++++++------- ldap/servers/slapd/back-ldbm/ldbm_modrdn.c | 29 +++++++------- 4 files changed, 76 insertions(+), 45 deletions(-)

New commits: commit ff601de4c21224f6fa5b995fd877907671f75575 Author: Noriko Hosoi nhosoi@totoro.usersys.redhat.com Date: Tue May 15 10:43:59 2012 -0700

Trac Ticket #359 - Database RUV could mismatch the one in changelog under the stress

https://fedorahosted.org/389/ticket/359

Fix Description: . Fix for llistRemoveCurrentAndGetNext (llist.c) in commit f0f74b57f81998a325dc7472b9ea9b44c5ff6439 was not correct. . Undo the changes made in ldbm_back_add, ldbm_back_modrdn (ldbm_add.c, ldbm_modrdn.c) in commit f0f74b57f81998a325dc7472b9ea9b44c5ff6439. . urp.c: In the original design, if op_result is LDA_SUCCESS and the return code is -1, the operation is skipped, but the max csn is updated in RUV. Currently, we have no way to do so. This patch eliminates the cases: 2 in urp_add_operation and 1 in urp_modrdn_operation are now LDAP_UNWILLING_TO_PERFORM; 1 in urp_modrdn_operation (the existing entry is the same as the target one) returns 0.

diff --git a/ldap/servers/plugins/replication/llist.c b/ldap/servers/plugins/replication/llist.c index e80f532..05cfa48 100644 --- a/ldap/servers/plugins/replication/llist.c +++ b/ldap/servers/plugins/replication/llist.c @@ -165,14 +165,14 @@ void* llistRemoveCurrentAndGetNext (LList *list, void **iterator) if (node) { prevNode->next = node->next; + if (list->tail == node) { + list->tail = prevNode; + } _llistDestroyNode (&node, NULL); node = prevNode->next; if (node) { return node->data; } else { - if (list->head->next == NULL) { - list->tail = NULL; - } return NULL; } } diff --git a/ldap/servers/plugins/replication/urp.c b/ldap/servers/plugins/replication/urp.c index bc32473..1d8799a 100644 --- a/ldap/servers/plugins/replication/urp.c +++ b/ldap/servers/plugins/replication/urp.c @@ -137,7 +137,17 @@ urp_add_operation( Slapi_PBlock *pb ) * - It could be a replay of the same Add, or * - It could be a UUID generation collision, or */ - op_result = LDAP_SUCCESS; + /* + * This operation won't be replayed. That is, this CSN won't update + * the max csn in RUV. The CSN is left uncommitted in RUV unless an + * error is set to op_result. Just to get rid of this CSN from RUV, + * setting an error to op_result + */ + /* op_result = LDAP_SUCCESS; */ + slapi_log_error(slapi_log_urp, sessionid, + "urp_add (%s): an entry with this uniqueid already exists.\n", + slapi_entry_get_dn_const(existing_uniqueid_entry)); + op_result= LDAP_UNWILLING_TO_PERFORM; slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result); rc= -1; /* Ignore this Operation */ PROFILE_POINT; /* Add Conflict; UniqueID Exists; Ignore */ @@ -249,7 +259,17 @@ urp_add_operation( Slapi_PBlock *pb ) * b) We've seen the Operation before. * Let's go with (b) and ignore the little bastard. */ - op_result= LDAP_SUCCESS; + /* + * This operation won't be replayed. That is, this CSN won't update + * the max csn in RUV. The CSN is left uncommitted in RUV unless an + * error is set to op_result. Just to get rid of this CSN from RUV, + * setting an error to op_result + */ + /* op_result = LDAP_SUCCESS; */ + slapi_log_error(slapi_log_urp, sessionid, + "urp_add (%s): The CSN of the Operation and the Entry DN are the same.", + slapi_entry_get_dn_const(existing_dn_entry)); + op_result= LDAP_UNWILLING_TO_PERFORM; slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result); rc= -1; /* Ignore this Operation */ PROFILE_POINT; /* Add Conflict; Entry Exists; Same CSN */ @@ -311,7 +331,17 @@ urp_modrdn_operation( Slapi_PBlock *pb ) * The Operation CSN is not newer than the DN CSN. * Either we're beaten by another ModRDN or we've applied the op. */ - op_result= LDAP_SUCCESS; + /* op_result= LDAP_SUCCESS; */ + /* + * This operation won't be replayed. That is, this CSN won't update + * the max csn in RUV. The CSN is left uncommitted in RUV unless an + * error is set to op_result. Just to get rid of this CSN from RUV, + * setting an error to op_result + */ + slapi_log_error(slapi_log_urp, sessionid, + "urp_modrdn (%s): operation CSN is newer than the DN CSN.\n", + slapi_entry_get_dn_const(target_entry)); + op_result= LDAP_UNWILLING_TO_PERFORM; slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result); rc= -1; /* Ignore the modrdn */ PROFILE_POINT; /* ModRDN Conflict; Entry with Target DN Exists; OPCSN is not newer. */ @@ -323,8 +353,8 @@ urp_modrdn_operation( Slapi_PBlock *pb ) /* newrdn is no need to be case-ignored (get_rdn_plus_uniqueid) */ slapi_pblock_get(pb, SLAPI_MODRDN_NEWRDN, &newrdn); slapi_pblock_get(pb, SLAPI_TARGET_UNIQUEID, &op_uniqueid); - slapi_pblock_get(pb, SLAPI_MODRDN_PARENT_ENTRY, &parent_entry); - slapi_pblock_get(pb, SLAPI_MODRDN_NEWPARENT_ENTRY, &new_parent_entry); + slapi_pblock_get(pb, SLAPI_MODRDN_PARENT_ENTRY, &parent_entry); + slapi_pblock_get(pb, SLAPI_MODRDN_NEWPARENT_ENTRY, &new_parent_entry); slapi_pblock_get(pb, SLAPI_MODRDN_NEWSUPERIOR_SDN, &newsuperior);

if ( is_tombstone_entry (target_entry) ) @@ -361,26 +391,29 @@ urp_modrdn_operation( Slapi_PBlock *pb ) goto bailout; }

- slapi_pblock_get(pb, SLAPI_MODRDN_EXISTING_ENTRY, &existing_entry); - if(existing_entry!=NULL) + slapi_pblock_get(pb, SLAPI_MODRDN_EXISTING_ENTRY, &existing_entry); + if(existing_entry!=NULL) { /* * An entry with the target DN already exists. - * The smaller dncsn wins. The loser changes its RDN to - * uniqueid+baserdn, and adds operational attribute - * ATTR_NSDS5_REPLCONFLIC + * The smaller dncsn wins. The loser changes its RDN to + * uniqueid+baserdn, and adds operational attribute + * ATTR_NSDS5_REPLCONFLIC */

existing_uniqueid = slapi_entry_get_uniqueid (existing_entry); existing_sdn = slapi_entry_get_sdn_const ( existing_entry);

/* - * Dismiss the operation if the existing entry is the same as the target one. + * It used to dismiss the operation if the existing entry is + * the same as the target one. + * But renaming the RDN with the one which only cases are different, + * cn=ABC --> cn=Abc, this case matches. We should go forward the op. */ if (strcmp(op_uniqueid, existing_uniqueid) == 0) { op_result= LDAP_SUCCESS; slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result); - rc = -1; /* Ignore the op */ + rc = 0; /* Don't ignore the op */ PROFILE_POINT; /* ModRDN Replay */ goto bailout; } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index fc7ed80..1539c7c 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -117,7 +117,6 @@ ldbm_back_add( Slapi_PBlock *pb ) int is_ruv = 0; /* True if the current entry is RUV */ CSN *opcsn = NULL; entry_address addr = {0}; - int val = 0;

slapi_pblock_get( pb, SLAPI_PLUGIN_PRIVATE, &li ); slapi_pblock_get( pb, SLAPI_ADD_ENTRY, &e ); @@ -1044,26 +1043,26 @@ error_return: disk_full = 1; }

- /* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */ - slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val); - if (!val) { - if (!ldap_result_code) { - ldap_result_code = LDAP_OPERATIONS_ERROR; - } - slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); - } - slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val ); - if (!val) { - val = -1; - slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &val ); - } - diskfull_return: if (disk_full) { rc= return_on_disk_full(li); } else { /* It is safer not to abort when the transaction is not started. */ if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) { + /* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */ + int val = 0; + slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val); + if (!val) { + if (!ldap_result_code) { + ldap_result_code = LDAP_OPERATIONS_ERROR; + } + slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); + } + slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val ); + if (!val) { + val = -1; + slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &val ); + } /* call the transaction post add plugins just before the abort */ if ((retval = plugin_call_plugins(pb, SLAPI_PLUGIN_BE_TXN_POST_ADD_FN))) { int opreturn = 0; diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 2e180f2..b3d8111 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -120,7 +120,6 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) const char *newdn = NULL; char *newrdn = NULL; int opreturn = 0; - int val = 0;

/* sdn & parentsdn need to be initialized before "goto *_return" */ slapi_sdn_init(&dn_newdn); @@ -1199,20 +1198,6 @@ error_return: disk_full = 1; }

- /* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */ - slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val); - if (!val) { - if (!ldap_result_code) { - ldap_result_code = LDAP_OPERATIONS_ERROR; - } - slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); - } - slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val ); - if (!val) { - opreturn = -1; - slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &opreturn ); - } - if (disk_full) { retval = return_on_disk_full(li); @@ -1222,6 +1207,20 @@ error_return: /* It is safer not to abort when the transaction is not started. */ if (txn.back_txn_txn && (txn.back_txn_txn != parent_txn)) { + /* make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set */ + int val = 0; + slapi_pblock_get(pb, SLAPI_RESULT_CODE, &val); + if (!val) { + if (!ldap_result_code) { + ldap_result_code = LDAP_OPERATIONS_ERROR; + } + slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ldap_result_code); + } + slapi_pblock_get( pb, SLAPI_PLUGIN_OPRETURN, &val ); + if (!val) { + opreturn = -1; + slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &opreturn ); + } /* call the transaction post modrdn plugins just before the commit */ if ((retval = plugin_call_plugins(pb, SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN))) { LDAPDebug1Arg( LDAP_DEBUG_TRACE, "SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN plugin "