From c89ea2f6b9aebeb8d2190ef9e4d1eee78bd63f06 Mon Sep 17 00:00:00 2001 From: Noriko Hosoi Date: May 09 2012 22:51:58 +0000 Subject: 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. . ldbm_back_add, ldbm_back_modrdn (ldbm_add.c, ldbm_modrdn.c) - make sure SLAPI_RESULT_CODE and SLAPI_PLUGIN_OPRETURN are set not just when the transaction is started, but in general. If an error occurs the RESULT_CODE triggers to remove the CSN from the RUV element. . plugin_call_func (plugin.c) - when the plugin type is be pre/ post op, respect the fatal error code (-1) instead of OR the results from all the plugins. The error code -1 is checked in ldap_back_add and ldbm_back_modrdn to distinguish from the URP operation bits. (cherry-picked from commit f0f74b57f81998a325dc7472b9ea9b44c5ff6439) --- 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/usn/usn.c b/ldap/servers/plugins/usn/usn.c index 0d8a065..812d83d 100644 --- a/ldap/servers/plugins/usn/usn.c +++ b/ldap/servers/plugins/usn/usn.c @@ -501,6 +501,7 @@ bail: /* * usn_bepreop_modify - add/replace next USN to the mods; * shared by modify and modrdn + * Note: bepreop should not return other than LDAP_SUCCESS. */ static int usn_bepreop_modify (Slapi_PBlock *pb) @@ -516,7 +517,8 @@ usn_bepreop_modify (Slapi_PBlock *pb) slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods); slapi_pblock_get(pb, SLAPI_BACKEND, &be); if (NULL == be) { - rc = LDAP_PARAM_ERROR; + slapi_log_error(SLAPI_LOG_FATAL, USN_PLUGIN_SUBSYSTEM, + "usn_bepreop_modify: no backend.\n"); goto bail; } if (LDAP_SUCCESS == _usn_mod_next_usn(&mods, be)) { diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index 1539c7c..fc7ed80 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -117,6 +117,7 @@ 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 ); @@ -1043,26 +1044,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 b3d8111..2e180f2 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_modrdn.c @@ -120,6 +120,7 @@ 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); @@ -1198,6 +1199,20 @@ 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); @@ -1207,20 +1222,6 @@ 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 " diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c index 1585779..8aa95ac 100644 --- a/ldap/servers/slapd/plugin.c +++ b/ldap/servers/slapd/plugin.c @@ -1466,8 +1466,14 @@ plugin_call_func (struct slapdplugin *list, int operation, Slapi_PBlock *pb, int } else if (SLAPI_PLUGIN_BEPREOPERATION == list->plg_type || SLAPI_PLUGIN_BEPOSTOPERATION == list->plg_type) { - /* OR the result into the return value for be pre/postops */ - return_value |= rc; + /* respect fatal error (-1); should not OR it */ + if (-1 == rc) { + return_value = rc; + } else if (-1 != return_value) { + /* OR the result into the return value + * for be pre/postops */ + return_value |= rc; + } } } /* counters_to_errors_log("after plugin call"); */