From adc4dd06c5b26e451944eac52544a28c5386bb9a Mon Sep 17 00:00:00 2001 From: Ludwig Krispenz Date: Apr 14 2014 08:23:18 +0000 Subject: Ticket 47712 - betxn: retro changelog broken after cancelled transaction Bug Description: If a transaction is aborted (eg failing automember) and the retrocl is either broken (first txn after enabling rcl) or there are gaps in the changenumbers The reason that there are gaps in changenumbers is that if the txn is undone the entry is removed from the RCL database, but the last changnumer was incremented already The reason that the RCL seems to be broken is that the entry is removed from the database, but is still in the entry cache. For the first change the changenumber is always recreated and it detects the entry in the entrycache and returns "already exists". If the gaps would be avoided this would also happen for later failures. Fix Description: If the RCL postop function is called with an error set in the pblock, set a flag that next time a chnagenumber will be assigned it is recreated from the database. It is not possible to remove the CL entry from the entry cache from the RCL plugin, cache functions are not available. The only solution is to avoid keeing RCL entries in the entry cache after their initial creation. Extend the operation flags to trigger a removal from cache in ldbm_back_add() https://fedorahosted.org/389/ticket/47712 Reviewed by: richm, thanks --- diff --git a/ldap/servers/plugins/retrocl/retrocl.h b/ldap/servers/plugins/retrocl/retrocl.h index 7c6865a..052c369 100644 --- a/ldap/servers/plugins/retrocl/retrocl.h +++ b/ldap/servers/plugins/retrocl/retrocl.h @@ -159,6 +159,7 @@ extern void retrocl_set_first_changenumber(changeNumber cn); extern changeNumber retrocl_get_last_changenumber(void); extern void retrocl_commit_changenumber(void); extern void retrocl_release_changenumber(void); +extern void retrocl_set_check_changenumber(void); extern changeNumber retrocl_assign_changenumber(void); extern int retrocl_get_changenumbers(void); extern void retrocl_forget_changenumbers(void); diff --git a/ldap/servers/plugins/retrocl/retrocl_cn.c b/ldap/servers/plugins/retrocl/retrocl_cn.c index f816730..6a39e63 100644 --- a/ldap/servers/plugins/retrocl/retrocl_cn.c +++ b/ldap/servers/plugins/retrocl/retrocl_cn.c @@ -45,6 +45,7 @@ static changeNumber retrocl_internal_cn = 0; static changeNumber retrocl_first_cn = 0; +static int check_last_changenumber = 0; /* * Function: a2changeNumber @@ -408,15 +409,20 @@ changeNumber retrocl_assign_changenumber(void) slapi_rwlock_wrlock(retrocl_cn_lock); - if(retrocl_internal_cn <= retrocl_first_cn){ + if((check_last_changenumber) || + ((retrocl_internal_cn <= retrocl_first_cn) && + (retrocl_internal_cn > 1 ))){ /* the numbers have become out of sync - retrocl_get_changenumbers * gets called only once during startup and it may have had a problem * getting the last changenumber. * If there was any problem then update the lastchangenumber from the changelog db. * This function is being called by only the thread that is actually writing * to the changelog. + * + * after the first change was applied both _cn numbers are 1, that's ok */ - retrocl_update_lastchangenumber(); + retrocl_update_lastchangenumber(); + check_last_changenumber = 0; } retrocl_internal_cn++; cn = retrocl_internal_cn; @@ -425,3 +431,10 @@ changeNumber retrocl_assign_changenumber(void) return cn; } + +void retrocl_set_check_changenumber(void) +{ + slapi_rwlock_wrlock(retrocl_cn_lock); + check_last_changenumber = 1; + slapi_rwlock_unlock(retrocl_cn_lock); +} diff --git a/ldap/servers/plugins/retrocl/retrocl_po.c b/ldap/servers/plugins/retrocl/retrocl_po.c index 0e2abc8..0e32d06 100644 --- a/ldap/servers/plugins/retrocl/retrocl_po.c +++ b/ldap/servers/plugins/retrocl/retrocl_po.c @@ -367,7 +367,8 @@ write_replog_db( newPb = slapi_pblock_new (); slapi_add_entry_internal_set_pb( newPb, e, NULL /* controls */, g_plg_identity[PLUGIN_RETROCL], - 0 /* actions */ ); + /* dont leave entry in cache if main oparation is aborted */ + SLAPI_OP_FLAG_NEVER_CACHE); slapi_add_internal_pb (newPb); slapi_pblock_get( newPb, SLAPI_PLUGIN_INTOP_RESULT, &rc ); slapi_pblock_destroy(newPb); @@ -607,6 +608,10 @@ int retrocl_postob (Slapi_PBlock *pb, int optype) if (rc != LDAP_SUCCESS) { LDAPDebug1Arg(LDAP_DEBUG_TRACE,"not applying change if op failed %d\n",rc); + /* this could also mean that the changenumber is no longer correct + * set a flag to check at next assignment + */ + retrocl_set_check_changenumber(); return SLAPI_PLUGIN_SUCCESS; } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_add.c b/ldap/servers/slapd/back-ldbm/ldbm_add.c index 7834b40..71ebc40 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_add.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_add.c @@ -113,6 +113,7 @@ ldbm_back_add( Slapi_PBlock *pb ) int is_resurect_operation= 0; int is_tombstone_operation= 0; int is_fixup_operation= 0; + int is_remove_from_cache= 0; int is_ruv = 0; /* True if the current entry is RUV */ CSN *opcsn = NULL; entry_address addr = {0}; @@ -131,6 +132,7 @@ ldbm_back_add( Slapi_PBlock *pb ) is_tombstone_operation= operation_is_flag_set(operation,OP_FLAG_TOMBSTONE_ENTRY); is_fixup_operation = operation_is_flag_set(operation, OP_FLAG_REPL_FIXUP); is_ruv = operation_is_flag_set(operation, OP_FLAG_REPL_RUV); + is_remove_from_cache = operation_is_flag_set(operation, OP_FLAG_NEVER_CACHE); inst = (ldbm_instance *) be->be_instance_info; if (inst && inst->inst_ref_count) { @@ -1166,6 +1168,8 @@ common_return: } } } + if (is_remove_from_cache) + CACHE_REMOVE(&inst->inst_cache, addingentry); CACHE_RETURN( &inst->inst_cache, &addingentry ); } if (inst->inst_ref_count) { diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 7fbadb3..6b4d80e 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -221,6 +221,7 @@ NSPR_API(PRUint32) PR_fprintf(struct PRFileDesc* fd, const char *fmt, ...) #define SLAPI_OP_FLAG_NEVER_CHAIN 0x000800 /* Do not chain the operation */ #define SLAPI_OP_FLAG_NO_ACCESS_CHECK 0x10000 /* Do not check for access control - bypass them */ #define SLAPI_OP_FLAG_BYPASS_REFERRALS 0x40000 /* Useful for performing internal operations on read-only replica */ +#define SLAPI_OP_FLAG_NEVER_CACHE 0x200000 /* added entry should not be kept in cache */ #define SLAPI_OC_FLAG_REQUIRED 0x0001 #define SLAPI_OC_FLAG_ALLOWED 0x0002 diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index 2fb0524..3443425 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -442,6 +442,7 @@ char *slapi_filter_to_string_internal( const struct slapi_filter *f, char *buf, #define OP_FLAG_PAGED_RESULTS 0x040000 /* simple paged results */ #define OP_FLAG_SERVER_SIDE_SORTING 0x080000 /* server side sorting */ #define OP_FLAG_REVERSE_CANDIDATE_ORDER 0x100000 /* reverse the search candidate list */ +#define OP_FLAG_NEVER_CACHE 0x200000 /* never keep the entry in cache */ /* reverse search states */ #define REV_STARTED 1