From 42060f20853f9dbffb679a3ff4ec00c2732cab2e Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Tue, 8 Apr 2014 14:12:32 -0400 Subject: [PATCH 195/225] Ticket 47767 - Nested tombstones become orphaned after purge Bug Description: If there are nested tombstone entries, the parents will always be purged first, which leaves its child entries orphaned. Fix Description: When doing the tombstone purge, process the candidate list in reverse order, which will remove the child entries before the parent entries. https://fedorahosted.org/389/ticket/47767 Reviewed by: nhosoi(Thanks!) (cherry picked from commit 48e2a96cc3369db2b8d739889be273700ca9c23b) --- ldap/servers/plugins/replication/repl5_replica.c | 8 +++- ldap/servers/slapd/back-ldbm/idl_common.c | 6 +++ ldap/servers/slapd/back-ldbm/ldbm_delete.c | 1 + ldap/servers/slapd/back-ldbm/ldbm_search.c | 47 +++++++++++++++++++++++- ldap/servers/slapd/back-ldbm/proto-back-ldbm.h | 1 + ldap/servers/slapd/operation.c | 1 + ldap/servers/slapd/slap.h | 1 + ldap/servers/slapd/slapi-plugin.h | 8 ++-- ldap/servers/slapd/slapi-private.h | 44 ++++++++++++---------- 9 files changed, 89 insertions(+), 28 deletions(-) diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c index 82c30c1..1cec805 100644 --- a/ldap/servers/plugins/replication/repl5_replica.c +++ b/ldap/servers/plugins/replication/repl5_replica.c @@ -2793,7 +2793,10 @@ process_reap_entry (Slapi_Entry *entry, void *cb_data) "%s\n", slapi_entry_get_dn(entry)); } } - (*num_entriesp)++; + if(!is_ruv_tombstone_entry(entry)){ + /* Don't update the count for the database tombstone entry */ + (*num_entriesp)++; + } return 0; } @@ -2878,7 +2881,8 @@ _replica_reap_tombstones(void *arg) slapi_search_internal_set_pb(pb, slapi_sdn_get_dn(replica->repl_root), LDAP_SCOPE_SUBTREE, "(&(objectclass=nstombstone)(nscpentrydn=*))", attrs, 0, ctrls, NULL, - repl_get_plugin_identity(PLUGIN_MULTIMASTER_REPLICATION), 0); + repl_get_plugin_identity(PLUGIN_MULTIMASTER_REPLICATION), + OP_FLAG_REVERSE_CANDIDATE_ORDER); cb_data.rc = 0; cb_data.num_entries = 0UL; diff --git a/ldap/servers/slapd/back-ldbm/idl_common.c b/ldap/servers/slapd/back-ldbm/idl_common.c index 584bba5..ee59ee2 100644 --- a/ldap/servers/slapd/back-ldbm/idl_common.c +++ b/ldap/servers/slapd/back-ldbm/idl_common.c @@ -494,3 +494,9 @@ ID idl_iterator_dereference_increment(idl_iterator *i, const IDList *idl) return t; } +ID idl_iterator_dereference_decrement(idl_iterator *i, const IDList *idl) +{ + idl_iterator_decrement(i); + return idl_iterator_dereference(*i,idl); + +} diff --git a/ldap/servers/slapd/back-ldbm/ldbm_delete.c b/ldap/servers/slapd/back-ldbm/ldbm_delete.c index d1ad3ef..a4b8d8e 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_delete.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_delete.c @@ -1228,5 +1228,6 @@ diskfull_return: { slapi_log_error (SLAPI_LOG_TRACE, "ldbm_back_delete", "leave conn=%" NSPRIu64 " op=%d\n", pb->pb_conn->c_connid, operation->o_opid); } + return rc; } diff --git a/ldap/servers/slapd/back-ldbm/ldbm_search.c b/ldap/servers/slapd/back-ldbm/ldbm_search.c index 552b65b..a097307 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_search.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_search.c @@ -1343,6 +1343,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) int pr_idx = -1; Slapi_Connection *conn; Slapi_Operation *op; + int reverse_list = 0; slapi_pblock_get( pb, SLAPI_SEARCH_TARGET_SDN, &basesdn ); if (NULL == basesdn) { @@ -1370,6 +1371,18 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) slapi_pblock_get( pb, SLAPI_CONNECTION, &conn ); slapi_pblock_get( pb, SLAPI_OPERATION, &op ); + if((reverse_list = operation_is_flag_set(op, OP_FLAG_REVERSE_CANDIDATE_ORDER))){ + /* + * Start at the end of the list and work our way forward. Since a single + * search can enter this function multiple times, we need to keep track + * of our state, and only initialize sr_current once. + */ + if(!op->o_reverse_search_state){ + sr->sr_current = sr->sr_candidates->b_nids; + op->o_reverse_search_state = REV_STARTED; + } + } + if ( !txn.back_txn_txn ) { dblayer_txn_init( li, &txn ); slapi_pblock_set( pb, SLAPI_TXN, txn.back_txn_txn ); @@ -1469,8 +1482,32 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) goto bail; } - /* get the entry */ - id = idl_iterator_dereference_increment(&(sr->sr_current), sr->sr_candidates); + /* + * Get the entry ID + */ + if(reverse_list){ + /* + * This is probably a tombstone reaping, we need to process in the candidate + * list in reserve order, or else we can orphan tombstone entries by removing + * it's parent tombstone entry first. + */ + id = idl_iterator_dereference_decrement(&(sr->sr_current), sr->sr_candidates); + if((sr->sr_current == 0) && op->o_reverse_search_state != LAST_REV_ENTRY){ + /* + * We hit the last entry and we need to process it, but the decrement + * function will keep returning the last entry. So we need to mark that + * we have hit the last entry so we know to stop on the next pass. + */ + op->o_reverse_search_state = LAST_REV_ENTRY; + } else if(op->o_reverse_search_state == LAST_REV_ENTRY){ + /* we're done */ + id = NOID; + } + } else { + /* Process the candidate list in the normal order. */ + id = idl_iterator_dereference_increment(&(sr->sr_current), sr->sr_candidates); + } + if ( id == NOID ) { /* No more entries */ @@ -1481,6 +1518,7 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) } slapi_pblock_set( pb, SLAPI_SEARCH_RESULT_ENTRY, NULL ); delete_search_result_set(pb, &sr); + op->o_reverse_search_state = 0; rc = 0; goto bail; } @@ -1686,7 +1724,12 @@ ldbm_back_next_search_entry_ext( Slapi_PBlock *pb, int use_extension ) } } } + bail: + if(rc){ + op->o_reverse_search_state = 0; + } + return rc; } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index d7b88ed..ee30c45 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -266,6 +266,7 @@ idl_iterator idl_iterator_increment(idl_iterator *i); idl_iterator idl_iterator_decrement(idl_iterator *i); ID idl_iterator_dereference(idl_iterator i, const IDList *idl); ID idl_iterator_dereference_increment(idl_iterator *i, const IDList *idl); +ID idl_iterator_dereference_decrement(idl_iterator *i, const IDList *idl); size_t idl_sizeof(IDList *idl); int idl_store_block(backend *be,DB *db,DBT *key,IDList *idl,DB_TXN *txn,struct attrinfo *a); void idl_set_tune(int val); diff --git a/ldap/servers/slapd/operation.c b/ldap/servers/slapd/operation.c index 01ebfef..3a58381 100644 --- a/ldap/servers/slapd/operation.c +++ b/ldap/servers/slapd/operation.c @@ -200,6 +200,7 @@ operation_new(int flags) o->o_connid = 0; o->o_next = NULL; o->o_flags= flags; + o->o_reverse_search_state = 0; if ( config_get_accesslog_level() & LDAP_DEBUG_TIMING ) { o->o_interval = PR_IntervalNow(); } else { diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index 2465500..047c945 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -1348,6 +1348,7 @@ typedef struct op { struct slapi_operation_parameters o_params; struct slapi_operation_results o_results; int o_pagedresults_sizelimit; + int o_reverse_search_state; } Operation; /* diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 304c5a8..80500a5 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -196,10 +196,10 @@ NSPR_API(PRUint32) PR_fprintf(struct PRFileDesc* fd, const char *fmt, ...) Used for DN. */ /* operation flags */ -#define SLAPI_OP_FLAG_INTERNAL 0x00020 /* An operation generated by the core server or a plugin. */ -#define SLAPI_OP_FLAG_NEVER_CHAIN 0x00800 /* 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_INTERNAL 0x000020 /* An operation generated by the core server or a plugin. */ +#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_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 8c5541a..b9131a5 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -395,43 +395,47 @@ slapi_filter_to_string_internal( const struct slapi_filter *f, char *buf, size_t /* operation.c */ -#define OP_FLAG_PS 0x00001 -#define OP_FLAG_PS_CHANGESONLY 0x00002 -#define OP_FLAG_GET_EFFECTIVE_RIGHTS 0x00004 -#define OP_FLAG_REPLICATED 0x00008 /* A Replicated Operation */ -#define OP_FLAG_REPL_FIXUP 0x00010 /* A Fixup Operation, +#define OP_FLAG_PS 0x000001 +#define OP_FLAG_PS_CHANGESONLY 0x000002 +#define OP_FLAG_GET_EFFECTIVE_RIGHTS 0x000004 +#define OP_FLAG_REPLICATED 0x000008 /* A Replicated Operation */ +#define OP_FLAG_REPL_FIXUP 0x000010 /* A Fixup Operation, * generated as a consequence * of a Replicated Operation. */ -#define OP_FLAG_INTERNAL SLAPI_OP_FLAG_INTERNAL /* 0x00020 */ -#define OP_FLAG_ACTION_LOG_ACCESS 0x00040 -#define OP_FLAG_ACTION_LOG_AUDIT 0x00080 -#define OP_FLAG_ACTION_SCHEMA_CHECK 0x00100 -#define OP_FLAG_ACTION_LOG_CHANGES 0x00200 -#define OP_FLAG_ACTION_INVOKE_FOR_REPLOP 0x00400 -#define OP_FLAG_NEVER_CHAIN SLAPI_OP_FLAG_NEVER_CHAIN /* 0x0800 */ -#define OP_FLAG_TOMBSTONE_ENTRY 0x01000 -#define OP_FLAG_RESURECT_ENTRY 0x02000 -#define OP_FLAG_LEGACY_REPLICATION_DN 0x04000 /* Operation done by legacy +#define OP_FLAG_INTERNAL SLAPI_OP_FLAG_INTERNAL /* 0x000020 */ +#define OP_FLAG_ACTION_LOG_ACCESS 0x000040 +#define OP_FLAG_ACTION_LOG_AUDIT 0x000080 +#define OP_FLAG_ACTION_SCHEMA_CHECK 0x000100 +#define OP_FLAG_ACTION_LOG_CHANGES 0x000200 +#define OP_FLAG_ACTION_INVOKE_FOR_REPLOP 0x000400 +#define OP_FLAG_NEVER_CHAIN SLAPI_OP_FLAG_NEVER_CHAIN /* 0x000800 */ +#define OP_FLAG_TOMBSTONE_ENTRY 0x001000 +#define OP_FLAG_RESURECT_ENTRY 0x002000 +#define OP_FLAG_LEGACY_REPLICATION_DN 0x004000 /* Operation done by legacy * replication DN */ -#define OP_FLAG_ACTION_NOLOG 0x08000 /* Do not log the entry in +#define OP_FLAG_ACTION_NOLOG 0x008000 /* Do not log the entry in * audit log or change log */ -#define OP_FLAG_SKIP_MODIFIED_ATTRS 0x10000 /* Do not update the +#define OP_FLAG_SKIP_MODIFIED_ATTRS 0x010000 /* Do not update the * modifiersname, * modifiedtimestamp, etc. * attributes */ -#define OP_FLAG_REPL_RUV 0x20000 /* Flag to tell to the backend +#define OP_FLAG_REPL_RUV 0x020000 /* Flag to tell to the backend * that the entry to be added/ * modified is RUV. This info * is used to skip VLV op. * (see #329951) */ -#define OP_FLAG_PAGED_RESULTS 0x40000 /* simple paged results */ -#define OP_FLAG_SERVER_SIDE_SORTING 0x80000 /* server side sorting */ +#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 */ +/* reverse search states */ +#define REV_STARTED 1 +#define LAST_REV_ENTRY 2 CSN *operation_get_csn(Slapi_Operation *op); void operation_set_csn(Slapi_Operation *op,CSN *csn); -- 1.8.1.4