389-commits January 2017

389-commits@lists.fedoraproject.org

7 participants
43 discussions

Branch '389-ds-base-1.2.11' - ldap/servers
by Mark Reynolds 13 Jan '17

13 Jan '17

ldap/servers/plugins/memberof/memberof.c | 18 +++++++++++++++++- ldap/servers/slapd/mapping_tree.c | 20 ++++++++++++++++++++ ldap/servers/slapd/slapi-plugin.h | 1 + 3 files changed, 38 insertions(+), 1 deletion(-) New commits: commit a87ddab64870a70b54eab8964ae1cdea9c5689b9 Author: Mark Reynolds <mreynolds(a)redhat.com> Date: Fri Jan 13 11:08:18 2017 -0500 Ticket 49072 - memberof fixup is not validating base dn Description: The basedn validation was not correctly backported to 1.2.11. This patch adds the appropriate checks. https://fedorahosted.org/389/ticket/49072 Reviewed by: nhosoi(Thanks!) diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c index 2cdaabb..aa54922 100644 --- a/ldap/servers/plugins/memberof/memberof.c +++ b/ldap/servers/plugins/memberof/memberof.c @@ -2282,10 +2282,11 @@ void memberof_fixup_task_thread(void *arg) { MemberOfConfig configCopy = {0, 0, 0, 0}; Slapi_Task *task = (Slapi_Task *)arg; + Slapi_DN *sdn; + Slapi_Backend *be; task_data *td = NULL; int rc = 0; - if (!task) { return; /* no task */ } @@ -2302,6 +2303,20 @@ void memberof_fixup_task_thread(void *arg) slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "Memberof task starts (filter: \"%s\") ...\n", td->filter_str); + /* Validate the search base */ + sdn = slapi_sdn_new_dn_byref(td->dn); + be = slapi_be_select_exact(sdn); + slapi_sdn_free(&sdn); + if (be == NULL) { + slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, + "memberof_fixup_task_thread - Failed to get be backend from (%s)\n", + td->dn); + slapi_task_log_notice(task, "Memberof task - Failed to get be backend from (%s)\n", + td->dn); + rc = -1; + goto done; + } + /* We need to get the config lock first. Trying to get the * config lock after we already hold the op lock can cause * a deadlock. */ @@ -2324,6 +2339,7 @@ void memberof_fixup_task_thread(void *arg) memberof_free_config(&configCopy); +done: slapi_task_log_notice(task, "Memberof task finished."); slapi_task_log_status(task, "Memberof task finished."); slapi_task_inc_progress(task); diff --git a/ldap/servers/slapd/mapping_tree.c b/ldap/servers/slapd/mapping_tree.c index 841ed32..f9dfcb9 100644 --- a/ldap/servers/slapd/mapping_tree.c +++ b/ldap/servers/slapd/mapping_tree.c @@ -3039,6 +3039,26 @@ slapi_be_select( const Slapi_DN *sdn ) /* JCM - The name of this should change?? return be; } +Slapi_Backend * +slapi_be_select_exact(const Slapi_DN *sdn) +{ + Slapi_Backend *be = NULL; + mapping_tree_node *node = NULL; + + if (!sdn) { + slapi_log_error(SLAPI_LOG_FATAL, "slapi_be_select_exact", + "Empty Slapi_DN is given.\n"); + return NULL; + } + node = slapi_get_mapping_tree_node_by_dn(sdn); + + if (node && node->mtn_be) { + be = node->mtn_be[0]; + } + + return be; +} + /* Check if the dn targets an internal reserved backends */ int slapi_on_internal_backends(const Slapi_DN *sdn) diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 4ed50f7..e973717 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -6039,6 +6039,7 @@ Slapi_Backend *slapi_be_new( const char *type, const char *name, int isprivate, int logchanges ); void slapi_be_free(Slapi_Backend **be); Slapi_Backend *slapi_be_select( const Slapi_DN *sdn ); +Slapi_Backend *slapi_be_select_exact(const Slapi_DN *sdn); Slapi_Backend *slapi_be_select_by_instance_name( const char *name ); int slapi_be_exist(const Slapi_DN *sdn); void slapi_be_delete_onexit(Slapi_Backend *be);

1 0

Branch '389-ds-base-1.2.11' - ldap/schema ldap/servers
by Ludwig Krispenz 13 Jan '17

13 Jan '17

