From 45e9bcc65b38aa0df7b1f256227237b9c44a5ecc Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: May 14 2012 22:24:40 +0000 Subject: 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 f92f3a6e91498770aa8e93b6842ba752d4ed279a was not correct. . Undo the changes made in ldbm_back_add, ldbm_back_modrdn (ldbm_add.c, ldbm_modrdn.c) in commit f92f3a6e91498770aa8e93b6842ba752d4ed279a. . 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. (cherry picked from commit 2bead40718b7729df9435c44ab11f796d76848bf) --- 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 25dfd5c..e70ae5b 100644 --- a/ldap/servers/plugins/replication/urp.c +++ b/ldap/servers/plugins/replication/urp.c @@ -139,7 +139,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 */ @@ -251,7 +261,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 */ @@ -315,7 +335,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. */ @@ -326,8 +356,8 @@ urp_modrdn_operation( Slapi_PBlock *pb ) target_sdn = slapi_entry_get_sdn_const (target_entry); 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) ) @@ -364,26 +394,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 132b125..df09c64 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -116,7 +116,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 ); @@ -965,20 +964,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) { - val = -1; - slapi_pblock_set( pb, SLAPI_PLUGIN_OPRETURN, &val ); - } - diskfull_return: if (disk_full) { rc= return_on_disk_full(li); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c index 0b56219..6b0fa34 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -111,7 +111,6 @@ ldbm_back_modrdn( Slapi_PBlock *pb ) entry_address *newsuperior_addr; char ebuf[BUFSIZ]; CSN *opcsn = NULL; - int val = 0; /* sdn & parentsdn need to be initialized before "goto *_return" */ slapi_sdn_init(&dn_newdn); @@ -1066,20 +1065,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) { - val = -1; - slapi_pblock_set(pb, SLAPI_PLUGIN_OPRETURN, &val); - } - if (disk_full) { retval = return_on_disk_full(li);