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

[hyc@symas.com]

Rein Tollevik wrote:
> On Wed, 30 Apr 2008, Howard Chu wrote:
>
>> rein@OpenLDAP.org wrote:
>
>>> syncprov_findbase() must search the backend saved with the syncrepl
>>> operation,
>>> not the one from the operation passed as argument.  The backend in the op
>>> argument can be a subordinate database, in which case the search for the
>>> base in
>>> the superior database will fail, and syncrepl consumers will be force to do
>>> a an
>>> unneccessary full refresh of the database.
>> OK.
>>
>>>   The patch at the end should fix
>>> this.  Note that both fop.o_bd and fop.o_bd->bd_info can be changed by the
>>> overlay_op_walk() call, which is the reason for the long pointer traversal
>>> to
>>> find the correct bd_info to save and restore.
>
>> But the overlay_op_walk call is only appropriate when the DB to be searched
>> is the current database, and the current DB is an overlay DB structure.
>
> Ah, the changing of the BackendDB->bd_info that takes place when
> overlays are called feels like an open pit I manage to fall into every
> time I get close to it...  I wish it could be replaced in a future
> version.

Agreed, it would have been safer as an Op-specific field, but that would have 
caused quite a lot of disruption to all existing backend code.

> 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.

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.)

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/