ldap/schema/01core389.ldif | 3 ldap/servers/plugins/replication/cl5_api.c | 16 +- ldap/servers/plugins/replication/cl5_clcache.c | 43 ++---- ldap/servers/plugins/replication/cl5_clcache.h | 2 ldap/servers/plugins/replication/repl5.h | 4 ldap/servers/plugins/replication/repl5_agmt.c | 115 +++++++++++++++++- ldap/servers/plugins/replication/repl5_agmtlist.c | 13 ++ ldap/servers/plugins/replication/repl5_inc_protocol.c | 7 - ldap/servers/plugins/replication/repl_globals.c | 1 9 files changed, 168 insertions(+), 36 deletions(-) New commits: commit 55aa091b4b7ae812aff1fb7378374c02fdf478b3 Author: Ludwig Krispenz <lkrispen(a)redhat.com> Date: Thu Jan 12 10:20:08 2017 +0100 Ticket 49020 - v1.2.11 do not treat missing csn as fatal This patch removes the automatic choice of an alternative csn when the calculated anchor csn is not found. In that case it does no longer go to fatal state but will retry later. It also adds a configuration parameter to thr replication agreement to allow to pick a "next best" anchorcsn if the original is not found to keep replicatio going. Reviewed by: Noriko, William diff --git a/ldap/schema/01core389.ldif b/ldap/schema/01core389.ldif index f013465..b17372d 100644 --- a/ldap/schema/01core389.ldif +++ b/ldap/schema/01core389.ldif @@ -160,6 +160,7 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2310 NAME 'nsds5ReplicaFlowControlWindow attributeTypes: ( 2.16.840.1.113730.3.1.2311 NAME 'nsds5ReplicaFlowControlPause' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2332 NAME 'allowWeakDHParam' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'Netscape Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2333 NAME 'nsds5ReplicaReleaseTimeout' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) +attributeTypes: ( 2.16.840.1.113730.3.1.2335 NAME 'nsds5ReplicaIgnoreMissingChange' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) # # objectclasses # @@ -171,7 +172,7 @@ objectClasses: ( 2.16.840.1.113730.3.2.110 NAME 'nsMappingTree' DESC 'Netscape d objectClasses: ( 2.16.840.1.113730.3.2.104 NAME 'nsContainer' DESC 'Netscape defined objectclass' SUP top MUST ( CN ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.108 NAME 'nsDS5Replica' DESC 'Netscape defined objectclass' SUP top MUST ( nsDS5ReplicaRoot $ nsDS5ReplicaId ) MAY (cn $ nsds5ReplicaCleanRUV $ nsds5ReplicaAbortCleanRUV $ nsDS5ReplicaType $ nsDS5ReplicaBindDN $ nsState $ nsDS5ReplicaName $ nsDS5Flags $ nsDS5Task $ nsDS5ReplicaReferral $ nsDS5ReplicaAutoReferral $ nsds5ReplicaPurgeDelay $ nsds5ReplicaTombstonePurgeInterval $ nsds5ReplicaChangeCount $ nsds5ReplicaLegacyConsumer $ nsds5ReplicaReleaseTimeout) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.113 NAME 'nsTombstone' DESC 'Netscape defined objectclass' SUP top MAY ( nsParentUniqueId $ nscpEntryDN ) X-ORIGIN 'Netscape Directory Server' ) -objectClasses: ( 2.16.840.1.113730.3.2.103 NAME 'nsDS5ReplicationAgreement' DESC 'Netscape defined objectclass' SUP top MUST ( cn ) MAY ( nsds5ReplicaCleanRUVNotified $ nsDS5ReplicaHost $ nsDS5ReplicaPort $ nsDS5ReplicaTransportInfo $ nsDS5ReplicaBindDN $ nsDS5ReplicaCredentials $ nsDS5ReplicaBindMethod $ nsDS5ReplicaRoot $ nsDS5ReplicatedAttributeList $ nsDS5ReplicatedAttributeListTotal $ nsDS5ReplicaUpdateSchedule $ nsds5BeginReplicaRefresh $ description $ nsds50ruv $ nsruvReplicaLastModified $ nsds5ReplicaTimeout $ nsds5replicaChangesSentSinceStartup $ nsds5replicaLastUpdateEnd $ nsds5replicaLastUpdateStart $ nsds5replicaLastUpdateStatus $ nsds5replicaUpdateInProgress $ nsds5replicaLastInitEnd $ nsds5ReplicaEnabled $ nsds5replicaLastInitStart $ nsds5replicaLastInitStatus $ nsds5debugreplicatimeout $ nsds5replicaBusyWaitTime $ nsds5ReplicaStripAttrs $ nsds5replicaSessionPauseTime $ nsds5ReplicaFlowControlWindow $ nsds5ReplicaFlowControlPause ) X-ORIGIN 'Netscape Directory Server' ) +objectClasses: ( 2.16.840.1.113730.3.2.103 NAME 'nsDS5ReplicationAgreement' DESC 'Netscape defined objectclass' SUP top MUST ( cn ) MAY ( nsds5ReplicaCleanRUVNotified $ nsDS5ReplicaHost $ nsDS5ReplicaPort $ nsDS5ReplicaTransportInfo $ nsDS5ReplicaBindDN $ nsDS5ReplicaCredentials $ nsDS5ReplicaBindMethod $ nsDS5ReplicaRoot $ nsDS5ReplicatedAttributeList $ nsDS5ReplicatedAttributeListTotal $ nsDS5ReplicaUpdateSchedule $ nsds5BeginReplicaRefresh $ description $ nsds50ruv $ nsruvReplicaLastModified $ nsds5ReplicaTimeout $ nsds5replicaChangesSentSinceStartup $ nsds5replicaLastUpdateEnd $ nsds5replicaLastUpdateStart $ nsds5replicaLastUpdateStatus $ nsds5replicaUpdateInProgress $ nsds5replicaLastInitEnd $ nsds5ReplicaEnabled $ nsds5replicaLastInitStart $ nsds5replicaLastInitStatus $ nsds5debugreplicatimeout $ nsds5replicaBusyWaitTime $ nsds5ReplicaStripAttrs $ nsds5replicaSessionPauseTime $ nsds5ReplicaFlowControlWindow $ nsds5ReplicaFlowControlPause $ nsds5ReplicaIgnoreMissingCha nge ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.39 NAME 'nsslapdConfig' DESC 'Netscape defined objectclass' SUP top MAY ( cn ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.317 NAME 'nsSaslMapping' DESC 'Netscape defined objectclass' SUP top MUST ( cn $ nsSaslMapRegexString $ nsSaslMapBaseDNTemplate $ nsSaslMapFilterTemplate ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.43 NAME 'nsSNMP' DESC 'Netscape defined objectclass' SUP top MUST ( cn $ nsSNMPEnabled ) MAY ( nsSNMPOrganization $ nsSNMPLocation $ nsSNMPContact $ nsSNMPDescription $ nsSNMPName $ nsSNMPMasterHost $ nsSNMPMasterPort ) X-ORIGIN 'Netscape Directory Server' ) diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c index 9b2ef37..a97f626 100644 --- a/ldap/servers/plugins/replication/cl5_api.c +++ b/ldap/servers/plugins/replication/cl5_api.c @@ -337,7 +337,7 @@ static int _cl5WriteBervals (struct berval **bv, char** buff, unsigned int *size static PRBool _cl5ValidReplayIterator (const CL5ReplayIterator *iterator); #endif static int _cl5PositionCursorForReplay (ReplicaId consumerRID, const RUV *consumerRuv, - Object *replica, Object *fileObject, CL5ReplayIterator **iterator); + Object *replica, Object *fileObject, CL5ReplayIterator **iterator, int *continue_on_missing); static int _cl5CheckMissingCSN (const CSN *minCsn, const RUV *supplierRUV, CL5DBFile *file); /* changelog trimming */ @@ -1544,7 +1544,7 @@ int cl5CreateReplayIteratorEx (Private_Repl_Protocol *prp, const RUV *consumerRu /* iterate through the ruv in csn order to find first master for which we can replay changes */ - rc = _cl5PositionCursorForReplay (consumerRID, consumerRuv, replica, obj, iterator); + rc = _cl5PositionCursorForReplay (consumerRID, consumerRuv, replica, obj, iterator, NULL); } else { @@ -1605,7 +1605,13 @@ int cl5CreateReplayIterator (Private_Repl_Protocol *prp, const RUV *consumerRuv, /* iterate through the ruv in csn order to find first master for which we can replay changes */ ReplicaId consumerRID = agmt_get_consumer_rid ( prp->agmt, prp->conn ); - rc = _cl5PositionCursorForReplay (consumerRID, consumerRuv, replica, obj, iterator); + int continue_on_missing = agmt_get_ignoremissing ( prp->agmt); + int save_cont_miss = continue_on_missing; + rc = _cl5PositionCursorForReplay (consumerRID, consumerRuv, replica, obj, iterator, &continue_on_missing); + if (save_cont_miss == 1 && continue_on_missing ==0) { + /* the option to continue once on a missing csn was used, rest */ + agmt_set_ignoremissing ( prp->agmt, 0); + } } else { @@ -5445,7 +5451,7 @@ struct replica_hash_entry static int _cl5PositionCursorForReplay (ReplicaId consumerRID, const RUV *consumerRuv, - Object *replica, Object *fileObj, CL5ReplayIterator **iterator) + Object *replica, Object *fileObj, CL5ReplayIterator **iterator, int *continue_on_missing) { CLC_Buffer *clcache = NULL; CL5DBFile *file; @@ -5489,7 +5495,7 @@ static int _cl5PositionCursorForReplay (ReplicaId consumerRID, const RUV *consum rc = clcache_get_buffer ( &clcache, file->db, consumerRID, consumerRuv, supplierRuv ); if ( rc != 0 ) goto done; - rc = clcache_load_buffer (clcache, &startCSN); + rc = clcache_load_buffer (clcache, &startCSN, continue_on_missing); if (rc == 0) { haveChanges = PR_TRUE; diff --git a/ldap/servers/plugins/replication/cl5_clcache.c b/ldap/servers/plugins/replication/cl5_clcache.c index f6c56a9..44b2817 100644 --- a/ldap/servers/plugins/replication/cl5_clcache.c +++ b/ldap/servers/plugins/replication/cl5_clcache.c @@ -354,7 +354,7 @@ clcache_return_buffer ( CLC_Buffer **buf ) * historic reason. */ int -clcache_load_buffer ( CLC_Buffer *buf, CSN **anchorCSN ) +clcache_load_buffer ( CLC_Buffer *buf, CSN **anchorCSN, int *continue_on_miss ) { int rc = 0; int flag = DB_NEXT; @@ -375,6 +375,22 @@ clcache_load_buffer ( CLC_Buffer *buf, CSN **anchorCSN ) if (anchorCSN) *anchorCSN = buf->buf_current_csn; rc = clcache_load_buffer_bulk ( buf, flag ); + if (rc == DB_NOTFOUND && continue_on_miss && *continue_on_miss) { + /* make replication going using next best startcsn */ + slapi_log_error(SLAPI_LOG_FATAL, buf->buf_agmt_name, + "clcache_load_buffer - Can't load changelog buffer starting at CSN %s with flag(%s). " + "Trying to use an alterantive start CSN.\n", + (char*)buf->buf_key.data, + flag==DB_NEXT?"DB_NEXT":"DB_SET" ); + rc = clcache_load_buffer_bulk ( buf, DB_SET_RANGE ); + if (rc == 0) { + slapi_log_error(SLAPI_LOG_FATAL, buf->buf_agmt_name, + "clcache_load_buffer - Using alternative start iteration csn: %s \n", + (char*)buf->buf_key.data); + } + /* the use of alternative start csns can be limited, record its usage */ + (*continue_on_miss)--; + } /* Reset some flag variables */ if ( rc == 0 ) { int i; @@ -437,23 +453,6 @@ retry: & buf->buf_key, & buf->buf_data, DB_SET ); - if (rc == DB_NOTFOUND) { - /* the start position in the changelog is not found - * 1. log an error - * 2. try to find another starting position as close - * as possible - */ - slapi_log_error ( SLAPI_LOG_FATAL, "clcache_load_buffer_bulk", - "changelog record with csn (%s) not found for DB_NEXT\n", - (char *)buf->buf_key.data ); - rc = cursor->c_get ( cursor, & buf->buf_key, & buf->buf_data, - DB_SET_RANGE ); - /* this moves the cursor ahead of the tageted csn, - * so we achieved what was intended with DB_SET/DB_NEXT - * continute at this csn. - */ - use_flag = DB_CURRENT; - } } /* @@ -462,12 +461,6 @@ retry: */ if ( 0 == rc || DB_BUFFER_SMALL == rc ) { rc = clcache_cursor_get ( cursor, buf, use_flag ); - if ( rc == DB_NOTFOUND && use_flag == DB_SET) { - slapi_log_error ( SLAPI_LOG_FATAL, "clcache_load_buffer_bulk", - "changelog record with csn (%s) not found for DB_SET\n", - (char *)buf->buf_key.data ); - rc = clcache_cursor_get ( cursor, buf, DB_SET_RANGE ); - } } } @@ -541,7 +534,7 @@ clcache_get_next_change ( CLC_Buffer *buf, void **key, size_t *keylen, void **da * We're done with the current buffer. Now load the next chunk. */ if ( NULL == *key && CLC_STATE_READY == buf->buf_state ) { - rc = clcache_load_buffer ( buf, NULL ); + rc = clcache_load_buffer ( buf, NULL, NULL ); if ( 0 == rc && buf->buf_record_ptr ) { DB_MULTIPLE_KEY_NEXT ( buf->buf_record_ptr, &buf->buf_data, *key, *keylen, *data, *datalen ); diff --git a/ldap/servers/plugins/replication/cl5_clcache.h b/ldap/servers/plugins/replication/cl5_clcache.h index 5cab3cb..3ce210b 100644 --- a/ldap/servers/plugins/replication/cl5_clcache.h +++ b/ldap/servers/plugins/replication/cl5_clcache.h @@ -52,7 +52,7 @@ typedef struct clc_buffer CLC_Buffer; int clcache_init ( DB_ENV **dbenv ); void clcache_set_config (); int clcache_get_buffer ( CLC_Buffer **buf, DB *db, ReplicaId consumer_rid, const RUV *consumer_ruv, const RUV *local_ruv ); -int clcache_load_buffer ( CLC_Buffer *buf, CSN **anchorCSN ); +int clcache_load_buffer ( CLC_Buffer *buf, CSN **anchorCSN, int *continue_on_miss ); void clcache_return_buffer ( CLC_Buffer **buf ); int clcache_get_next_change ( CLC_Buffer *buf, void **key, size_t *keylen, void **data, size_t *datalen, CSN **csn ); void clcache_destroy (); diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h index 0db632e..80ef757 100644 --- a/ldap/servers/plugins/replication/repl5.h +++ b/ldap/servers/plugins/replication/repl5.h @@ -175,6 +175,7 @@ extern const char *type_nsds5ReplicaCleanRUVnotified; extern const char *type_nsds5ReplicaFlowControlWindow; extern const char *type_nsds5ReplicaFlowControlPause; extern const char *type_replicaReleaseTimeout; +extern const char *type_replicaIgnoreMissingChange; /* Attribute names for windows replication agreements */ extern const char *type_nsds7WindowsReplicaArea; @@ -321,6 +322,7 @@ long agmt_get_busywaittime(const Repl_Agmt *ra); long agmt_get_pausetime(const Repl_Agmt *ra); long agmt_get_flowcontrolwindow(const Repl_Agmt *ra); long agmt_get_flowcontrolpause(const Repl_Agmt *ra); +long agmt_get_ignoremissing(const Repl_Agmt *ra); int agmt_start(Repl_Agmt *ra); int windows_agmt_start(Repl_Agmt *ra); int agmt_stop(Repl_Agmt *ra); @@ -343,6 +345,7 @@ int agmt_set_schedule_from_entry( Repl_Agmt *ra, const Slapi_Entry *e ); int agmt_set_timeout_from_entry( Repl_Agmt *ra, const Slapi_Entry *e ); int agmt_set_flowcontrolwindow_from_entry(Repl_Agmt *ra, const Slapi_Entry *e); int agmt_set_flowcontrolpause_from_entry(Repl_Agmt *ra, const Slapi_Entry *e); +int agmt_set_ignoremissing_from_entry(Repl_Agmt *ra, const Slapi_Entry *e); int agmt_set_busywaittime_from_entry( Repl_Agmt *ra, const Slapi_Entry *e ); int agmt_set_pausetime_from_entry( Repl_Agmt *ra, const Slapi_Entry *e ); int agmt_set_credentials_from_entry( Repl_Agmt *ra, const Slapi_Entry *e ); @@ -388,6 +391,7 @@ int agmt_set_enabled_from_entry(Repl_Agmt *ra, Slapi_Entry *e, char *returntext) char **agmt_get_attrs_to_strip(Repl_Agmt *ra); int agmt_set_attrs_to_strip(Repl_Agmt *ra, Slapi_Entry *e); int agmt_set_timeout(Repl_Agmt *ra, long timeout); +int agmt_set_ignoremissing(Repl_Agmt *ra, long ignoremissing); void agmt_update_done(Repl_Agmt *ra, int is_total); typedef struct replica Replica; diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c index 41a81ca..4c67e57 100644 --- a/ldap/servers/plugins/replication/repl5_agmt.c +++ b/ldap/servers/plugins/replication/repl5_agmt.c @@ -150,6 +150,7 @@ typedef struct repl5agmt { long flowControlPause; /* When nb of not acknowledged entries overpass totalUpdateWindow * This is the duration (in msec) that the RA will pause before sending the next entry */ + long ignoreMissingChange; /* if set replication will try to continue even if change cannot be found in changelog */ } repl5agmt; /* Forward declarations */ @@ -159,7 +160,7 @@ static int get_agmt_status(Slapi_PBlock *pb, Slapi_Entry* e, Slapi_Entry* entryAfter, int *returncode, char *returntext, void *arg); static int agmt_set_bind_method_no_lock(Repl_Agmt *ra, const Slapi_Entry *e); static int agmt_set_transportinfo_no_lock(Repl_Agmt *ra, const Slapi_Entry *e); - +static void agmt_replica_reset_ignoremissing (const Repl_Agmt *agmt); /* Schema for replication agreement: @@ -362,6 +363,21 @@ agmt_new_from_entry(Slapi_Entry *e) } } + /* continue on missing change ? */ + ra->ignoreMissingChange = 0; + tmpstr = slapi_entry_attr_get_charptr(e, type_replicaIgnoreMissingChange); + if (NULL != tmpstr) + { + if (strcasecmp(tmpstr,"off") == 0 || strcasecmp(tmpstr,"never") == 0) { + ra->ignoreMissingChange = 0; + } else if (strcasecmp(tmpstr,"on") == 0 || strcasecmp(tmpstr,"once") == 0) { + ra->ignoreMissingChange = 1; + } else if (strcasecmp(tmpstr,"always") == 0) { + ra->ignoreMissingChange = -1; + } + slapi_ch_free_string(&tmpstr); + } + /* DN of entry at root of replicated area */ tmpstr = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaRoot); if (NULL != tmpstr) @@ -1012,6 +1028,16 @@ agmt_get_flowcontrolpause(const Repl_Agmt *ra) PR_Unlock(ra->lock); return return_value; } +long +agmt_get_ignoremissing(const Repl_Agmt *ra) +{ + long return_value; + PR_ASSERT(NULL != ra); + PR_Lock(ra->lock); + return_value = ra->ignoreMissingChange; + PR_Unlock(ra->lock); + return return_value; +} /* * Warning - reference to the long name of the agreement is returned. * The long name of an agreement is the DN of the agreement entry, @@ -1826,6 +1852,48 @@ agmt_set_flowcontrolpause_from_entry(Repl_Agmt *ra, const Slapi_Entry *e) } return return_value; } +/* add comment here */ +int +agmt_set_ignoremissing_from_entry(Repl_Agmt *ra, const Slapi_Entry *e) +{ + Slapi_Attr *sattr = NULL; + int return_value = -1; + + PR_ASSERT(NULL != ra); + PR_Lock(ra->lock); + if (ra->stop_in_progress) + { + PR_Unlock(ra->lock); + return return_value; + } + + slapi_entry_attr_find(e, type_replicaIgnoreMissingChange, &sattr); + if (NULL != sattr) + { + Slapi_Value *sval = NULL; + slapi_attr_first_value(sattr, &sval); + if (NULL != sval) + { + const char *tmpval = slapi_value_get_string(sval); + if (strcasecmp(tmpval,"off") == 0 || strcasecmp(tmpval,"never") == 0) { + ra->ignoreMissingChange = 0; + return_value = 0; + } else if (strcasecmp(tmpval,"on") == 0 || strcasecmp(tmpval,"once") == 0) { + ra->ignoreMissingChange = 1; + return_value = 0; + } else if (strcasecmp(tmpval,"always") == 0) { + ra->ignoreMissingChange = -1; + return_value = 0; + } + } + } + PR_Unlock(ra->lock); + if (return_value == 0) + { + prot_notify_agmt_changed(ra->protocol, ra->long_name); + } + return return_value; +} int agmt_set_timeout(Repl_Agmt *ra, long timeout) @@ -1867,6 +1935,20 @@ agmt_set_flowcontrolpause(Repl_Agmt *ra, long pause) return 0; } +int +agmt_set_ignoremissing(Repl_Agmt *ra, long ignoremissing) +{ + PR_Lock(ra->lock); + if (ra->stop_in_progress){ + PR_Unlock(ra->lock); + return -1; + } + ra->ignoreMissingChange = ignoremissing; + PR_Unlock(ra->lock); + /* if reset to 0 update the entry */ + agmt_replica_reset_ignoremissing(ra); + return 0; +} /* * Set or reset the busywaittime @@ -2101,6 +2183,37 @@ agmt_replica_init_done (const Repl_Agmt *agmt) slapi_pblock_destroy (pb); } + +/* delete nsds5replicaIgnoreMissingChange attribute */ +static void +agmt_replica_reset_ignoremissing (const Repl_Agmt *agmt) +{ + int rc; + Slapi_PBlock *pb = slapi_pblock_new (); + LDAPMod *mods [2]; + LDAPMod mod; + + mods[0] = &mod; + mods[1] = NULL; + mod.mod_op = LDAP_MOD_DELETE | LDAP_MOD_BVALUES; + mod.mod_type = (char*)type_replicaIgnoreMissingChange; + mod.mod_bvalues = NULL; + + slapi_modify_internal_set_pb_ext(pb, agmt->dn, mods, NULL/* controls */, + NULL/* uniqueid */, repl_get_plugin_identity (PLUGIN_MULTIMASTER_REPLICATION), 0/* flags */); + slapi_modify_internal_pb (pb); + + slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &rc); + if (rc != LDAP_SUCCESS && rc != LDAP_NO_SUCH_ATTRIBUTE) + { + slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "agmt_replica_ignoremissing: " + "failed to remove (%s) attribute from (%s) entry; LDAP error - %d\n", + type_replicaIgnoreMissingChange, slapi_sdn_get_ndn (agmt->dn), rc); + } + + slapi_pblock_destroy (pb); +} + /* Agreement object is acquired on behalf of the caller. The caller is responsible for releasing the object when it is no longer used */ diff --git a/ldap/servers/plugins/replication/repl5_agmtlist.c b/ldap/servers/plugins/replication/repl5_agmtlist.c index caa41af..7789c3f 100644 --- a/ldap/servers/plugins/replication/repl5_agmtlist.c +++ b/ldap/servers/plugins/replication/repl5_agmtlist.c @@ -371,6 +371,19 @@ agmtlist_modify_callback(Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entry } } else if (slapi_attr_types_equivalent(mods[i]->mod_type, + type_replicaIgnoreMissingChange)) + { + /* New replica timeout */ + if (agmt_set_ignoremissing_from_entry(agmt, e) != 0) + { + slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "agmtlist_modify_callback - " + "Failed to update the ignorMissingChange attribute for agreement %s\n", + agmt_get_long_name(agmt)); + *returncode = LDAP_OPERATIONS_ERROR; + rc = SLAPI_DSE_CALLBACK_ERROR; + } + } + else if (slapi_attr_types_equivalent(mods[i]->mod_type, type_nsds5ReplicaBusyWaitTime)) { /* New replica busywaittime */ diff --git a/ldap/servers/plugins/replication/repl5_inc_protocol.c b/ldap/servers/plugins/replication/repl5_inc_protocol.c index e6920e8..8cf72a1 100644 --- a/ldap/servers/plugins/replication/repl5_inc_protocol.c +++ b/ldap/servers/plugins/replication/repl5_inc_protocol.c @@ -1709,13 +1709,14 @@ send_updates(Private_Repl_Protocol *prp, RUV *remote_update_vector, PRUint32 *nu "%s: Data required to update replica has been purged. " "The replica must be reinitialized.\n", agmt_get_long_name(prp->agmt)); - return_value = UPDATE_FATAL_ERROR; + return_value = UPDATE_TRANSIENT_ERROR; break; case CL5_MISSING_DATA: /* data should be in the changelog, but is missing */ slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, - "%s: Missing data encountered\n", + "send_updates - %s: Missing data encountered. " + "If the error persists the replica must be reinitialized.\n", agmt_get_long_name(prp->agmt)); - return_value = UPDATE_FATAL_ERROR; + return_value = UPDATE_TRANSIENT_ERROR; break; case CL5_UNKNOWN_ERROR: /* unclassified error */ slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, diff --git a/ldap/servers/plugins/replication/repl_globals.c b/ldap/servers/plugins/replication/repl_globals.c index 1d35241..4c9f274 100644 --- a/ldap/servers/plugins/replication/repl_globals.c +++ b/ldap/servers/plugins/replication/repl_globals.c @@ -135,6 +135,7 @@ const char *type_nsds5ReplicaCleanRUVnotified = "nsds5ReplicaCleanRUVNotified"; const char* type_nsds5ReplicaFlowControlWindow = "nsds5ReplicaFlowControlWindow"; const char* type_nsds5ReplicaFlowControlPause = "nsds5ReplicaFlowControlPause"; +const char* type_replicaIgnoreMissingChange = "nsds5ReplicaIgnoreMissingChange"; /* windows sync specific attributes */ const char *type_nsds7WindowsReplicaArea = "nsds7WindowsReplicaSubtree";

1 0

2 commits - ldap/servers Makefile.am
by William Brown 12 Jan '17

12 Jan '17

