From 66e90fc7f2bbe3b302257d9fc74fa8753dc7aca6 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: Sun, 6 May 2012 21:14:53 -0700 Subject: [PATCH] Trac Ticket #359 - Database RUV could mismatch the one in changelog under the stress https://fedorahosted.org/389/ticket/359 Fix description . csnplRollUp (csnpl.c) - To get the first committed csndata, if there are preceded uncommitted csn's in the csnpl list, this patch skips them and returns the first committed csn. . llistRemoveCurrentAndGetNext (llist.c) - when the last item in the list is removed, tail pointer is initialized, too. . multimaster_preop|bepreop_* (repl5_plugins.c) - process_operation is moved from multimaster_preop_* to multimaster_bepreop_* to avoid the uncommitted csn set in the csnpl (RUV element) by process_operation is left without being committed, which is done at the BE_TXN_POST timing. --- ldap/servers/plugins/replication/csnpl.c | 23 +++-- ldap/servers/plugins/replication/llist.c | 8 +- ldap/servers/plugins/replication/repl5_plugins.c | 111 +++++++++------------- 3 files changed, 63 insertions(+), 79 deletions(-) diff --git a/ldap/servers/plugins/replication/csnpl.c b/ldap/servers/plugins/replication/csnpl.c index 62f4fc4..16cebf1 100644 --- a/ldap/servers/plugins/replication/csnpl.c +++ b/ldap/servers/plugins/replication/csnpl.c @@ -298,28 +298,31 @@ csnplRollUp(CSNPL *csnpl, CSN **first_commited) CSN *largest_committed_csn = NULL; csnpldata *data; PRBool freeit = PR_TRUE; + void *iterator; slapi_rwlock_wrlock (csnpl->csnLock); if (first_commited) { /* Avoid non-initialization issues due to careless callers */ *first_commited = NULL; } - data = (csnpldata *)llistGetHead(csnpl->csnList); - while (NULL != data && data->committed) + data = (csnpldata *)llistGetFirst(csnpl->csnList, &iterator); + while (NULL != data) { if (NULL != largest_committed_csn && freeit) { csn_free(&largest_committed_csn); } - freeit = PR_TRUE; - largest_committed_csn = data->csn; /* Save it */ - if (first_commited && (*first_commited == NULL)) { - *first_commited = data->csn; - freeit = PR_FALSE; + if (data->committed) { + freeit = PR_TRUE; + largest_committed_csn = data->csn; /* Save it */ + if (first_commited && (*first_commited == NULL)) { + *first_commited = data->csn; + freeit = PR_FALSE; + } + data = (csnpldata *)llistRemoveCurrentAndGetNext(csnpl->csnList, &iterator); + } else { + data = (csnpldata *)llistGetNext (csnpl->csnList, &iterator); } - data = (csnpldata*)llistRemoveHead (csnpl->csnList); - slapi_ch_free((void **)&data); - data = (csnpldata *)llistGetHead(csnpl->csnList); } #ifdef DEBUG diff --git a/ldap/servers/plugins/replication/llist.c b/ldap/servers/plugins/replication/llist.c index bd0bc1d..e80f532 100644 --- a/ldap/servers/plugins/replication/llist.c +++ b/ldap/servers/plugins/replication/llist.c @@ -167,10 +167,14 @@ void* llistRemoveCurrentAndGetNext (LList *list, void **iterator) prevNode->next = node->next; _llistDestroyNode (&node, NULL); node = prevNode->next; - if (node) + if (node) { return node->data; - else + } else { + if (list->head->next == NULL) { + list->tail = NULL; + } return NULL; + } } else return NULL; diff --git a/ldap/servers/plugins/replication/repl5_plugins.c b/ldap/servers/plugins/replication/repl5_plugins.c index dfbb80e..558c64a 100644 --- a/ldap/servers/plugins/replication/repl5_plugins.c +++ b/ldap/servers/plugins/replication/repl5_plugins.c @@ -224,21 +224,6 @@ multimaster_preop_add (Slapi_PBlock *pb) * uniqueid should already be an attribute of the replicated entry. */ struct slapi_operation_parameters *op_params; - - /* we don't want to process replicated operations with csn smaller - than the corresponding csn in the consumer's ruv */ - if (!process_operation (pb, csn)) - { - slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, - "replication operation not processed, replica unavailable " - "or csn ignored", 0, 0); - csn_free (&csn); - slapi_ch_free ((void**)&target_uuid); - slapi_ch_free ((void**)&superior_uuid); - - return -1; - } - operation_set_csn(op, csn); slapi_pblock_set( pb, SLAPI_TARGET_UNIQUEID, target_uuid); slapi_pblock_get( pb, SLAPI_OPERATION_PARAMETERS, &op_params ); @@ -348,22 +333,6 @@ multimaster_preop_delete (Slapi_PBlock *pb) } else if (1 == drc) { - /* we don't want to process replicated operations with csn smaller - than the corresponding csn in the consumer's ruv */ - if (!process_operation (pb, csn)) - { - slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, - "replication operation not processed, replica unavailable " - "or csn ignored", 0, 0); - slapi_log_error(SLAPI_LOG_REPL, REPLICATION_SUBSYSTEM, - "%s replication operation not processed, replica unavailable " - "or csn ignored\n", sessionid); - csn_free (&csn); - slapi_ch_free ((void**)&target_uuid); - - return -1; - } - /* * For delete operations, we pass the uniqueid of the deleted entry * to the backend and let it sort out which entry to really delete. @@ -447,22 +416,6 @@ multimaster_preop_modify (Slapi_PBlock *pb) } else if (1 == drc) { - /* we don't want to process replicated operations with csn smaller - than the corresponding csn in the consumer's ruv */ - if (!process_operation (pb, csn)) - { - slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, - "replication operation not processed, replica unavailable " - "or csn ignored", 0, 0); - slapi_log_error(SLAPI_LOG_REPL, REPLICATION_SUBSYSTEM, - "%s replication operation not processed, replica unavailable " - "or csn ignored\n", sessionid); - csn_free (&csn); - slapi_ch_free ((void**)&target_uuid); - - return -1; - } - /* * For modify operations, we pass the uniqueid of the modified entry * to the backend and let it sort out which entry to really modify. @@ -554,22 +507,6 @@ multimaster_preop_modrdn (Slapi_PBlock *pb) * was sent, we decode that as well. */ struct slapi_operation_parameters *op_params; - - /* we don't want to process replicated operations with csn smaller - than the corresponding csn in the consumer's ruv */ - if (!process_operation (pb, csn)) - { - slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, - "replication operation not processed, replica unavailable " - "or csn ignored", 0, 0); - csn_free (&csn); - slapi_ch_free ((void**)&target_uuid); - slapi_ch_free ((void**)&newsuperior_uuid); - ldap_mods_free (modrdn_mods, 1); - - return -1; - } - operation_set_csn(op, csn); slapi_pblock_set(pb, SLAPI_TARGET_UNIQUEID, target_uuid); slapi_pblock_get( pb, SLAPI_OPERATION_PARAMETERS, &op_params ); @@ -706,9 +643,19 @@ multimaster_bepreop_add (Slapi_PBlock *pb) if (!is_fixup_operation) { slapi_pblock_set(pb, SLAPI_TXN_RUV_MODS_FN, - (void *)replica_ruv_smods_for_op); + (void *)replica_ruv_smods_for_op); if (is_replicated_operation) { rc = urp_add_operation(pb); + if (0 == rc) { + const CSN *csn = operation_get_csn(op); + /* we don't want to process replicated operations with csn + * smaller than the corresponding csn in the consumer's ruv */ + rc = process_operation (pb, csn); + if (rc) { + slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, + "replication operation not processed, csn ignored", 0, 0); + } + } } } @@ -731,9 +678,19 @@ multimaster_bepreop_delete (Slapi_PBlock *pb) if(!is_fixup_operation) { slapi_pblock_set(pb, SLAPI_TXN_RUV_MODS_FN, - (void *)replica_ruv_smods_for_op); + (void *)replica_ruv_smods_for_op); if (is_replicated_operation) { rc = urp_delete_operation(pb); + if (0 == rc) { + const CSN *csn = operation_get_csn(op); + /* we don't want to process replicated operations with csn + * smaller than the corresponding csn in the consumer's ruv */ + rc = process_operation (pb, csn); + if (rc) { + slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, + "replication operation not processed, csn ignored", 0, 0); + } + } } } @@ -756,9 +713,19 @@ multimaster_bepreop_modify (Slapi_PBlock *pb) if(!is_fixup_operation) { slapi_pblock_set(pb, SLAPI_TXN_RUV_MODS_FN, - (void *)replica_ruv_smods_for_op); + (void *)replica_ruv_smods_for_op); if (is_replicated_operation) { rc = urp_modify_operation(pb); + if (0 == rc) { + const CSN *csn = operation_get_csn(op); + /* we don't want to process replicated operations with csn + * smaller than the corresponding csn in the consumer's ruv */ + rc = process_operation (pb, csn); + if (rc) { + slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, + "replication operation not processed, csn ignored", 0, 0); + } + } } } @@ -784,9 +751,19 @@ multimaster_bepreop_modrdn (Slapi_PBlock *pb) if(!is_fixup_operation) { slapi_pblock_set(pb, SLAPI_TXN_RUV_MODS_FN, - (void *)replica_ruv_smods_for_op); + (void *)replica_ruv_smods_for_op); if (is_replicated_operation) { rc = urp_modrdn_operation(pb); + if (0 == rc) { + const CSN *csn = operation_get_csn(op); + /* we don't want to process replicated operations with csn + * smaller than the corresponding csn in the consumer's ruv */ + rc = process_operation (pb, csn); + if (rc) { + slapi_send_ldap_result(pb, LDAP_SUCCESS, 0, + "replication operation not processed, csn ignored", 0, 0); + } + } } } -- 1.7.7.6