Re: (ITS#5487) syncprov_findbase must search the backend from the syncrepl search

[rein@OpenLDAP.org]

On Sun, 4 May 2008, hyc@symas.com wrote:

> Rein Tollevik wrote:

>> A new patch that I hope should fix this is at the end.  It always use
>> be_search, after putting back the original bd_info if needed.  I feel
>> that using the generic be_search is better than interfering directly
>> with the overlay code as overlay_op_walk does.  I also tested for
>> SLAP_ISOVERLAY rather than PS_IS_REFRESHING, as that appeared more
>> generic to me.  But again, I may be totally wrong here.  Does this
>> patch look better?
>
> SLAP_IS_OVERLAY will never be true here. That flag is only set when the
> BackendDB being tested is a local copy of a real BackendDB structure. The
> structure referenced in s_op is always a real BackendDB.

No, it is true when syncprov_findbase is called from syncprov_op_search,
as s_op is the op passed to that function.  The new patch at the end uses
o_bd->bd_self, which should eliminate this test since copying the db is
not necessary.

> In fact, if you're always going to use s_op and be_search, there's no
> further work needed, because the regular overlay infrastructure will
> always make a new local BackendDB copy itself. (And of course, some of
> that would be wasted effort, which is why the original code uses
> overlay_op_walk. Since op->o_bd is already an overlay DB, there's no
> need to make yet another copy for the first-search case.)

I understand, but I still prefer the clarity of always using be_search.
After all, this function is only called the first time a consumer is
initialized or after an update of the search base entry.  I.e the few
extra cycles shouldn't be measurable, even when the search base is the
glue suffix where updates of the contextCSN causes the entry to be
written more often than most other entries.

Rein

Index: OpenLDAP/servers/slapd/overlays/syncprov.c
===================================================================
RCS file: /f/CVSROOT/drift/OpenLDAP/servers/slapd/overlays/syncprov.c,v
retrieving revision 1.1.1.18
retrieving revision 1.20
diff -u -u -r1.1.1.18 -r1.20
--- OpenLDAP/servers/slapd/overlays/syncprov.c	30 Apr 2008 11:17:58 -0000	1.1.1.18
+++ OpenLDAP/servers/slapd/overlays/syncprov.c	5 May 2008 17:45:38 -0000	1.20
@@ -404,7 +404,6 @@
  		slap_callback cb = {0};
  		Operation fop;
  		SlapReply frs = { REP_RESULT };
-		BackendInfo *bi;
  		int rc;

  		fc->fss->s_flags ^= PS_FIND_BASE;
@@ -412,11 +411,10 @@

  		fop = *fc->fss->s_op;

+		fop.o_bd = fop.o_bd->bd_self;
  		fop.o_hdr = op->o_hdr;
-		fop.o_bd = op->o_bd;
  		fop.o_time = op->o_time;
  		fop.o_tincr = op->o_tincr;
-		bi = op->o_bd->bd_info;

  		cb.sc_response = findbase_cb;
  		cb.sc_private = fc;
@@ -434,8 +432,7 @@
  		fop.ors_filter = &generic_filter;
  		fop.ors_filterstr = generic_filterstr;

-		rc = overlay_op_walk( &fop, &frs, op_search, on->on_info, on );
-		op->o_bd->bd_info = bi;
+		rc = fop.o_bd->be_search( &fop, &frs );
  	} else {
  		ldap_pvt_thread_mutex_unlock( &fc->fss->s_mutex );
  		fc->fbase = 1;