Makefile.am | 4 ldap/servers/slapd/back-ldbm/back-ldbm.h | 7 ldap/servers/slapd/back-ldbm/cache.c | 13 ldap/servers/slapd/back-ldbm/ldbm_config.c | 21 - ldap/servers/slapd/back-ldbm/start.c | 440 ++++++++++++++++------------- 5 files changed, 285 insertions(+), 200 deletions(-) New commits: commit 28e5e150c084db4ab1d1e1b944dfe1fe44270ce9 Author: William Brown <firstyear(a)redhat.com> Date: Mon Jan 9 12:59:22 2017 +1000 Ticket 49028 - Autosize database cache by default. Bug Description: We ship Directory Server with very conservative defaults. These are often not fit for production, yet many people run with them and complain about the performance of Directory Server. Fix Description: Instead of relying on the admin to tune their system, by defaulting to the autotuning newly installed systems will get the "best possible" experience. Please see: http://www.port389.org/docs/389ds/design/autotuning.html Additionally, we now clamp autotuned caches to 64mb boundaries, and default to cachesizes of 0. If we detect 0, we run the autotune, if else we leave it alone. This way existing installs won't see a change, but new deployments will autoscale to the hardware. Existing users of autosizing who want the value to change every startup will still have the same behaviour. https://fedorahosted.org/389/ticket/49028 Author: wibrown Review by: nhosoi (Thanks!) diff --git a/ldap/servers/slapd/back-ldbm/back-ldbm.h b/ldap/servers/slapd/back-ldbm/back-ldbm.h index ecef956..a5fc540 100644 --- a/ldap/servers/slapd/back-ldbm/back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/back-ldbm.h @@ -160,8 +160,11 @@ typedef unsigned short u_int16_t; #define LDBM_VERSION_31 "Netscape-ldbm/3.1" #define LDBM_FILENAME_SUFFIX LDBM_SUFFIX #define DBVERSION_FILENAME "DBVERSION" -#define DEFAULT_CACHE_SIZE (size_t)33554432 -#define DEFAULT_CACHE_SIZE_STR "33554432" +/* 0 here means to let the autotuning reset the value on first run */ +/* cache can't get any smaller than this (in bytes) */ +#define MINCACHESIZE (size_t)512000 +#define DEFAULT_CACHE_SIZE (size_t)0 +#define DEFAULT_CACHE_SIZE_STR "0" #define DEFAULT_CACHE_ENTRIES -1 /* no limit */ #define DEFAULT_DNCACHE_SIZE (size_t)16777216 #define DEFAULT_DNCACHE_SIZE_STR "16777216" diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c index 3a86d86..ade2240 100644 --- a/ldap/servers/slapd/back-ldbm/cache.c +++ b/ldap/servers/slapd/back-ldbm/cache.c @@ -21,8 +21,6 @@ /* #define CACHE_DEBUG * causes slowdown */ #endif -/* cache can't get any smaller than this (in bytes) */ -#define MINCACHESIZE (size_t)512000 /* don't let hash be smaller than this # of slots */ #define MINHASHSIZE 1024 @@ -657,10 +655,13 @@ static void entrycache_set_max_size(struct cache *cache, size_t bytes) struct backentry *eflushtemp = NULL; if (bytes < MINCACHESIZE) { - bytes = MINCACHESIZE; - slapi_log_err(SLAPI_LOG_WARNING, - "entrycache_set_max_size", "Minimum cache size is %lu -- rounding up\n", - MINCACHESIZE); + /* During startup, this value can be 0 to indicate an autotune is about + * to happen. In that case, suppress this warning. + */ + if (bytes > 0) { + slapi_log_err(SLAPI_LOG_WARNING, "entrycache_set_max_size", "Minimum cache size is %lu -- rounding up\n", MINCACHESIZE); + } + bytes = MINCACHESIZE; } cache_lock(cache); cache->c_maxsize = bytes; diff --git a/ldap/servers/slapd/back-ldbm/ldbm_config.c b/ldap/servers/slapd/back-ldbm/ldbm_config.c index bf1b9c7..8541224 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_config.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_config.c @@ -420,7 +420,11 @@ static int ldbm_config_dbcachesize_set(void *arg, void *value, char *errorbuf, i /* Stop the user configuring a stupidly small cache */ /* min: 8KB (page size) * def thrd cnts (threadnumber==20). */ #define DBDEFMINSIZ 500000 - if (val < DBDEFMINSIZ) { + /* We allow a value of 0, because the autotuting in start.c will + * register that, and trigger the recalculation of the dbcachesize as + * needed on the next start up. + */ + if (val < DBDEFMINSIZ && val > 0) { slapi_log_err(SLAPI_LOG_NOTICE,"ldbm_config_dbcachesize_set", "cache too small, increasing to %dK bytes\n", DBDEFMINSIZ/1000); val = DBDEFMINSIZ; @@ -435,14 +439,17 @@ static int ldbm_config_dbcachesize_set(void *arg, void *value, char *errorbuf, i } if (CONFIG_PHASE_RUNNING == phase) { li->li_new_dbcachesize = val; - slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_config_dbcachesize_set", - "New db cache size will not take affect until the server is restarted\n"); + if (val == 0) { + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_config_dbcachesize_set", "cache size reset to 0, will be autosized on next startup.\n"); + } else { + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_config_dbcachesize_set", "New db cache size will not take affect until the server is restarted\n"); + } } else { li->li_new_dbcachesize = val; li->li_dbcachesize = val; } } - + return retval; } @@ -1501,7 +1508,7 @@ static config_info ldbm_config[] = { {CONFIG_MODE, CONFIG_TYPE_INT_OCTAL, "0600", &ldbm_config_mode_get, &ldbm_config_mode_set, CONFIG_FLAG_ALWAYS_SHOW|CONFIG_FLAG_ALLOW_RUNNING_CHANGE}, {CONFIG_IDLISTSCANLIMIT, CONFIG_TYPE_INT, "4000", &ldbm_config_allidsthreshold_get, &ldbm_config_allidsthreshold_set, CONFIG_FLAG_ALWAYS_SHOW|CONFIG_FLAG_ALLOW_RUNNING_CHANGE}, {CONFIG_DIRECTORY, CONFIG_TYPE_STRING, "", &ldbm_config_directory_get, &ldbm_config_directory_set, CONFIG_FLAG_ALWAYS_SHOW|CONFIG_FLAG_ALLOW_RUNNING_CHANGE|CONFIG_FLAG_SKIP_DEFAULT_SETTING}, - {CONFIG_DBCACHESIZE, CONFIG_TYPE_SIZE_T, DEFAULT_DBCACHE_SIZE_STR, &ldbm_config_dbcachesize_get, &ldbm_config_dbcachesize_set, CONFIG_FLAG_ALWAYS_SHOW|CONFIG_FLAG_ALLOW_RUNNING_CHANGE}, + {CONFIG_DBCACHESIZE, CONFIG_TYPE_SIZE_T, DEFAULT_CACHE_SIZE_STR, &ldbm_config_dbcachesize_get, &ldbm_config_dbcachesize_set, CONFIG_FLAG_ALWAYS_SHOW|CONFIG_FLAG_ALLOW_RUNNING_CHANGE}, {CONFIG_DBNCACHE, CONFIG_TYPE_INT, "0", &ldbm_config_dbncache_get, &ldbm_config_dbncache_set, CONFIG_FLAG_ALLOW_RUNNING_CHANGE}, {CONFIG_MAXPASSBEFOREMERGE, CONFIG_TYPE_INT, "100", &ldbm_config_maxpassbeforemerge_get, &ldbm_config_maxpassbeforemerge_set, 0}, @@ -1536,7 +1543,7 @@ static config_info ldbm_config[] = { {CONFIG_DB_HOME_DIRECTORY, CONFIG_TYPE_STRING, "", &ldbm_config_db_home_directory_get, &ldbm_config_db_home_directory_set, 0}, {CONFIG_IMPORT_CACHE_AUTOSIZE, CONFIG_TYPE_INT, "-1", &ldbm_config_import_cache_autosize_get, &ldbm_config_import_cache_autosize_set, CONFIG_FLAG_ALWAYS_SHOW|CONFIG_FLAG_ALLOW_RUNNING_CHANGE}, {CONFIG_CACHE_AUTOSIZE, CONFIG_TYPE_INT, "0", &ldbm_config_cache_autosize_get, &ldbm_config_cache_autosize_set, 0}, - {CONFIG_CACHE_AUTOSIZE_SPLIT, CONFIG_TYPE_INT, "50", &ldbm_config_cache_autosize_split_get, &ldbm_config_cache_autosize_split_set, 0}, + {CONFIG_CACHE_AUTOSIZE_SPLIT, CONFIG_TYPE_INT, "40", &ldbm_config_cache_autosize_split_get, &ldbm_config_cache_autosize_split_set, 0}, {CONFIG_IMPORT_CACHESIZE, CONFIG_TYPE_SIZE_T, "20000000", &ldbm_config_import_cachesize_get, &ldbm_config_import_cachesize_set, CONFIG_FLAG_ALWAYS_SHOW|CONFIG_FLAG_ALLOW_RUNNING_CHANGE}, {CONFIG_IDL_SWITCH, CONFIG_TYPE_STRING, "new", &ldbm_config_idl_get_idl_new, &ldbm_config_idl_set_tune, CONFIG_FLAG_ALWAYS_SHOW}, {CONFIG_IDL_UPDATE, CONFIG_TYPE_ONOFF, "on", &ldbm_config_idl_get_update, &ldbm_config_idl_set_update, 0}, @@ -1616,7 +1623,7 @@ ldbm_config_read_instance_entries(struct ldbminfo *li, const char *backend_type) return rc; } -/* Reads in any config information held in the dse for the ldbm plugin. +/* Reads in any config information held in the dse for the ldbm plugin. * Creates dse entries used to configure the ldbm plugin and dblayer * if they don't already exist. Registers dse callback functions to * maintain those dse entries. Returns 0 on success. diff --git a/ldap/servers/slapd/back-ldbm/start.c b/ldap/servers/slapd/back-ldbm/start.c index 836e67f..56dd2c4 100644 --- a/ldap/servers/slapd/back-ldbm/start.c +++ b/ldap/servers/slapd/back-ldbm/start.c @@ -17,6 +17,8 @@ #include "back-ldbm.h" + + static int initialized = 0; int @@ -25,6 +27,254 @@ ldbm_back_isinitialized() return initialized; } +static int +ldbm_back_start_autotune(struct ldbminfo *li) { + Object *inst_obj = NULL; + ldbm_instance *inst = NULL; + /* size_t is a platform unsigned int, IE uint64_t */ + size_t total_cache_size = 0; + size_t pagesize = 0; + size_t pages = 0; + size_t procpages __attribute__((unused)) = 0; + size_t availpages = 0; + size_t cache_size_to_configure = 0; + size_t zone_pages = 0; + size_t db_pages = 0; + size_t entry_pages = 0; + size_t import_pages = 0; + size_t zone_size = 0; + size_t import_size = 0; + size_t cache_size = 0; + size_t db_size = 0; + /* For clamping the autotune value to a 64Mb boundary */ + size_t clamp_pages = 0; + size_t clamp_div = 0; + size_t clamp_mod = 0; + /* Backend count */ + size_t backend_count = 0; + + int_fast32_t autosize_percentage = 0; + int_fast32_t autosize_db_percentage_split = 0; + int_fast32_t import_percentage = 0; + int32_t issane = 0; + char *msg = ""; /* This will be set by one of the two cache sizing paths below. */ + char size_to_str[32]; /* big enough to hold %ld */ + + /* == Begin autotune == */ + + /* + * The process that we take here now defaults to autotune first, then override + * with manual values if so chosen. + * + * This means first off, we need to check for valid autosizing values. + * We then calculate what our system tuning would be. We clamp these to the + * nearest value. IE 487MB would be 510656512 bytes, so we clamp this to + * 536870912 bytes, aka 512MB. This is aligned to 64MB boundaries. + * + * Now that we have these values, we then check the values of dbcachesize + * and cachememsize. If they are 0, we set them to the auto-calculated value. + * If they are non-0, we skip the value. + * + * This way, we are really autotuning on "first run", and if the admin wants + * to up the values, they merely need to reset the value to 0, and let the + * server restart. + * + * wibrown 2017 + */ + + /* sanity check the autosizing values, + no value or sum of values larger than 100. + */ + backend_count = objset_size(li->li_instance_set); + + /* If autosize == 0, set autosize_per to 10. */ + if (li->li_cache_autosize <= 0) { + /* First, set our message. In the case autosize is 0, we calculate some + * sane defaults and populate these values, but it's only on first run. + */ + msg = "This can be corrected by altering the values of nsslapd-dbcachesize, nsslapd-cachememsize and nsslapd-dncachememsize\n"; + autosize_percentage = 10; + } else { + /* In this case we really are setting the values each start up, so + * change the msg. + */ + msg = "This can be corrected by altering the values of nsslapd-cache-autosize, nsslapd-cache-autosize-split and nsslapd-dncachememsize\n"; + autosize_percentage = li->li_cache_autosize; + } + /* Has to be less than 0, 0 means to disable I think */ + if (li->li_import_cache_autosize < 0) { + import_percentage = 50; + } else { + import_percentage = li->li_import_cache_autosize; + } + /* This doesn't control the availability of the feature, so we can take the + * default from ldbm_config.c + */ + autosize_db_percentage_split = li->li_cache_autosize_split; + + + /* Check the values are sane. */ + if ((autosize_percentage > 100) || (import_percentage > 100) || (autosize_db_percentage_split > 100) || + ((autosize_percentage > 0) && (import_percentage > 0) && (autosize_percentage + import_percentage > 100))) { + slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "Cache autosizing: bad settings, value or sum of values can not larger than 100.\n"); + return SLAPI_FAIL_GENERAL; + } + + if (util_info_sys_pages(&pagesize, &pages, &procpages, &availpages) != 0) { + slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "Unable to determine system page limits\n"); + return SLAPI_FAIL_GENERAL; + } + + if (pagesize == 0) { + /* If this happens, we are in a very bad state indeed... */ + slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "Unable to determine system page size\n"); + return SLAPI_FAIL_GENERAL; + } + + /* calculate the needed values */ + zone_pages = (autosize_percentage * pages) / 100; + zone_size = zone_pages * pagesize; + /* This is how much we "might" use, lets check it's sane. */ + /* In the case it is not, this will *reduce* the allocation */ + issane = util_is_cachesize_sane(&zone_size); + if (!issane) { + slapi_log_err(SLAPI_LOG_WARNING, "ldbm_back_start", "Your autosized cache values have been reduced. Likely your nsslapd-cache-autosize percentage is too high.\n"); + slapi_log_err(SLAPI_LOG_WARNING, "ldbm_back_start", "%s", msg); + } + /* It's valid, lets divide it up and set according to user prefs */ + zone_pages = zone_size / pagesize; + db_pages = (autosize_db_percentage_split * zone_pages) / 100; + + /* Cap the DB size at 512MB, as this doesn't help perf much more (lkrispen's advice) */ + if ((db_pages * pagesize) > (512 * MEGABYTE)) { + db_pages = (512 * MEGABYTE) / pagesize; + } + + /* Number of entry cache pages per backend. */ + entry_pages = (zone_pages - db_pages) / backend_count; + /* Now, clamp this value to a 64mb boundary. */ + /* How many pages are in 64mb? */ + clamp_pages = (64 * MEGABYTE) / pagesize; + /* Now divide the entry pages by this, and also mod. If mod != 0, we need + * to add 1 to the diveded number. This should give us: + * 510 * 1024 * 1024 == 510MB + * 534773760 bytes + * 130560 pages at 4096 pages. + * 16384 pages for 64Mb + * 130560 / 16384 = 7 + * 130560 % 16384 = 15872 which is != 0 + * therfore 7 + 1, aka 8 * 16384 = 131072 pages = 536870912 bytes = 512MB. + */ + clamp_div = entry_pages / clamp_pages; + clamp_mod = entry_pages % clamp_pages; + if (clamp_mod != 0) { + /* If we want to clamp down, remove this line. This would change the above from 510mb -> 448mb. */ + clamp_div += 1; + entry_pages = clamp_div * clamp_pages; + } + + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", "found %luk physical memory\n", pages*(pagesize/1024)); + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", "found %luk avaliable\n", zone_pages*(pagesize/1024)); + + /* We've now calculated the autotuning values. Do we need to apply it? + * we use the logic of "if size is 0, or autosize is > 0. This way three + * options can happen. + * + * First, during first run, dbcache is 0, and autosize is 0. So we apply + * the autotuned value ONLY on first run. + * Second, once the admin sets a value, or autotuning set a value, it sticks. + * Third, if the admin really does want autosizing to take effect every + * start up, we disregard the defined value. + */ + + /* First, check the dbcache */ + if (li->li_dbcachesize == 0 || li->li_cache_autosize > 0) { + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", "cache autosizing: db cache: %luk\n", db_pages*(pagesize/1024)); + cache_size_to_configure = (unsigned long)(db_pages * pagesize); + if (cache_size_to_configure < (500 * MEGABYTE)) { + cache_size_to_configure = (unsigned long)((db_pages * pagesize) / 1.25); + } + /* Have to set this value through text. */ + sprintf(size_to_str, "%lu", cache_size_to_configure); + ldbm_config_internal_set(li, CONFIG_DBCACHESIZE, size_to_str); + } + total_cache_size += li->li_dbcachesize; + + /* For each backend */ + /* apply the appropriate cache size if 0 */ + li->li_cache_autosize_ec = (unsigned long)entry_pages * pagesize; + + for (inst_obj = objset_first_obj(li->li_instance_set); inst_obj; + inst_obj = objset_next_obj(li->li_instance_set, inst_obj)) { + + inst = (ldbm_instance *)object_get_data(inst_obj); + cache_size = (PRUint64)cache_get_max_size(&(inst->inst_cache)); + + /* This is the point where we decide to apply or not. + * We have to check for the mincachesize as setting 0 resets + * to this value. This could cause an issue with a *tiny* install, but + * it's highly unlikely. + */ + if (cache_size == 0 || cache_size == MINCACHESIZE || li->li_cache_autosize > 0) { + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", "cache autosizing: %s entry cache (%lu total): %luk\n", inst->inst_name, backend_count, entry_pages*(pagesize/1024)); + cache_set_max_entries(&(inst->inst_cache), -1); + cache_set_max_size(&(inst->inst_cache), li->li_cache_autosize_ec, CACHE_TYPE_ENTRY); + } + /* Refresh this value now. */ + cache_size = (PRUint64)cache_get_max_size(&(inst->inst_cache)); + db_size = dblayer_get_id2entry_size(inst); + if (cache_size < db_size) { + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", + "%s: entry cache size %lu B is " + "less than db size %lu B; " + "We recommend to increase the entry cache size " + "nsslapd-cachememsize.\n", + inst->inst_name, cache_size, db_size); + } + /* We need to get each instances dncache size to add to the total */ + /* Else we can't properly check the cache allocations below */ + /* Trac 48831 exists to allow this to be auto-sized too ... */ + total_cache_size += (PRUint64)cache_get_max_size(&(inst->inst_dncache)); + total_cache_size += cache_size; + } + /* autosizing importCache */ + if (li->li_import_cache_autosize > 0) { + /* Use import percentage here, as it's been corrected for -1 behaviour */ + import_pages = (import_percentage * pages) / 100; + import_size = import_pages * pagesize; + issane = util_is_cachesize_sane(&import_size); + if (!issane) { + slapi_log_err(SLAPI_LOG_WARNING, "ldbm_back_start", "Your autosized import cache values have been reduced. Likely your nsslapd-import-cache-autosize percentage is too high.\n"); + } + /* We just accept the reduced allocation here. */ + import_pages = import_size / pagesize; + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", "cache autosizing: import cache: %luk\n", + import_pages*(pagesize/1024)); + + sprintf(size_to_str, "%lu", (unsigned long)(import_pages * pagesize)); + ldbm_config_internal_set(li, CONFIG_IMPORT_CACHESIZE, size_to_str); + } + + /* Finally, lets check that the total result is sane. */ + slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", "total cache size: %lu B; \n", total_cache_size); + + issane = util_is_cachesize_sane(&total_cache_size); + if (!issane) { + /* Right, it's time to panic */ + slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "It is highly likely your memory configuration of all backends will EXCEED your systems memory.\n"); + slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "In a future release this WILL prevent server start up. You MUST alter your configuration.\n"); + slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "Total entry cache size: %lu B; dbcache size: %lu B; available memory size: %lu B; \n", + (PRUint64)total_cache_size, (PRUint64)li->li_dbcachesize, availpages * pagesize + ); + slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "%s\n", msg); + /* WB 2016 - This should be UNCOMMENTED in a future release */ + /* return SLAPI_FAIL_GENERAL; */ + } + + /* == End autotune == */ + return 0; +} + /* * Start the LDBM plugin, and all its instances. */ @@ -32,34 +282,9 @@ int ldbm_back_start( Slapi_PBlock *pb ) { struct ldbminfo *li; - char *home_dir; - int action; - int retval; - int issane = 0; - PRUint64 total_cache_size = 0; - size_t pagesize = 0; - size_t pages = 0; - size_t procpages = 0; - size_t availpages = 0; - char *msg = ""; /* This will be set by one of the two cache sizing paths below. */ - - char s[32]; /* big enough to hold %ld */ - unsigned long cache_size_to_configure = 0; - int zone_pages; - int db_pages; - int entry_pages; - int import_pages; - size_t zone_size; - size_t import_size; - size_t total_size; - Object *inst_obj; - ldbm_instance *inst; - PRUint64 cache_size; - PRUint64 dncache_size; - PRUint64 db_size; -#ifndef LINUX - PRUint64 memsize = pages * pagesize; -#endif + char *home_dir = NULL; + int action = 0 ; + int retval = 0; slapi_log_err(SLAPI_LOG_TRACE, "ldbm_back_start", "ldbm backend starting\n"); @@ -124,163 +349,12 @@ ldbm_back_start( Slapi_PBlock *pb ) ldbm_config_internal_set(li, CONFIG_DIRECTORY, "get default"); } - /* sanity check the autosizing values, - no value or sum of values larger than 100. - */ - if ((li->li_cache_autosize > 100) || - (li->li_cache_autosize_split > 100) || - (li->li_import_cache_autosize > 100) || - ((li->li_cache_autosize > 0) && (li->li_import_cache_autosize > 0) && - (li->li_cache_autosize + li->li_import_cache_autosize > 100))) { - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "Cache autosizing: bad settings, " - "value or sum of values can not larger than 100.\n"); - } else { - if (util_info_sys_pages(&pagesize, &pages, &procpages, &availpages) != 0) { - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "Unable to determine system page limits\n"); - return SLAPI_FAIL_GENERAL; - } - if (pagesize) { - if (li->li_cache_autosize == 0) { - /* First, set our message. */ - msg = "This can be corrected by altering the values of nsslapd-dbcachesize, nsslapd-cachememsize and nsslapd-dncachememsize\n"; - - for (inst_obj = objset_first_obj(li->li_instance_set); inst_obj; - inst_obj = objset_next_obj(li->li_instance_set, inst_obj)) { - inst = (ldbm_instance *)object_get_data(inst_obj); - cache_size = (PRUint64)cache_get_max_size(&(inst->inst_cache)); - db_size = dblayer_get_id2entry_size(inst); - if (cache_size < db_size) { - slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", - "%s: entry cache size %lu B is " - "less than db size %lu B; " - "We recommend to increase the entry cache size " - "nsslapd-cachememsize.\n", - inst->inst_name, cache_size, db_size); - } else { - slapi_log_err(SLAPI_LOG_BACKLDBM, - "ldbm_back_start", "%s: entry cache size: %lu B; db size: %lu B\n", - inst->inst_name, cache_size, db_size); - } - /* Get the dn_cachesize */ - dncache_size = (PRUint64)cache_get_max_size(&(inst->inst_dncache)); - total_cache_size += cache_size + dncache_size; - slapi_log_err(SLAPI_LOG_BACKLDBM, - "ldbm_back_start", "total cache size: %lu B; \n", - total_cache_size); - } - slapi_log_err(SLAPI_LOG_BACKLDBM, - "ldbm_back_start", "Total entry cache size: %lu B; " - "dbcache size: %lu B; " - "available memory size: %lu B;\n", -#ifdef LINUX - (PRUint64)total_cache_size, (PRUint64)li->li_dbcachesize, availpages * pagesize -#else - (PRUint64)total_cache_size, (PRUint64)li->li_dbcachesize, memsize -#endif - ); - - /* autosizing dbCache and entryCache */ - } else if (li->li_cache_autosize > 0) { - msg = "This can be corrected by altering the values of nsslapd-cache-autosize, nsslapd-cache-autosize-split and nsslapd-dncachememsize\n"; - zone_pages = (li->li_cache_autosize * pages) / 100; - zone_size = zone_pages * pagesize; - /* This is how much we "might" use, lets check it's sane. */ - /* In the case it is not, this will *reduce* the allocation */ - issane = util_is_cachesize_sane(&zone_size); - if (!issane) { - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", - "Your autosized cache values have been reduced. Likely your nsslapd-cache-autosize percentage is too high.\n"); - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "%s", msg); - } - /* It's valid, lets divide it up and set according to user prefs */ - zone_pages = zone_size / pagesize; - db_pages = (li->li_cache_autosize_split * zone_pages) / 100; - entry_pages = (zone_pages - db_pages) / objset_size(li->li_instance_set); - /* We update this for the is-sane check below. */ - total_cache_size = (zone_pages - db_pages) * pagesize; - - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "cache autosizing. found %luk physical memory\n", - pages*(pagesize/1024)); - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "cache autosizing. found %luk avaliable\n", - zone_pages*(pagesize/1024)); - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "cache autosizing: db cache: %luk, " - "each entry cache (%d total): %luk\n", - db_pages*(pagesize/1024), objset_size(li->li_instance_set), - entry_pages*(pagesize/1024)); - - /* libdb allocates 1.25x the amount we tell it to, - * but only for values < 500Meg - * For the larger memory, the overhead is relatively small. */ - cache_size_to_configure = (unsigned long)(db_pages * pagesize); - if (cache_size_to_configure < (500 * MEGABYTE)) { - cache_size_to_configure = (unsigned long)((db_pages * pagesize) / 1.25); - } - sprintf(s, "%lu", cache_size_to_configure); - ldbm_config_internal_set(li, CONFIG_DBCACHESIZE, s); - li->li_cache_autosize_ec = (unsigned long)entry_pages * pagesize; - - for (inst_obj = objset_first_obj(li->li_instance_set); inst_obj; - inst_obj = objset_next_obj(li->li_instance_set, inst_obj)) { - inst = (ldbm_instance *)object_get_data(inst_obj); - cache_set_max_entries(&(inst->inst_cache), -1); - cache_set_max_size(&(inst->inst_cache), - li->li_cache_autosize_ec, CACHE_TYPE_ENTRY); - /* We need to get each instances dncache size to add to the total */ - /* Else we can't properly check the cache allocations below */ - /* Trac 48831 exists to allow this to be auto-sized too ... */ - total_cache_size += (PRUint64)cache_get_max_size(&(inst->inst_dncache)); - } - } - /* autosizing importCache */ - if (li->li_import_cache_autosize > 0) { - /* For some reason, -1 means 50 ... */ - if (li->li_import_cache_autosize == -1) { - li->li_import_cache_autosize = 50; - } - import_pages = (li->li_import_cache_autosize * pages) / 100; - import_size = import_pages * pagesize; - issane = util_is_cachesize_sane(&import_size); - if (!issane) { - slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", - "Your autosized import cache values have been reduced. " - "Likely your nsslapd-import-cache-autosize percentage is too high.\n"); - } - /* We just accept the reduced allocation here. */ - import_pages = import_size / pagesize; - slapi_log_err(SLAPI_LOG_NOTICE, "ldbm_back_start", "cache autosizing: import cache: %luk\n", - import_pages*(pagesize/1024)); - - sprintf(s, "%lu", (unsigned long)(import_pages * pagesize)); - ldbm_config_internal_set(li, CONFIG_IMPORT_CACHESIZE, s); - } - } - } - - /* Finally, lets check that the total result is sane. */ - - total_size = total_cache_size + (PRUint64)li->li_dbcachesize; - issane = util_is_cachesize_sane(&total_size); - if (!issane) { - /* Right, it's time to panic */ - slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", - "It is highly likely your memory configuration of all backends will EXCEED your systems memory.\n"); - slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", - "In a future release this WILL prevent server start up. You MUST alter your configuration.\n"); - slapi_log_err(SLAPI_LOG_CRIT, "ldbm_back_start", "Total entry cache size: %lu B; " - "dbcache size: %lu B; available memory size: %lu B; \n", -#ifdef LINUX - (PRUint64)total_cache_size, (PRUint64)li->li_dbcachesize, availpages * pagesize -#else - (PRUint64)total_cache_size, (PRUint64)li->li_dbcachesize, memsize -#endif - ); - slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "%s\n", msg); - /* WB 2016 - This should be UNCOMMENTED in a future release */ - /* return SLAPI_FAIL_GENERAL; */ + retval = ldbm_back_start_autotune(li); + if (retval != 0) { + slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_start", "Failed to set database tuning on backends\n"); + return SLAPI_FAIL_GENERAL; } - - retval = check_db_version(li, &action); if (0 != retval) { commit a694a4c7dbe52afb6ecb4c9715a18d4f192304bc Author: William Brown <firstyear(a)redhat.com> Date: Thu Jan 12 14:02:59 2017 +1000 Ticket 49089 - Fix invalid cxxlink statement from hpux Bug Description: As a remnant of past build issues, in certain plugins we defined that we needed a CXXLINK statement. This increased the likelihood of a broken build in parallel, but does not completely remove it. Fix Description: The CXXLINK is not needed now, so we have commented it out for now, with intent to remove in the future if no issues arise. https://fedorahosted.org/389/ticket/49089 Author: wibrown Review by: nhosoi, lslebodn (Thanks!) diff --git a/Makefile.am b/Makefile.am index 7d49e88..5f27681 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1182,7 +1182,7 @@ libacl_plugin_la_SOURCES = ldap/servers/plugins/acl/acl.c \ libacl_plugin_la_CPPFLAGS = -I$(srcdir)/include/libaccess $(PLUGIN_CPPFLAGS) libacl_plugin_la_LIBADD = libslapd.la libns-dshttpd.la $(LDAPSDK_LINK) $(NSPR_LINK) $(LIBCSTD) $(LIBCRUN) libacl_plugin_la_LDFLAGS = -avoid-version -libacl_plugin_la_LINK = $(CXXLINK) -avoid-version +# libacl_plugin_la_LINK = $(CXXLINK) -avoid-version #------------------------ # libaddn-plugin @@ -1276,7 +1276,7 @@ libcollation_plugin_la_SOURCES = ldap/servers/plugins/collation/collate.c \ libcollation_plugin_la_CPPFLAGS = $(PLUGIN_CPPFLAGS) @icu_inc@ libcollation_plugin_la_LIBADD = libslapd.la $(LDAPSDK_LINK) $(NSPR_LINK) $(ICU_LINK) $(LIBCSTD) $(LIBCRUN) libcollation_plugin_la_LDFLAGS = -avoid-version -libcollation_plugin_la_LINK = $(CXXLINK) -avoid-version +# libcollation_plugin_la_LINK = $(CXXLINK) -avoid-version #------------------------ # libcos-plugin

