[Date Prev][Date Next] [Chronological] [Thread] [Top]
Re: (ITS#5488) syncrepl received contextCSN not passed on to syncprov consumers

To: openldap-its@OpenLDAP.org
Subject: Re: (ITS#5488) syncrepl received contextCSN not passed on to syncprov consumers
From: hyc@symas.com
Date: Wed, 30 Apr 2008 22:54:24 GMT
rein@OpenLDAP.org wrote:
> Full_Name: Rein Tollevik
> Version: CVS head
> OS:
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (81.93.160.250)
> Submitted by: rein
>
>
> When syncrepl and syncprov are both used on a glue database, the
> contextCSN received from the syncrepl producers are not passed on to the
> syncprov consumers when changes in subordinate databases are received.
> The reason is that syncrepl queues the CSNs in the glue backend, while
> syncprov fetches them from the backend where the changes are made.  As a
> consequence, the consumers will be passed a cookie without any csn
> value.
>
> My first attempt at fixing this was to change syncprov to fetch the
> queued csn values from the glue backend where it was used.  But that
> failed as other modules queues the csn values in their own backend when
> they changes things.

What other modules? Generally there cannot be any other sources of changes.

> Instead I changed ctxcsn.c so that it always
> queues them in the glue backend where syncprov is used.  But I don't
> feel that my understanding of this stuff is good enough to be sure that
> this is the optimal solution..

I definitely don't like references to the syncprov overlay appearing in main 
slapd code like that. We need a different solution.

At one point in the past, I had changed syncrepl.c to queue the CSNs in both 
places, but that seemed rather sloppy. Still, it may work best here.

> Btw, in syncprov_checkpoint() there is a similar SLAP_GLUE_SUBORDINATE
> test, should that have included an overlay_is_inst() clause as well?

Perhaps. You would have to use op->o_bd->bd_self instead of op->o_bd on that call.

> Rein Tollevik
> Basefarm AS
>
> Index: OpenLDAP/servers/slapd/ctxcsn.c
> diff -u OpenLDAP/servers/slapd/ctxcsn.c:1.1.1.11
> OpenLDAP/servers/slapd/ctxcsn.c:1.2
> --- OpenLDAP/servers/slapd/ctxcsn.c:1.1.1.11	Sat Mar 22 16:47:49 2008
> +++ OpenLDAP/servers/slapd/ctxcsn.c	Wed Apr 30 19:44:25 2008
> @@ -37,67 +37,78 @@
>   )
>   {
>   	struct slap_csn_entry *csne, *committed_csne = NULL;
> +	Backend *be = op->o_bd;
>
>   	if ( maxcsn ) {
>   		BER_BVZERO( maxcsn );
>   	}
>
> -	ldap_pvt_thread_mutex_lock( op->o_bd->be_pcl_mutexp );
> +	if ( SLAP_GLUE_SUBORDINATE( be )&&  !overlay_is_inst( be, "syncprov" ))
> +		be = select_backend(&be->be_nsuffix[0], 1 );
> +	ldap_pvt_thread_mutex_lock( be->be_pcl_mutexp );
>
> -	LDAP_TAILQ_FOREACH( csne, op->o_bd->be_pending_csn_list, ce_csn_link ) {
> +	LDAP_TAILQ_FOREACH( csne, be->be_pending_csn_list, ce_csn_link ) {
>   		if ( csne->ce_opid == op->o_opid&&  csne->ce_connid == op->o_connid ) {
>   			csne->ce_state = SLAP_CSN_COMMIT;
>   			break;
>   		}
>   	}
>
> -	LDAP_TAILQ_FOREACH( csne, op->o_bd->be_pending_csn_list, ce_csn_link ) {
> +	LDAP_TAILQ_FOREACH( csne, be->be_pending_csn_list, ce_csn_link ) {
>   		if ( csne->ce_state == SLAP_CSN_COMMIT ) committed_csne = csne;
>   		if ( csne->ce_state == SLAP_CSN_PENDING ) break;
>   	}
>
>   	if ( committed_csne&&  maxcsn ) *maxcsn = committed_csne->ce_csn;
> -	ldap_pvt_thread_mutex_unlock( op->o_bd->be_pcl_mutexp );
> +	ldap_pvt_thread_mutex_unlock( be->be_pcl_mutexp );
>   }
>
>   void
>   slap_rewind_commit_csn( Operation *op )
>   {
>   	struct slap_csn_entry *csne;
> +	Backend *be = op->o_bd;
>
> -	ldap_pvt_thread_mutex_lock( op->o_bd->be_pcl_mutexp );
> +	if ( SLAP_GLUE_SUBORDINATE( be )&&  !overlay_is_inst( be, "syncprov" ))
> +		be = select_backend(&be->be_nsuffix[0], 1 );
> +	ldap_pvt_thread_mutex_lock( be->be_pcl_mutexp );
>
> -	LDAP_TAILQ_FOREACH( csne, op->o_bd->be_pending_csn_list, ce_csn_link ) {
> +	LDAP_TAILQ_FOREACH( csne, be->be_pending_csn_list, ce_csn_link ) {
>   		if ( csne->ce_opid == op->o_opid&&  csne->ce_connid == op->o_connid ) {
>   			csne->ce_state = SLAP_CSN_PENDING;
>   			break;
>   		}
>   	}
>
> -	ldap_pvt_thread_mutex_unlock( op->o_bd->be_pcl_mutexp );
> +	ldap_pvt_thread_mutex_unlock( be->be_pcl_mutexp );
>   }
>
>   void
>   slap_graduate_commit_csn( Operation *op )
>   {
>   	struct slap_csn_entry *csne;
> +	Backend *be;
>
>   	if ( op == NULL ) return;
>   	if ( op->o_bd == NULL ) return;
>
> +	be = op->o_bd;
> +	if ( SLAP_GLUE_SUBORDINATE( be )&&  !overlay_is_inst( be, "syncprov" ))
> +		be = select_backend(&be->be_nsuffix[0], 1 );
> +
>   #if 0
>   	/* it is NULL when we get here from the frontendDB;
>   	 * alternate fix: initialize frontendDB like all other backends */
> -	assert( op->o_bd->be_pcl_mutexp != NULL );
> +	assert( be->be_pcl_mutexp != NULL );
>   #endif
>   	
> -	if ( op->o_bd->be_pcl_mutexp == NULL ) return;
> +	if ( be->be_pcl_mutexp == NULL ) return;
>
> -	ldap_pvt_thread_mutex_lock( op->o_bd->be_pcl_mutexp );
> +	ldap_pvt_thread_mutex_lock( be->be_pcl_mutexp );
>
> -	LDAP_TAILQ_FOREACH( csne, op->o_bd->be_pending_csn_list, ce_csn_link ) {
> +	LDAP_TAILQ_FOREACH( csne, be->be_pending_csn_list, ce_csn_link ) {
>   		if ( csne->ce_opid == op->o_opid&&  csne->ce_connid == op->o_connid ) {
> -			LDAP_TAILQ_REMOVE( op->o_bd->be_pending_csn_list,
> +			LDAP_TAILQ_REMOVE( be->be_pending_csn_list,
>   				csne, ce_csn_link );
>   			Debug( LDAP_DEBUG_SYNC, "slap_graduate_commit_csn: removing %p %s\n",
>   				csne->ce_csn.bv_val, csne->ce_csn.bv_val, 0 );
> @@ -110,7 +121,7 @@
>   		}
>   	}
>
> -	ldap_pvt_thread_mutex_unlock( op->o_bd->be_pcl_mutexp );
> +	ldap_pvt_thread_mutex_unlock( be->be_pcl_mutexp );
>
>   	return;
>   }
> @@ -161,22 +172,25 @@
>   	struct berval *csn )
>   {
>   	struct slap_csn_entry *pending;
> +	Backend *be = op->o_bd;
>
>   	pending = (struct slap_csn_entry *) ch_calloc( 1,
>   			sizeof( struct slap_csn_entry ));
>
>   	Debug( LDAP_DEBUG_SYNC, "slap_queue_csn: queing %p %s\n", csn->bv_val,
> csn->bv_val, 0 );
>
> -	ldap_pvt_thread_mutex_lock( op->o_bd->be_pcl_mutexp );
> +	if ( SLAP_GLUE_SUBORDINATE( be )&&  !overlay_is_inst( be, "syncprov" ))
> +		be = select_backend(&be->be_nsuffix[0], 1 );
> +	ldap_pvt_thread_mutex_lock( be->be_pcl_mutexp );
>
>   	ber_dupbv(&pending->ce_csn, csn );
>   	ber_bvreplace_x(&op->o_csn,&pending->ce_csn, op->o_tmpmemctx );
>   	pending->ce_connid = op->o_connid;
>   	pending->ce_opid = op->o_opid;
>   	pending->ce_state = SLAP_CSN_PENDING;
> -	LDAP_TAILQ_INSERT_TAIL( op->o_bd->be_pending_csn_list,
> +	LDAP_TAILQ_INSERT_TAIL( be->be_pending_csn_list,
>   		pending, ce_csn_link );
> -	ldap_pvt_thread_mutex_unlock( op->o_bd->be_pcl_mutexp );
> +	ldap_pvt_thread_mutex_unlock( be->be_pcl_mutexp );
>   }
>
>   int
>
>
>


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/
Prev by Date: (ITS#5491) slapd crashes with ldappasswd
Index(es):
- Chronological
- Thread