1 0

dirsrvtests/tests
by William Brown 12 Jan '17

12 Jan '17

dirsrvtests/tests/suites/basic/basic_test.py | 3 + dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py | 2 + dirsrvtests/tests/tickets/ticket1347760_test.py | 4 +- dirsrvtests/tests/tickets/ticket48272_test.py | 4 ++ dirsrvtests/tests/tickets/ticket48383_test.py | 8 ++-- dirsrvtests/tests/tickets/ticket48956_test.py | 27 ++++++--------- 6 files changed, 27 insertions(+), 21 deletions(-) New commits: commit f0fdc4d515fd2dfa492100eb5583f9fa14cee134 Author: William Brown <firstyear(a)redhat.com> Date: Thu Jan 12 08:20:40 2017 +1000 Ticket 49087 - ds resolve jenkins issues. Bug Description: Our jenkins tests were failing, resolve it! Fix Description: This fixes the use of the paths module in many cases, and fixes the debugging in addn tests. https://fedorahosted.org/389/ticket/49087 Author: wibrown Review by: nhosoi (Thanks!) diff --git a/dirsrvtests/tests/suites/basic/basic_test.py b/dirsrvtests/tests/suites/basic/basic_test.py index 21db6c5..c7b9566 100644 --- a/dirsrvtests/tests/suites/basic/basic_test.py +++ b/dirsrvtests/tests/suites/basic/basic_test.py @@ -8,6 +8,7 @@ # from subprocess import check_output +import time import ldap.sasl import pytest from lib389.tasks import * @@ -659,6 +660,8 @@ def test_basic_dse(topology_st, import_example_ldif): log.info('dse.ldif was not corrupted, and the server was restarted') log.info('test_basic_dse: PASSED') + # Give the server time to startup, in some conditions this can be racey without systemd notification. Only affects this one test though... + time.sleep(10) @pytest.mark.parametrize("rootdse_attr_name", ROOTDSE_DEF_ATTR_LIST) diff --git a/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py b/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py index c32f3e5..1ae1034 100644 --- a/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py +++ b/dirsrvtests/tests/suites/dynamic-plugins/plugin_tests.py @@ -1783,6 +1783,8 @@ def test_passthru(inst, args=None): args_instance[SER_CREATION_SUFFIX] = PASS_SUFFIX1 args_passthru_inst = args_instance.copy() passthru_inst.allocate(args_passthru_inst) + if passthru_inst.exists(): + passthru_inst.delete() passthru_inst.create() passthru_inst.open() diff --git a/dirsrvtests/tests/tickets/ticket1347760_test.py b/dirsrvtests/tests/tickets/ticket1347760_test.py index e7fb07f..9019764 100644 --- a/dirsrvtests/tests/tickets/ticket1347760_test.py +++ b/dirsrvtests/tests/tickets/ticket1347760_test.py @@ -420,8 +420,8 @@ def test_ticket1347760(topology_st): check_op_result(topology_st.standalone, 'delete', BOGUSDN, None, exists, rc) log.info('Inactivate %s' % BINDDN) - nsinactivate = '%s/sbin/ns-inactivate.pl' % topology_st.standalone.prefix - p = Popen([nsinactivate, '-Z', 'standalone', '-D', DN_DM, '-w', PASSWORD, '-I', BINDDN]) + nsinactivate = os.path.join(topology_st.standalone.get_sbin_dir(), 'ns-inactivate.pl') + p = Popen([nsinactivate, '-Z', topology_st.standalone.serverid, '-D', DN_DM, '-w', PASSWORD, '-I', BINDDN]) assert (p.wait() == 0) log.info('Bind as {%s,%s} which should fail with %s.' % (BINDDN, BUID, ldap.UNWILLING_TO_PERFORM.__name__)) diff --git a/dirsrvtests/tests/tickets/ticket48272_test.py b/dirsrvtests/tests/tickets/ticket48272_test.py index 2b71609..8f3af70 100644 --- a/dirsrvtests/tests/tickets/ticket48272_test.py +++ b/dirsrvtests/tests/tickets/ticket48272_test.py @@ -79,6 +79,10 @@ def test_ticket48272(topology_st): # Add a user _create_user(topology_st.standalone, USER1, USER1_DN) + if DEBUGGING is not False: + print("Attach now") + time.sleep(20) + # Make sure our binds still work. assert (_bind(USER1_DN, PW)) # Test an anonymous bind diff --git a/dirsrvtests/tests/tickets/ticket48383_test.py b/dirsrvtests/tests/tickets/ticket48383_test.py index 50f17f0..b1fa93d 100644 --- a/dirsrvtests/tests/tickets/ticket48383_test.py +++ b/dirsrvtests/tests/tickets/ticket48383_test.py @@ -66,15 +66,15 @@ def test_ticket48383(topology_st): # Given the formula, by default, this means DS will make the buffsize 400k # So an object with a 1MB attribute should break indexing + ldifpath = os.path.join(topology_st.standalone.get_ldif_dir(), "%s.ldif" % SERVERID_STANDALONE) + # stop the server topology_st.standalone.stop(timeout=30) # Now export and import the DB. It's easier than db2index ... topology_st.standalone.db2ldif(bename=DEFAULT_BENAME, suffixes=[DEFAULT_SUFFIX], excludeSuffixes=[], - encrypt=False, repl_data=True, - outputfile='{}/{}.ldif'.format(topology_st.standalone.ldifdir, SERVERID_STANDALONE)) + encrypt=False, repl_data=True, outputfile=ldifpath) - result = topology_st.standalone.ldif2db(DEFAULT_BENAME, None, None, False, - '{}/{}.ldif'.format(topology_st.standalone.ldifdir, SERVERID_STANDALONE)) + result = topology_st.standalone.ldif2db(DEFAULT_BENAME, None, None, False, ldifpath) assert (result) diff --git a/dirsrvtests/tests/tickets/ticket48956_test.py b/dirsrvtests/tests/tickets/ticket48956_test.py index b1200be..8715d9e 100644 --- a/dirsrvtests/tests/tickets/ticket48956_test.py +++ b/dirsrvtests/tests/tickets/ticket48956_test.py @@ -27,21 +27,18 @@ log = logging.getLogger(__name__) def _check_status(topology_st, user, expected): - nsaccountstatus = '%s/sbin/ns-accountstatus.pl' % topology_st.standalone.prefix - proc = subprocess.Popen( - [nsaccountstatus, '-Z', 'standalone', '-D', DN_DM, '-w', PASSWORD, '-p', str(topology_st.standalone.port), '-I', - user], stdout=subprocess.PIPE, stderr=subprocess.PIPE) - - found = False - while True: - l = proc.stdout.readline() - log.info("output: %s" % l) - if l == "": - break - if expected in l: - found = True - break - return found + nsaccountstatus = os.path.join(topology_st.standalone.ds_paths.sbin_dir, "ns-accountstatus.pl") + + try: + output = subprocess.check_output( [nsaccountstatus, '-Z', topology_st.standalone.serverid, '-D', DN_DM, '-w', PASSWORD, '-p', str(topology_st.standalone.port), '-I', user]) + except subprocess.CalledProcessError as err: + output = err.output + + log.info("output: %s" % output) + + if expected in output: + return True + return False def _check_inactivity(topology_st, mysuffix):

1 0

Branch '389-ds-base-1.3.5' - 3 commits - dirsrvtests/tests ldap/servers
by Noriko Hosoi 12 Jan '17

12 Jan '17

dirsrvtests/tests/tickets/ticket548_test.py | 7 + ldap/servers/slapd/pw.c | 99 +++++++++++----------------- 2 files changed, 48 insertions(+), 58 deletions(-) New commits: commit b9e565df5d304ba9fb516b987ad62480590a5845 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Wed Jan 11 15:14:07 2017 -0800 Ticket #49082 - Adjusted the CI test case to the fix. Description: Fix password expiration related shadow attributes (cherry picked from commit 5a6a5a18d0458bd147af57a06158245f329ddba3) diff --git a/dirsrvtests/tests/tickets/ticket548_test.py b/dirsrvtests/tests/tickets/ticket548_test.py index a8562c2..b0feff4 100644 --- a/dirsrvtests/tests/tickets/ticket548_test.py +++ b/dirsrvtests/tests/tickets/ticket548_test.py @@ -95,6 +95,13 @@ def set_global_pwpolicy(topology, min_=1, max_=10, warn=3): log.error('Failed to set passwordMinAge: error ' + e.message['desc']) assert False + log.info(" Set global password Expiration -- on\n") + try: + topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordExp', 'on')]) + except ldap.LDAPError as e: + log.error('Failed to set passwordExp: error ' + e.message['desc']) + assert False + log.info(" Set global password Max Age -- %s days\n" % max_) try: topology.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordMaxAge', '%s' % max_secs)]) commit faae0fa5a4a6b3d590c1a9e068d9436965cc49c9 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Wed Jan 11 15:04:42 2017 -0800 Ticket #49082 - Fix password expiration related shadow attributes The original patch was provided by Gordon Messmer (gordon.messmer(a)gmail.com) with the description: Bug description: Shadow attributes (in /etc/shadow and in LDAP) are typically unset when no policy is in place. 389-ds will incorrectly return values (possibly set to 0) when there is no policy. Fix description: Only auto-fill shadow attributes when a password policy is available. These are empty when no policy is in place. Don't auto-fill expiration related shadow attributes if passwords never expire. Reviewed by William Brown <wibrown(a)redhat.com> (Thanks!!). (cherry picked from commit 5bcd966b73708f6b558f01e6b11a7a11e8d3b126) diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index ce1ca2a..30a2cb9 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -2802,7 +2802,7 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) { const char *dn = NULL; passwdPolicy *pwpolicy = NULL; - long long shadowval = 0; + long long shadowval = -1; Slapi_Mods *smods = NULL; LDAPMod **mods; long long sval; @@ -2840,64 +2840,66 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } - } else { - shadowval = 0; } - shmin = slapi_entry_attr_get_charptr(*e, "shadowMin"); - if (shmin) { - sval = strtoll(shmin, NULL, 0); - if (sval != shadowval) { - slapi_ch_free_string(&shmin); - shmin = slapi_ch_smprintf("%lld", shadowval); + if (shadowval > 0) { + shmin = slapi_entry_attr_get_charptr(*e, "shadowMin"); + if (shmin) { + sval = strtoll(shmin, NULL, 0); + if (sval != shadowval) { + slapi_ch_free_string(&shmin); + shmin = slapi_ch_smprintf("%lld", shadowval); + mod_num++; + } + } else { mod_num++; + shmin = slapi_ch_smprintf("%lld", shadowval); } - } else { - mod_num++; - shmin = slapi_ch_smprintf("%lld", shadowval); } /* shadowMax - the maximum number of days for which the user password remains valid. */ - if (pwpolicy->pw_maxage > 0) { + shadowval = -1; + if (pwpolicy->pw_exp == 1 && pwpolicy->pw_maxage > 0) { shadowval = pwpolicy->pw_maxage / _SEC_PER_DAY; if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } - } else { - shadowval = _MAX_SHADOW; } - shmax = slapi_entry_attr_get_charptr(*e, "shadowMax"); - if (shmax) { - sval = strtoll(shmax, NULL, 0); - if (sval != shadowval) { - slapi_ch_free_string(&shmax); - shmax = slapi_ch_smprintf("%lld", shadowval); + if (shadowval > 0) { + shmax = slapi_entry_attr_get_charptr(*e, "shadowMax"); + if (shmax) { + sval = strtoll(shmax, NULL, 0); + if (sval != shadowval) { + slapi_ch_free_string(&shmax); + shmax = slapi_ch_smprintf("%lld", shadowval); + mod_num++; + } + } else { mod_num++; + shmax = slapi_ch_smprintf("%lld", shadowval); } - } else { - mod_num++; - shmax = slapi_ch_smprintf("%lld", shadowval); } /* shadowWarning - the number of days of advance warning given to the user before the user password expires. */ - if (pwpolicy->pw_warning > 0) { + shadowval = -1; + if (pwpolicy->pw_exp == 1 && pwpolicy->pw_warning > 0) { shadowval = pwpolicy->pw_warning / _SEC_PER_DAY; if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } - } else { - shadowval = 0; } - shwarn = slapi_entry_attr_get_charptr(*e, "shadowWarning"); - if (shwarn) { - sval = strtoll(shwarn, NULL, 0); - if (sval != shadowval) { - slapi_ch_free_string(&shwarn); - shwarn = slapi_ch_smprintf("%lld", shadowval); + if (shadowval > 0) { + shwarn = slapi_entry_attr_get_charptr(*e, "shadowWarning"); + if (shwarn) { + sval = strtoll(shwarn, NULL, 0); + if (sval != shadowval) { + slapi_ch_free_string(&shwarn); + shwarn = slapi_ch_smprintf("%lld", shadowval); + mod_num++; + } + } else { mod_num++; + shwarn = slapi_ch_smprintf("%lld", shadowval); } - } else { - mod_num++; - shwarn = slapi_ch_smprintf("%lld", shadowval); } smods = slapi_mods_new(); commit 2ca12fc5b79dbbb8889eba6da7b4ce59cd6cb86d Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Wed Jan 11 14:14:40 2017 -0800 Ticket #49080 - shadowExpire should not be a calculated value Description: Reverting the changes made on shadowExpire in the ticket 548. Thanks to Gordon Messmer (gordon.messmer(a)gmail.com) for providing the original patch. Reviewed by William Brown <wibrown(a)redhat.com> (Thanks!!). (cherry picked from commit 14eb192b0f99ae3d811fd8a5bb40713bc85ea533) diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index 6f02f90..ce1ca2a 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -2803,7 +2803,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) const char *dn = NULL; passwdPolicy *pwpolicy = NULL; long long shadowval = 0; - long long exptime = 0; Slapi_Mods *smods = NULL; LDAPMod **mods; long long sval; @@ -2811,7 +2810,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) char *shmin = NULL; char *shmax = NULL; char *shwarn = NULL; - char *shexp = NULL; int rc = 0; if (!e || !*e) { @@ -2861,7 +2859,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) /* shadowMax - the maximum number of days for which the user password remains valid. */ if (pwpolicy->pw_maxage > 0) { shadowval = pwpolicy->pw_maxage / _SEC_PER_DAY; - exptime = time_plus_sec(current_time(), pwpolicy->pw_maxage); if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } @@ -2903,22 +2900,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) shwarn = slapi_ch_smprintf("%lld", shadowval); } - /* shadowExpire - the date on which the user login will be disabled. */ - if (exptime) { - shexp = slapi_entry_attr_get_charptr(*e, "shadowExpire"); - exptime /= _SEC_PER_DAY; - if (shexp) { - sval = strtoll(shexp, NULL, 0); - if (sval != exptime) { - slapi_ch_free_string(&shexp); - shexp = slapi_ch_smprintf("%lld", exptime); - mod_num++; - } - } else { - mod_num++; - shexp = slapi_ch_smprintf("%lld", exptime); - } - } smods = slapi_mods_new(); slapi_mods_init(smods, mod_num); if (shmin) { @@ -2933,10 +2914,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) slapi_mods_add(smods, LDAP_MOD_REPLACE, "shadowWarning", strlen(shwarn), shwarn); slapi_ch_free_string(&shwarn); } - if (shexp) { - slapi_mods_add(smods, LDAP_MOD_REPLACE, "shadowExpire", strlen(shexp), shexp); - slapi_ch_free_string(&shexp); - } /* Apply the mods to create the resulting entry. */ mods = slapi_mods_get_ldapmods_byref(smods); if (mods) { @@ -2947,11 +2924,15 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) } slapi_mods_free(&smods); -#if 0 /* These 2 attributes are no need (or not able) to auto-fill. */ +#if 0 /* These 3 attributes are no need (or not able) to auto-fill. */ /* * shadowInactive - the number of days of inactivity allowed for the user. * Password Policy does not have the corresponding parameter. * + * shadowExpire - the number of days since Jan 1, 1970 after which the + * account, not the password, will expire. This is not affected by the + * Password Policy. + * * shadowFlag - not currently in use. */ #endif

1 0

3 commits - dirsrvtests/tests ldap/servers
by Noriko Hosoi 11 Jan '17

11 Jan '17

dirsrvtests/tests/tickets/ticket548_test.py | 7 + ldap/servers/slapd/pw.c | 99 +++++++++++----------------- 2 files changed, 48 insertions(+), 58 deletions(-) New commits: commit 5a6a5a18d0458bd147af57a06158245f329ddba3 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Wed Jan 11 15:14:07 2017 -0800 Ticket #49082 - Adjusted the CI test case to the fix. Description: Fix password expiration related shadow attributes diff --git a/dirsrvtests/tests/tickets/ticket548_test.py b/dirsrvtests/tests/tickets/ticket548_test.py index 8c55254..40b8048 100644 --- a/dirsrvtests/tests/tickets/ticket548_test.py +++ b/dirsrvtests/tests/tickets/ticket548_test.py @@ -54,6 +54,13 @@ def set_global_pwpolicy(topology_st, min_=1, max_=10, warn=3): log.error('Failed to set passwordMinAge: error ' + e.message['desc']) assert False + log.info(" Set global password Expiration -- on\n") + try: + topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordExp', 'on')]) + except ldap.LDAPError as e: + log.error('Failed to set passwordExp: error ' + e.message['desc']) + assert False + log.info(" Set global password Max Age -- %s days\n" % max_) try: topology_st.standalone.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'passwordMaxAge', '%s' % max_secs)]) commit 5bcd966b73708f6b558f01e6b11a7a11e8d3b126 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Wed Jan 11 15:04:42 2017 -0800 Ticket #49082 - Fix password expiration related shadow attributes The original patch was provided by Gordon Messmer (gordon.messmer(a)gmail.com) with the description: Bug description: Shadow attributes (in /etc/shadow and in LDAP) are typically unset when no policy is in place. 389-ds will incorrectly return values (possibly set to 0) when there is no policy. Fix description: Only auto-fill shadow attributes when a password policy is available. These are empty when no policy is in place. Don't auto-fill expiration related shadow attributes if passwords never expire. Reviewed by William Brown <wibrown(a)redhat.com> (Thanks!!). diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index 650c1bc..215c9eb 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -2823,7 +2823,7 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) { const char *dn = NULL; passwdPolicy *pwpolicy = NULL; - long long shadowval = 0; + long long shadowval = -1; Slapi_Mods *smods = NULL; LDAPMod **mods; long long sval; @@ -2861,64 +2861,66 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } - } else { - shadowval = 0; } - shmin = slapi_entry_attr_get_charptr(*e, "shadowMin"); - if (shmin) { - sval = strtoll(shmin, NULL, 0); - if (sval != shadowval) { - slapi_ch_free_string(&shmin); - shmin = slapi_ch_smprintf("%lld", shadowval); + if (shadowval > 0) { + shmin = slapi_entry_attr_get_charptr(*e, "shadowMin"); + if (shmin) { + sval = strtoll(shmin, NULL, 0); + if (sval != shadowval) { + slapi_ch_free_string(&shmin); + shmin = slapi_ch_smprintf("%lld", shadowval); + mod_num++; + } + } else { mod_num++; + shmin = slapi_ch_smprintf("%lld", shadowval); } - } else { - mod_num++; - shmin = slapi_ch_smprintf("%lld", shadowval); } /* shadowMax - the maximum number of days for which the user password remains valid. */ - if (pwpolicy->pw_maxage > 0) { + shadowval = -1; + if (pwpolicy->pw_exp == 1 && pwpolicy->pw_maxage > 0) { shadowval = pwpolicy->pw_maxage / _SEC_PER_DAY; if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } - } else { - shadowval = _MAX_SHADOW; } - shmax = slapi_entry_attr_get_charptr(*e, "shadowMax"); - if (shmax) { - sval = strtoll(shmax, NULL, 0); - if (sval != shadowval) { - slapi_ch_free_string(&shmax); - shmax = slapi_ch_smprintf("%lld", shadowval); + if (shadowval > 0) { + shmax = slapi_entry_attr_get_charptr(*e, "shadowMax"); + if (shmax) { + sval = strtoll(shmax, NULL, 0); + if (sval != shadowval) { + slapi_ch_free_string(&shmax); + shmax = slapi_ch_smprintf("%lld", shadowval); + mod_num++; + } + } else { mod_num++; + shmax = slapi_ch_smprintf("%lld", shadowval); } - } else { - mod_num++; - shmax = slapi_ch_smprintf("%lld", shadowval); } /* shadowWarning - the number of days of advance warning given to the user before the user password expires. */ - if (pwpolicy->pw_warning > 0) { + shadowval = -1; + if (pwpolicy->pw_exp == 1 && pwpolicy->pw_warning > 0) { shadowval = pwpolicy->pw_warning / _SEC_PER_DAY; if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } - } else { - shadowval = 0; } - shwarn = slapi_entry_attr_get_charptr(*e, "shadowWarning"); - if (shwarn) { - sval = strtoll(shwarn, NULL, 0); - if (sval != shadowval) { - slapi_ch_free_string(&shwarn); - shwarn = slapi_ch_smprintf("%lld", shadowval); + if (shadowval > 0) { + shwarn = slapi_entry_attr_get_charptr(*e, "shadowWarning"); + if (shwarn) { + sval = strtoll(shwarn, NULL, 0); + if (sval != shadowval) { + slapi_ch_free_string(&shwarn); + shwarn = slapi_ch_smprintf("%lld", shadowval); + mod_num++; + } + } else { mod_num++; + shwarn = slapi_ch_smprintf("%lld", shadowval); } - } else { - mod_num++; - shwarn = slapi_ch_smprintf("%lld", shadowval); } smods = slapi_mods_new(); commit 14eb192b0f99ae3d811fd8a5bb40713bc85ea533 Author: Noriko Hosoi <nhosoi(a)redhat.com> Date: Wed Jan 11 14:14:40 2017 -0800 Ticket #49080 - shadowExpire should not be a calculated value Description: Reverting the changes made on shadowExpire in the ticket 548. Thanks to Gordon Messmer (gordon.messmer(a)gmail.com) for providing the original patch. Reviewed by William Brown <wibrown(a)redhat.com> (Thanks!!). diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index 5f95cca..650c1bc 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -2824,7 +2824,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) const char *dn = NULL; passwdPolicy *pwpolicy = NULL; long long shadowval = 0; - long long exptime = 0; Slapi_Mods *smods = NULL; LDAPMod **mods; long long sval; @@ -2832,7 +2831,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) char *shmin = NULL; char *shmax = NULL; char *shwarn = NULL; - char *shexp = NULL; int rc = 0; if (!e || !*e) { @@ -2882,7 +2880,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) /* shadowMax - the maximum number of days for which the user password remains valid. */ if (pwpolicy->pw_maxage > 0) { shadowval = pwpolicy->pw_maxage / _SEC_PER_DAY; - exptime = time_plus_sec(current_time(), pwpolicy->pw_maxage); if (shadowval > _MAX_SHADOW) { shadowval = _MAX_SHADOW; } @@ -2924,22 +2921,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) shwarn = slapi_ch_smprintf("%lld", shadowval); } - /* shadowExpire - the date on which the user login will be disabled. */ - if (exptime) { - shexp = slapi_entry_attr_get_charptr(*e, "shadowExpire"); - exptime /= _SEC_PER_DAY; - if (shexp) { - sval = strtoll(shexp, NULL, 0); - if (sval != exptime) { - slapi_ch_free_string(&shexp); - shexp = slapi_ch_smprintf("%lld", exptime); - mod_num++; - } - } else { - mod_num++; - shexp = slapi_ch_smprintf("%lld", exptime); - } - } smods = slapi_mods_new(); slapi_mods_init(smods, mod_num); if (shmin) { @@ -2954,10 +2935,6 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) slapi_mods_add(smods, LDAP_MOD_REPLACE, "shadowWarning", strlen(shwarn), shwarn); slapi_ch_free_string(&shwarn); } - if (shexp) { - slapi_mods_add(smods, LDAP_MOD_REPLACE, "shadowExpire", strlen(shexp), shexp); - slapi_ch_free_string(&shexp); - } /* Apply the mods to create the resulting entry. */ mods = slapi_mods_get_ldapmods_byref(smods); if (mods) { @@ -2968,11 +2945,15 @@ add_shadow_ext_password_attrs(Slapi_PBlock *pb, Slapi_Entry **e) } slapi_mods_free(&smods); -#if 0 /* These 2 attributes are no need (or not able) to auto-fill. */ +#if 0 /* These 3 attributes are no need (or not able) to auto-fill. */ /* * shadowInactive - the number of days of inactivity allowed for the user. * Password Policy does not have the corresponding parameter. * + * shadowExpire - the number of days since Jan 1, 1970 after which the + * account, not the password, will expire. This is not affected by the + * Password Policy. + * * shadowFlag - not currently in use. */ #endif

1 0

ldap/servers
by William Brown 11 Jan '17

11 Jan '17

ldap/servers/slapd/add.c | 6 +++++- ldap/servers/slapd/modify.c | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) New commits: commit 9835e2b94e675f2c7b0922e1084e292f44f98c92 Author: William Brown <firstyear(a)redhat.com> Date: Wed Jan 11 13:16:48 2017 +1000 Ticket 49027 - on secfailure do not store cleartext password content Bug Description: During development of the pbkdf2 module, I noticed that when the backend was unable to hash the password content, the password was stored as {CLEAR}<password> into the database. This may be considered a leak of password material as we write it clear text to disk. Fix Description: If the pw_enc callback from the password module returns any value except 0, we return an unwilling to perform, and generate an error to the error log. This prevents the leak, and notifies the admin and user of the issue quickly. https://fedorahosted.org/389/ticket/49027 Author: wibrown Review by: nhosoi (Thanks!) diff --git a/ldap/servers/slapd/add.c b/ldap/servers/slapd/add.c index 8e671bb..1b994a0 100644 --- a/ldap/servers/slapd/add.c +++ b/ldap/servers/slapd/add.c @@ -567,7 +567,11 @@ static void op_shared_add (Slapi_PBlock *pb) valuearray_add_valuearray(&unhashed_password_vals, present_values, 0); valuearray_add_valuearray(&vals, present_values, 0); - pw_encodevals_ext(pb, slapi_entry_get_sdn (e), vals); + if (pw_encodevals_ext(pb, slapi_entry_get_sdn (e), vals) != 0) { + slapi_log_err(SLAPI_LOG_CRIT, "op_shared_add", "Unable to hash userPassword attribute for %s.\n", slapi_entry_get_dn_const(e)); + send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM, NULL, "Unable to store attribute \"userPassword\" correctly\n", 0, NULL); + goto done; + } add_password_attrs(pb, operation, e); slapi_entry_attr_replace_sv(e, SLAPI_USERPWD_ATTR, vals); valuearray_free(&vals); diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index 51bf057..4bef90a 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -959,7 +959,11 @@ static void op_shared_modify (Slapi_PBlock *pb, int pw_change, char *old_pw) valuearray_init_bervalarray(pw_mod->mod_bvalues, &va); /* encode password */ - pw_encodevals_ext(pb, sdn, va); + if (pw_encodevals_ext(pb, sdn, va) ) { + slapi_log_err(SLAPI_LOG_CRIT, "op_shared_modify", "Unable to hash userPassword attribute for %s.\n", slapi_entry_get_dn_const(e)); + send_ldap_result(pb, LDAP_UNWILLING_TO_PERFORM, NULL, "Unable to store attribute \"userPassword\" correctly\n", 0, NULL); + goto free_and_return; + } /* remove current clear value of userpassword */ ber_bvecfree(pw_mod->mod_bvalues);

1 0

ldap/servers
by thierry bordaz 11 Jan '17

11 Jan '17

ldap/servers/plugins/memberof/memberof.c | 578 ++++++++++++++++++++++++++++++- 1 file changed, 563 insertions(+), 15 deletions(-) New commits: commit dfcef185ff10eb817a6b4620bd9b5f0d20e8b053 Author: Thierry Bordaz <tbordaz(a)redhat.com> Date: Mon Jan 9 16:48:50 2017 +0100 Ticket 49031 - Improve memberof with a cache of ancestors for groups Bug Description: During a group update, memberof computes the ancestors of all impacted leafs and groups. To compute the ancestors it lookup recursively and so is going to compute the ancestors of a given group as many times there are impacted nodes under that group. Fix Description: The ticket implements a cache. It contains for a given group all its ancestors, so that if it is needed to recompute the ancestors of that group it will take it from the cache. https://fedorahosted.org/389/ticket/49031 Reviewed by: William Brown (thanks !!!!!) Platforms tested: F24 Flag Day: no Doc impact: no diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c index 8028163..7e4b828 100644 --- a/ldap/servers/plugins/memberof/memberof.c +++ b/ldap/servers/plugins/memberof/memberof.c @@ -34,6 +34,8 @@ # include <config.h> #endif +#include <ctype.h> +#include <time.h> #include "slapi-plugin.h" #include "string.h" #include "nspr.h" @@ -53,6 +55,7 @@ static int usetxn = 0; static int premodfn = 0; #define MEMBEROF_HASHTABLE_SIZE 1000 static PLHashTable *fixup_entry_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */ +static PLHashTable *group_ancestors_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */ typedef struct _memberofstringll { @@ -66,8 +69,37 @@ typedef struct _memberof_get_groups_data Slapi_Value *memberdn_val; Slapi_ValueSet **groupvals; Slapi_ValueSet **group_norm_vals; + Slapi_ValueSet **already_seen_ndn_vals; + PRBool use_cache; } memberof_get_groups_data; +/* The key to access the hash table is the normalized DN + * The normalized DN is stored in the value because: + * - It is used in slapi_valueset_find + * - It is used to fill the memberof_get_groups_data.group_norm_vals + */ +typedef struct _memberof_cached_value +{ + char *key; + char *group_dn_val; + char *group_ndn_val; + int valid; +} memberof_cached_value; +struct cache_stat +{ + int total_lookup; + int successfull_lookup; + int total_add; + int total_remove; + int total_enumerate; + int cumul_duration_lookup; + int cumul_duration_add; + int cumul_duration_remove; + int cumul_duration_enumerate; +}; +static struct cache_stat cache_stat; +#define MEMBEROF_CACHE_DEBUG 0 + typedef struct _task_data { char *dn; @@ -127,7 +159,7 @@ static int memberof_qsort_compare(const void *a, const void *b); static void memberof_load_array(Slapi_Value **array, Slapi_Attr *attr); static int memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *sdn); static int memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn, MemberOfConfig *config, - char **types, plugin_search_entry_callback callback, void *callback_data); + char **types, plugin_search_entry_callback callback, void *callback_data, int *cached); static int memberof_is_direct_member(MemberOfConfig *config, Slapi_Value *groupdn, Slapi_Value *memberdn); static int memberof_is_grouping_attr(char *type, MemberOfConfig *config); @@ -159,6 +191,12 @@ static int memberof_add_objectclass(char *auto_add_oc, const char *dn); static int memberof_add_memberof_attr(LDAPMod **mods, const char *dn, char *add_oc); static PLHashTable *hashtable_new(); static void fixup_hashtable_empty(char *msg); +static PLHashTable *hashtable_new(); +static void ancestor_hashtable_empty(char *msg); +static void ancestor_hashtable_entry_free(memberof_cached_value *entry); +static memberof_cached_value *ancestors_cache_lookup(const char *ndn); +static PRBool ancestors_cache_remove(const char *ndn); +static PLHashEntry *ancestors_cache_add(const void *key, void *value); /*** implementation ***/ @@ -351,6 +389,7 @@ int memberof_postop_start(Slapi_PBlock *pb) } fixup_entry_hashtable = hashtable_new(); + group_ancestors_hashtable = hashtable_new(); /* Set the alternate config area if one is defined. */ slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_AREA, &config_area); @@ -451,6 +490,10 @@ int memberof_postop_close(Slapi_PBlock *pb) PL_HashTableDestroy(fixup_entry_hashtable); } + if (group_ancestors_hashtable) { + ancestor_hashtable_empty("memberof_postop_close empty group_ancestors_hashtable"); + PL_HashTableDestroy(group_ancestors_hashtable); + } slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM, "<-- memberof_postop_close\n" ); return 0; @@ -602,6 +645,7 @@ memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN * int i = 0; char *groupattrs[2] = {0, 0}; int rc = LDAP_SUCCESS; + int cached = 0; /* Loop through each grouping attribute to find groups that have * dn as a member. For any matches, delete the dn value from the @@ -613,8 +657,9 @@ memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN * groupattrs[0] = config->groupattrs[i]; + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_del_dn_from_groups: Ancestors of %s\n", slapi_sdn_get_dn(sdn)); rc = memberof_call_foreach_dn(pb, sdn, config, groupattrs, - memberof_del_dn_type_callback, &data); + memberof_del_dn_type_callback, &data, &cached); } return rc; @@ -677,6 +722,53 @@ memberof_scope_is_child_of_dn(MemberOfConfig *config, Slapi_DN *sdn) } return NULL; } + +static void +add_ancestors_cbdata(memberof_cached_value *ancestors, void *callback_data) +{ + Slapi_Value *sval; + int val_index; + Slapi_ValueSet *group_norm_vals_cbdata = *((memberof_get_groups_data*) callback_data)->group_norm_vals; + Slapi_ValueSet *group_vals_cbdata = *((memberof_get_groups_data*) callback_data)->groupvals; + Slapi_Value *memberdn_val = ((memberof_get_groups_data*) callback_data)->memberdn_val; + int added_group; + PRBool empty_ancestor = PR_FALSE; + + for (val_index = 0, added_group = 0; ancestors[val_index].valid; val_index++) { + /* For each of its ancestor (not already present) add it to callback_data */ + + if (ancestors[val_index].group_ndn_val == NULL) { + /* This is a node with no ancestor + * ancestors should only contains this empty valid value + * but just in case let the loop continue instead of a break + */ + empty_ancestor = PR_TRUE; + continue; + } + + sval = slapi_value_new_string(ancestors[val_index].group_ndn_val); + if (sval) { + if (!slapi_valueset_find( + ((memberof_get_groups_data*) callback_data)->config->group_slapiattrs[0], group_norm_vals_cbdata, sval)) { + /* This ancestor was not already present in the callback data + * => Add it to the callback_data + * Using slapi_valueset_add_value it consumes sval + * so do not free sval + */ + slapi_valueset_add_value_ext(group_norm_vals_cbdata, sval, SLAPI_VALUE_FLAG_PASSIN); + sval = slapi_value_new_string(ancestors[val_index].group_dn_val); + slapi_valueset_add_value_ext(group_vals_cbdata, sval, SLAPI_VALUE_FLAG_PASSIN); + added_group++; + } else { + /* This ancestor was already present, free sval that will not be consumed */ + slapi_value_free(&sval); + } + } + } + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "add_ancestors_cbdata: Ancestors of %s contained %d groups. %d added. %s\n", + slapi_value_get_string(memberdn_val), val_index, added_group, empty_ancestor ? "no ancestors" : ""); +} + /* * Does a callback search of "type=dn" under the db suffix that "dn" is in, * unless all_backends is set, then we look at all the backends. If "dn" @@ -685,7 +777,7 @@ memberof_scope_is_child_of_dn(MemberOfConfig *config, Slapi_DN *sdn) */ int memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn, - MemberOfConfig *config, char **types, plugin_search_entry_callback callback, void *callback_data) + MemberOfConfig *config, char **types, plugin_search_entry_callback callback, void *callback_data, int *cached) { Slapi_PBlock *search_pb = NULL; Slapi_DN *base_sdn = NULL; @@ -700,11 +792,36 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn, int free_it = 0; int rc = 0; int i = 0; + memberof_cached_value *ht_grp = NULL; + memberof_get_groups_data *data = (memberof_get_groups_data*) callback_data; + const char *ndn = slapi_sdn_get_ndn(sdn); + + *cached = 0; if (!memberof_entry_in_scope(config, sdn)) { return (rc); } + /* Here we will retrieve the ancestor of sdn. + * The key access is the normalized sdn + * This is done through recursive internal searches of parents + * If the ancestors of sdn are already cached, just use + * this value + */ + if (data && data->use_cache) { + ht_grp = ancestors_cache_lookup((const void *) ndn); + if (ht_grp) { +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s already cached (%x)\n", ndn, ht_grp); +#endif + add_ancestors_cbdata(ht_grp, callback_data); + *cached = 1; + return (rc); + } + } +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s not cached\n", ndn); +#endif /* Count the number of types. */ for (num_types = 0; types && types[num_types]; num_types++) { @@ -971,6 +1088,7 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, int i = 0; char *groupattrs[2] = {0, 0}; int ret = LDAP_SUCCESS; + int cached = 0; /* Loop through each grouping attribute to find groups that have * pre_dn as a member. For any matches, replace pre_dn with post_dn @@ -985,9 +1103,10 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, groupattrs[0] = config->groupattrs[i]; + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_replace_dn_from_groups: Ancestors of %s\n", slapi_sdn_get_dn(post_sdn)); if((ret = memberof_call_foreach_dn(pb, pre_sdn, config, groupattrs, memberof_replace_dn_type_callback, - &data))) + &data, &cached))) { break; } @@ -2046,28 +2165,236 @@ memberof_get_groups(MemberOfConfig *config, Slapi_DN *member_sdn) { Slapi_ValueSet *groupvals = slapi_valueset_new(); Slapi_ValueSet *group_norm_vals = slapi_valueset_new(); + Slapi_ValueSet *already_seen_ndn_vals = slapi_valueset_new(); Slapi_Value *memberdn_val = slapi_value_new_string(slapi_sdn_get_ndn(member_sdn)); slapi_value_set_flags(memberdn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS); - memberof_get_groups_data data = {config, memberdn_val, &groupvals, &group_norm_vals}; + memberof_get_groups_data data = {config, memberdn_val, &groupvals, &group_norm_vals, &already_seen_ndn_vals, PR_TRUE}; memberof_get_groups_r(config, member_sdn, &data); slapi_value_free(&memberdn_val); slapi_valueset_free(group_norm_vals); + slapi_valueset_free(already_seen_ndn_vals); return groupvals; } +void +dump_cache_entry(memberof_cached_value *double_check, const char *msg) +{ + int i; + for(i = 0; double_check[i].valid; i++) { + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "dump_cache_entry: %s -> %s\n", + msg ? msg : "<no key>", + double_check[i].group_dn_val ? double_check[i].group_dn_val : "NULL"); + } +} +/* + * A cache value consist of an array of all dn/ndn of the groups member_ndn_val belongs to + * The last element of the array has 'valid=0' + * the firsts elements of the array has 'valid=1' and the dn/ndn of group it belong to + */ +static void +cache_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *groups) +{ + Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)groups)->groupvals; + Slapi_Value *sval; + Slapi_DN *sdn = 0; + const char *dn; + const char *ndn; + const char *key; + char *key_copy; + int hint = 0; + int count; + int index; + memberof_cached_value *cache_entry; +#if MEMBEROF_CACHE_DEBUG + memberof_cached_value *double_check; +#endif + + if ((member_ndn_val == NULL) || (*member_ndn_val == NULL)) { + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Fail to cache groups ancestor of unknown member\n"); + return; + } + + /* Allocate the cache entry and fill it */ + count = slapi_valueset_count(groupvals); + if (count == 0) { + /* There is no group containing member_ndn_val + * so cache the NULL value + */ + cache_entry = (memberof_cached_value *) slapi_ch_calloc(2, sizeof(memberof_cached_value)); + if (!cache_entry) { + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Fail to cache no group are ancestor of %s\n", + slapi_value_get_string(*member_ndn_val)); + return; + } + index = 0; + cache_entry[index].key = NULL; /* only the last element (valid=0) contains the key */ + cache_entry[index].group_dn_val = NULL; + cache_entry[index].group_ndn_val = NULL; + cache_entry[index].valid = 1; /* this entry is valid and indicate no group contain member_ndn_val */ +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: For %s cache %s\n", + slapi_value_get_string(*member_ndn_val), cache_entry[index].group_dn_val ? cache_entry[index].group_dn_val : "<empty>"); +#endif + index++; + + } else { + cache_entry = (memberof_cached_value *) slapi_ch_calloc(count + 1, sizeof (memberof_cached_value)); + if (!cache_entry) { + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Fail to cache groups ancestor of %s\n", + slapi_value_get_string(*member_ndn_val)); + return; + } + + /* Store the dn/ndn into the cache_entry */ + index = 0; + hint = slapi_valueset_first_value(groupvals, &sval); + while (sval) { + /* In case of recursion the member_ndn can be present + * in its own ancestors. Skip it + */ + if (memberof_compare(groups->config, member_ndn_val, &sval)) { + /* add this dn/ndn even if it is NULL + * in fact a node belonging to no group needs to be cached + */ + dn = slapi_value_get_string(sval); + sdn = slapi_sdn_new_dn_byval(dn); + ndn = slapi_sdn_get_ndn(sdn); + + cache_entry[index].key = NULL; /* only the last element (valid=0) contains the key */ + cache_entry[index].group_dn_val = slapi_ch_strdup(dn); + cache_entry[index].group_ndn_val = slapi_ch_strdup(ndn); + cache_entry[index].valid = 1; +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: %s\n", + cache_entry[index].group_dn_val ? cache_entry[index].group_dn_val : "<empty>"); +#endif + index++; + slapi_sdn_free(&sdn); + } + + hint = slapi_valueset_next_value(groupvals, hint, &sval); + } + } + /* This is marking the end of the cache_entry */ + key = slapi_value_get_string(*member_ndn_val); + key_copy = slapi_ch_strdup(key); + cache_entry[index].key = key_copy; + cache_entry[index].group_dn_val = NULL; + cache_entry[index].group_ndn_val = NULL; + cache_entry[index].valid = 0; + + /* Cache the ancestor of member_ndn_val using the + * normalized DN key + */ +#if MEMBEROF_CACHE_DEBUG + dump_cache_entry(cache_entry, key); +#endif + if (ancestors_cache_add((const void*) key_copy, (void *) cache_entry) == NULL) { + slapi_log_err( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Failed to cache ancestor of %s\n", key); + ancestor_hashtable_entry_free(cache_entry); + slapi_ch_free ((void**)&cache_entry); + return; + } +#if MEMBEROF_CACHE_DEBUG + if (double_check = ancestors_cache_lookup((const void*) key)) { + dump_cache_entry(double_check, "read back"); + } +#endif +} +/* + * Add in v2 the values that are in v1 + * If the values are already present in v2, they are skipped + * It does not consum the values in v1 + */ +static void +merge_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *v1, memberof_get_groups_data *v2) +{ + Slapi_Value *sval_dn = 0; + Slapi_Value *sval_ndn = 0; + Slapi_Value *sval; + Slapi_DN *val_sdn = 0; + int hint = 0; + MemberOfConfig *config = ((memberof_get_groups_data*)v2)->config; + Slapi_ValueSet *v1_groupvals = *((memberof_get_groups_data*)v1)->groupvals; + Slapi_ValueSet *v2_groupvals = *((memberof_get_groups_data*)v2)->groupvals; + Slapi_ValueSet *v2_group_norm_vals = *((memberof_get_groups_data*)v2)->group_norm_vals; + int merged_cnt = 0; + + hint = slapi_valueset_first_value(v1_groupvals, &sval); + while (sval) { + if (memberof_compare(config, member_ndn_val, &sval)) { + sval_dn = slapi_value_new_string(slapi_value_get_string(sval)); + if (sval_dn) { + /* Use the normalized dn from v1 to search it + * in v2 + */ + val_sdn = slapi_sdn_new_dn_byval(slapi_value_get_string(sval_dn)); + sval_ndn = slapi_value_new_string(slapi_sdn_get_ndn(val_sdn)); + if (!slapi_valueset_find( + ((memberof_get_groups_data*) v2)->config->group_slapiattrs[0], v2_group_norm_vals, sval_ndn)) { + /* This ancestor was not already present in v2 => Add it + * Using slapi_valueset_add_value it consumes val + * so do not free sval + */ +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "merge_ancestors: add %s\n", slapi_value_get_string(sval_ndn)); +#endif + slapi_valueset_add_value_ext(v2_groupvals, sval_dn, SLAPI_VALUE_FLAG_PASSIN); + slapi_valueset_add_value_ext(v2_group_norm_vals, sval_ndn, SLAPI_VALUE_FLAG_PASSIN); + merged_cnt++; + } else { + /* This ancestor was already present, free sval_ndn/sval_dn that will not be consumed */ +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "merge_ancestors: skip (already present) %s\n", slapi_value_get_string(sval_ndn)); +#endif + slapi_value_free(&sval_dn); + slapi_value_free(&sval_ndn); + } + slapi_sdn_free(&val_sdn); + } + } + hint = slapi_valueset_next_value(v1_groupvals, hint, &sval); + } +} + int memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn, memberof_get_groups_data *data) { + Slapi_ValueSet *groupvals = slapi_valueset_new(); + Slapi_ValueSet *group_norm_vals = slapi_valueset_new(); + Slapi_Value *member_ndn_val = + slapi_value_new_string(slapi_sdn_get_ndn(member_sdn)); + int rc; + int cached = 0; + + slapi_value_set_flags(member_ndn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS); + + memberof_get_groups_data member_data = {config, member_ndn_val, &groupvals, &group_norm_vals, data->already_seen_ndn_vals, data->use_cache}; + /* Search for any grouping attributes that point to memberdn. * For each match, add it to the list, recurse and do same search */ - return memberof_call_foreach_dn(NULL, member_sdn, config, config->groupattrs, - memberof_get_groups_callback, data); +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_get_groups_r: Ancestors of %s\n", slapi_sdn_get_dn(member_sdn)); +#endif + rc = memberof_call_foreach_dn(NULL, member_sdn, config, config->groupattrs, + memberof_get_groups_callback, &member_data, &cached); + + merge_ancestors(&member_ndn_val, &member_data, data); + if (!cached && member_data.use_cache) + cache_ancestors(&member_ndn_val, &member_data); + + + slapi_value_free(&member_ndn_val); + slapi_valueset_free(groupvals); + slapi_valueset_free(group_norm_vals); + + return rc; } /* memberof_get_groups_callback() @@ -2081,8 +2408,10 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data) char *group_dn = slapi_entry_get_dn(e); Slapi_Value *group_ndn_val = 0; Slapi_Value *group_dn_val = 0; + Slapi_Value *already_seen_ndn_val = 0; Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)callback_data)->groupvals; Slapi_ValueSet *group_norm_vals = *((memberof_get_groups_data*)callback_data)->group_norm_vals; + Slapi_ValueSet *already_seen_ndn_vals = *((memberof_get_groups_data*)callback_data)->already_seen_ndn_vals; MemberOfConfig *config = ((memberof_get_groups_data*)callback_data)->config; int rc = 0; @@ -2115,6 +2444,7 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data) "memberof_get_groups_callback - Group recursion" " detected in %s\n" ,group_ndn); slapi_value_free(&group_ndn_val); + ((memberof_get_groups_data*)callback_data)->use_cache = PR_FALSE; goto bail; } @@ -2124,7 +2454,7 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data) * performed. Since all of the grouping attributes are validated to use the Dinstinguished * Name syntax, we can safely just use the first group_slapiattr. */ if (slapi_valueset_find( - ((memberof_get_groups_data*)callback_data)->config->group_slapiattrs[0], group_norm_vals, group_ndn_val)) + ((memberof_get_groups_data*)callback_data)->config->group_slapiattrs[0], already_seen_ndn_vals, group_ndn_val)) { /* we either hit a recursive grouping, or an entry is * a member of a group through multiple paths. Either @@ -2134,6 +2464,7 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data) "memberof_get_groups_callback - Possible group recursion" " detected in %s\n" ,group_ndn); slapi_value_free(&group_ndn_val); + ((memberof_get_groups_data*)callback_data)->use_cache = PR_FALSE; goto bail; } @@ -2141,12 +2472,17 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data) if (memberof_entry_in_scope(config, group_sdn)) { /* Push group_dn_val into the valueset. This memory is now owned * by the valueset. */ + slapi_valueset_add_value_ext(group_norm_vals, group_ndn_val, SLAPI_VALUE_FLAG_PASSIN); + group_dn_val = slapi_value_new_string(group_dn); slapi_valueset_add_value_ext(groupvals, group_dn_val, SLAPI_VALUE_FLAG_PASSIN); - slapi_valueset_add_value_ext(group_norm_vals, group_ndn_val, SLAPI_VALUE_FLAG_PASSIN); + + /* push this ndn to detect group recursion */ + already_seen_ndn_val = slapi_value_new_string(group_ndn); + slapi_valueset_add_value_ext(already_seen_ndn_vals, already_seen_ndn_val, SLAPI_VALUE_FLAG_PASSIN); } if(!config->skip_nested || config->fixup_task){ - /* now recurse to find parent groups of e */ + /* now recurse to find ancestors groups of e */ memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config, group_sdn, callback_data); } @@ -2239,9 +2575,10 @@ memberof_test_membership(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *group_sdn) { char *attrs[2] = {config->memberof_attr, 0}; + int cached = 0; return memberof_call_foreach_dn(pb, group_sdn, config, attrs, - memberof_test_membership_callback, config); + memberof_test_membership_callback, config, &cached); } /* @@ -2286,10 +2623,12 @@ int memberof_test_membership_callback(Slapi_Entry *e, void *callback_data) candidate_array = (Slapi_Value**) - slapi_ch_calloc(1, sizeof(Slapi_Value*)*total); + slapi_ch_malloc(sizeof(Slapi_Value*)*total); + memset(candidate_array, 0, sizeof(Slapi_Value*)*total); member_array = (Slapi_Value**) - slapi_ch_calloc(1, sizeof(Slapi_Value*)*total); + slapi_ch_malloc(sizeof(Slapi_Value*)*total); + memset(member_array, 0, sizeof(Slapi_Value*)*total); hint = slapi_attr_first_value(attr, &val); @@ -2636,10 +2975,25 @@ void memberof_lock() if (fixup_entry_hashtable) { fixup_hashtable_empty("memberof_lock"); } + if (group_ancestors_hashtable) { + ancestor_hashtable_empty("memberof_lock empty group_ancestors_hashtable"); + memset(&cache_stat, 0, sizeof(cache_stat)); + } } void memberof_unlock() { + if (group_ancestors_hashtable) { + ancestor_hashtable_empty("memberof_unlock empty group_ancestors_hashtable"); +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics: total lookup %d (success %d), add %d, remove %d, enum %d\n", + cache_stat.total_lookup, cache_stat.successfull_lookup, + cache_stat.total_add, cache_stat.total_remove, cache_stat.total_enumerate); + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics duration: lookup %ld, add %ld, remove %ld, enum %ld\n", + cache_stat.cumul_duration_lookup, cache_stat.cumul_duration_add, + cache_stat.cumul_duration_remove, cache_stat.cumul_duration_enumerate); +#endif + } if (fixup_entry_hashtable) { fixup_hashtable_empty("memberof_lock"); } @@ -2881,6 +3235,101 @@ int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *t return rc; } +static memberof_cached_value * +ancestors_cache_lookup(const char *ndn) +{ + memberof_cached_value *e; +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + long int start; + struct timespec tsnow; +#endif + + cache_stat.total_lookup++; + +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) { + start = 0; + } else { + start = tsnow.tv_nsec; + } +#endif + + e = (memberof_cached_value *) PL_HashTableLookupConst(group_ancestors_hashtable, (const void *) ndn); + +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (start) { + if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) { + cache_stat.cumul_duration_lookup += (tsnow.tv_nsec - start); + } + } +#endif + + if (e) + cache_stat.successfull_lookup++; + return e; + +} +static PRBool +ancestors_cache_remove(const char *ndn) +{ + PRBool rc; +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + long int start; + struct timespec tsnow; +#endif + + cache_stat.total_remove++; + +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) { + start = 0; + } else { + start = tsnow.tv_nsec; + } +#endif + + rc = PL_HashTableRemove(group_ancestors_hashtable, (const void *) ndn); + +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (start) { + if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) { + cache_stat.cumul_duration_remove += (tsnow.tv_nsec - start); + } + } +#endif + return rc; +} + +static PLHashEntry * +ancestors_cache_add(const void *key, void *value) +{ + PLHashEntry *e; +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + long int start; + struct timespec tsnow; +#endif + cache_stat.total_add++; + +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) { + start = 0; + } else { + start = tsnow.tv_nsec; + } +#endif + + e = PL_HashTableAdd(group_ancestors_hashtable, key, value); + +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (start) { + if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) { + cache_stat.cumul_duration_add += (tsnow.tv_nsec - start); + } + } +#endif + return e; +} + /* memberof_fix_memberof_callback() * Add initial and/or fix up broken group list in entry * @@ -2910,13 +3359,43 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data) /* Check if the entry has not already been fixed */ ndn = slapi_sdn_get_ndn(sdn); if (ndn && fixup_entry_hashtable && PL_HashTableLookupConst(fixup_entry_hashtable, (void*) ndn)) { - slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "Entry %s already fixed up\n", ndn); + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Entry %s already fixed up\n", ndn); goto bail; } /* get a list of all of the groups this user belongs to */ groups = memberof_get_groups(config, sdn); + if(config->group_filter) { + if (slapi_filter_test_simple(e, config->group_filter)) { + const char *ndn; + memberof_cached_value *ht_grp; + + /* This entry is not a group + * if (likely) we cached its ancestor it is useless + * so free this memory + */ + ndn = slapi_sdn_get_ndn(sdn); +#if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: This is NOT a group %s\n", ndn); +#endif + ht_grp = ancestors_cache_lookup((const void *) ndn); + if (ht_grp) { + if (ancestors_cache_remove((const void *) ndn)) { + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: free cached values for %s\n", ndn); + ancestor_hashtable_entry_free(ht_grp); + slapi_ch_free((void **) &ht_grp); + } else { + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Fail to remove that leaf node %s\n", ndn); + } + } else { + /* This is quite unexpected, after a call to memberof_get_groups + * ndn ancestors should be in the cache + */ + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Weird, %s is not in the cache\n", ndn); + } + } + } /* If we found some groups, replace the existing memberOf attribute * with the found values. */ if (groups && slapi_valueset_count(groups)) @@ -2959,7 +3438,7 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data) if (fixup_entry_hashtable) { dn_copy = slapi_ch_strdup(ndn); if (PL_HashTableAdd(fixup_entry_hashtable, dn_copy, dn_copy) == NULL) { - slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: " + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: " "failed to add dn (%s) in the fixup hashtable; NSPR error - %d\n", dn_copy, PR_GetError()); slapi_ch_free((void **) &dn_copy); @@ -3134,3 +3613,72 @@ static void fixup_hashtable_empty(char *msg) PL_HashTableEnumerateEntries(fixup_entry_hashtable, fixup_hashtable_remove, msg); } } + + +/* allocates the plugin hashtable + * This hash table is used by operation and is protected from + * concurrent operations with the memberof_lock (if not usetxn, memberof_lock + * is not implemented and the hash table will be not used. + * + * The hash table contains all the DN of the entries for which the memberof + * attribute has been computed/updated during the current operation + * + * hash table should be empty at the beginning and end of the plugin callback + */ + +static +void ancestor_hashtable_entry_free(memberof_cached_value *entry) +{ + int i; + for (i = 0; entry[i].valid; i++) { + slapi_ch_free((void **) &entry[i].group_dn_val); + slapi_ch_free((void **) &entry[i].group_ndn_val); + } + /* Here we are at the ending element containing the key */ + slapi_ch_free((void**) &entry[i].key); +} +/* this function called for each hash node during hash destruction */ +static PRIntn ancestor_hashtable_remove(PLHashEntry *he, PRIntn index, void *arg) +{ + memberof_cached_value *group_ancestor_array; + + if (he == NULL) + return HT_ENUMERATE_NEXT; + + + group_ancestor_array = (memberof_cached_value *) he->value; + ancestor_hashtable_entry_free(group_ancestor_array); + slapi_ch_free((void **)&group_ancestor_array); + + return HT_ENUMERATE_REMOVE; +} + +static void ancestor_hashtable_empty(char *msg) +{ +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + long int start; + struct timespec tsnow; +#endif + + if (group_ancestors_hashtable) { + cache_stat.total_enumerate++; +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) { + start = 0; + } else { + start = tsnow.tv_nsec; + } +#endif + PL_HashTableEnumerateEntries(group_ancestors_hashtable, ancestor_hashtable_remove, msg); + +#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (start) { + if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) { + cache_stat.cumul_duration_enumerate += (tsnow.tv_nsec - start); + } + } +#endif + } + +} +

1 0

ldap/servers
by thierry bordaz 11 Jan '17

11 Jan '17

ldap/servers/plugins/cos/cos_cache.c | 73 +++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 12 deletions(-) New commits: commit ac44337bd97fe63071e7d83e9dcd788f2af1feab Author: Thierry Bordaz <tbordaz(a)redhat.com> Date: Tue Jan 10 14:32:53 2017 +0100 Ticket 49079: deadlock on cos cache rebuild Bug Description: To rebuild the cache cos_cache_creation the thread gets cos definitions from backend. It means change_lock is held then cos_cache_creation will acquire some backend pages. A deadlock can happen if cos_post_op is called while backend is locked. For example if a bepreop (urp) does an internal update on a cos definition. Then the thread holds backend pages, that will be needed by cos_cache_creation, and will acquire change_lock for notification of the cos_cache thread Fix Description: Let cos cache rebuild thread run without holding change_lock. The lock prevents parallel run but a flag can do the same. https://fedorahosted.org/389/ticket/49079 Reviewed by: William Brown and Ludwig Krispenz (thanks to you both !!) Platforms tested: F23 Flag Day: no Doc impact: no diff --git a/ldap/servers/plugins/cos/cos_cache.c b/ldap/servers/plugins/cos/cos_cache.c index 5522d6e..8942254 100644 --- a/ldap/servers/plugins/cos/cos_cache.c +++ b/ldap/servers/plugins/cos/cos_cache.c @@ -111,7 +111,9 @@ void * cos_get_plugin_identity(void); /* the global plugin handle */ static volatile vattr_sp_handle *vattr_handle = NULL; +/* both variables are protected by change_lock */ static int cos_cache_notify_flag = 0; +static PRBool cos_cache_at_work = PR_FALSE; /* service definition cache structs */ @@ -199,7 +201,8 @@ typedef struct _cos_cache cosCache; static cosCache *pCache; /* always the current global cache, only use getref to get */ /* the place to start if you want a new cache */ -static int cos_cache_create(void); +static int cos_cache_create_unlock(void); +static int cos_cache_creation_lock(void); /* cache index related functions */ static int cos_cache_index_all(cosCache *pCache); @@ -386,7 +389,7 @@ static void cos_cache_wait_on_change(void *arg) pCache = 0; /* create initial cache */ - cos_cache_create(); + cos_cache_creation_lock(); slapi_lock_mutex(start_lock); started = 1; @@ -419,7 +422,7 @@ static void cos_cache_wait_on_change(void *arg) * before we go running off doing lots of stuff lets check if we should stop */ if(keeprunning) { - cos_cache_create(); + cos_cache_creation_lock(); } cos_cache_notify_flag = 0; /* Dealt with it */ }/* while */ @@ -431,22 +434,25 @@ static void cos_cache_wait_on_change(void *arg) slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "<-- cos_cache_wait_on_change thread exit\n"); } + /* - cos_cache_create + cos_cache_create_unlock --------------------- Walks the definitions in the DIT and creates the cache. Once created, it swaps the new cache for the old one, releasing its refcount to the old cache and allowing it to be destroyed. + + called while change_lock is NOT held */ -static int cos_cache_create(void) +static int cos_cache_create_unlock(void) { int ret = -1; cosCache *pNewCache; static int firstTime = 1; int cache_built = 0; - slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_create\n"); + slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_create_unlock\n"); pNewCache = (cosCache*)slapi_ch_malloc(sizeof(cosCache)); if(pNewCache) @@ -509,21 +515,21 @@ static int cos_cache_create(void) { /* we should not go on without proper schema checking */ cos_cache_release(pNewCache); - slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_create - Failed to cache the schema\n"); + slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_create_unlock - Failed to cache the schema\n"); } } else { /* currently we cannot go on without the indexes */ cos_cache_release(pNewCache); - slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_create - Failed to index cache\n"); + slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_create_unlock - Failed to index cache\n"); } } else { if(firstTime) { - slapi_log_err(SLAPI_LOG_PLUGIN, COS_PLUGIN_SUBSYSTEM, "cos_cache_create - cos disabled\n"); + slapi_log_err(SLAPI_LOG_PLUGIN, COS_PLUGIN_SUBSYSTEM, "cos_cache_create_unlock - cos disabled\n"); firstTime = 0; } @@ -531,7 +537,7 @@ static int cos_cache_create(void) } } else - slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_create - Memory allocation failure\n"); + slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_create_unlock - Memory allocation failure\n"); /* make sure we have a new cache */ @@ -563,10 +569,53 @@ static int cos_cache_create(void) } - slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "<-- cos_cache_create\n"); + slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "<-- cos_cache_create_unlock\n"); return ret; } +/* cos_cache_creation_lock is called with change_lock being hold: + * slapi_lock_mutex(change_lock) + * + * To rebuild the cache cos_cache_creation gets cos definitions from backend, that + * means change_lock is held then cos_cache_creation will acquire some backend pages. + * + * A deadlock can happen if cos_post_op is called while backend is locked. + * For example if a bepreop (urp) does an internal update on a cos definition, + * the thread holds backend pages that will be needed by cos_cache_creation. + * + * A solution is to use a flag 'cos_cache_at_work' protected by change_lock, + * release change_lock, recreate the cos_cache, acquire change_lock reset the flag. + * + * returned value: result of cos_cache_create_unlock + * + */ +static int cos_cache_creation_lock(void) +{ + int ret = -1; + int max_tries = 10; + + for (; max_tries != 0; max_tries--) { + /* if the cos_cache is already under work (cos_cache_create_unlock) + * wait 1 second + */ + if (cos_cache_at_work) { + slapi_log_err(SLAPI_LOG_FATAL, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_creation_lock already rebuilding cos_cache... retry\n"); + DS_Sleep (PR_MillisecondsToInterval(1000)); + continue; + } + cos_cache_at_work = PR_TRUE; + slapi_unlock_mutex(change_lock); + ret = cos_cache_create_unlock(); + slapi_lock_mutex(change_lock); + cos_cache_at_work = PR_FALSE; + break; + } + if (!max_tries) { + slapi_log_err(SLAPI_LOG_FATAL, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_creation_lock rebuilt was to long, skip this rebuild\n"); + } + + return ret; +} /* cos_cache_build_definition_list @@ -1648,7 +1697,7 @@ int cos_cache_getref(cos_cache **pptheCache) slapi_lock_mutex(change_lock); if(pCache == NULL) { - if(cos_cache_create()) + if(cos_cache_creation_lock()) { /* there was a problem or no COS definitions were found */ slapi_log_err(SLAPI_LOG_PLUGIN, COS_PLUGIN_SUBSYSTEM, "cos_cache_getref - No cos cache created\n");

1 0

dirsrvtests/tests
by Simon Pichugin 11 Jan '17

11 Jan '17

dirsrvtests/tests/tickets/ticket48665_test.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) New commits: commit 9feca58eb9ed7f6e9ed2a2050320529e954599a8 Author: Simon Pichugin <spichugi(a)redhat.com> Date: Tue Jan 10 13:55:30 2017 +0100 Ticket 48665 - Fix RHEL6 test compatibility issues Bug description: It is impossible to use inst.whoami_s() on RHEL6. It throws PROTOCOL_ERROR: 'unsupported extended operation'. Fix description: Use a search to verify that the server has not crushed. https://fedorahosted.org/389/ticket/48665 Reviewed by: nhosoi (Thanks!) diff --git a/dirsrvtests/tests/tickets/ticket48665_test.py b/dirsrvtests/tests/tickets/ticket48665_test.py index 818fad2..b11f75a 100644 --- a/dirsrvtests/tests/tickets/ticket48665_test.py +++ b/dirsrvtests/tests/tickets/ticket48665_test.py @@ -25,9 +25,11 @@ def test_ticket48665(topology_st): modlist) except: pass + # Check the server has not commited seppuku. - result = topology_st.standalone.whoami_s() - assert (DN_DM.lower() in result.lower()) + entries = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, '(cn=*)') + assert len(entries) > 0 + log.info('{} entries are returned from the server.'.format(len(entries))) # This has a magic hack to determine if we are in cn=config. try: @@ -37,17 +39,20 @@ def test_ticket48665(topology_st): log.fatal('Failed to change nsslapd-cachememsize ' + e.message['desc']) # Check the server has not commited seppuku. - result = topology_st.standalone.whoami_s() - assert (DN_DM.lower() in result.lower()) + entries = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, '(cn=*)') + assert len(entries) > 0 + log.info('{} entries are returned from the server.'.format(len(entries))) # Now try with mod_replace. This should be okay. modlist = [(ldap.MOD_REPLACE, 'nsslapd-cachememsize', '1')] topology_st.standalone.modify_s("cn=%s,cn=ldbm database,cn=plugins,cn=config" % DEFAULT_BENAME, modlist) + # Check the server has not commited seppuku. - result = topology_st.standalone.whoami_s() - assert (DN_DM.lower() in result.lower()) + entries = topology_st.standalone.search_s(DEFAULT_SUFFIX, ldap.SCOPE_SUBTREE, '(cn=*)') + assert len(entries) > 0 + log.info('{} entries are returned from the server.'.format(len(entries))) log.info('Test complete')

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

389-commits January 2017