Re: (ITS#6716) syncprov sessionlog 'sl_mincsn' not assigned a value.

[hyc@symas.com]

Chris Mikkelson wrote:
> On Thu, Jan 13, 2011 at 12:30:32PM -0800, Howard Chu wrote:
>> cmikk@qwest.net wrote:
>>> Other than the patch I suggested (using the list head's
>>> CSN value directly), I can think of two other approaches:
>>>
>>> 	1) When adding an entry to the sessionlog, check
>>> 	if sl_mincsn is empty. If it is, update sl_mincsn
>>> 	to the new entry's CSN.
>>>
>>> 	2) When initializing the sessionlog, set sl_mincsn
>>> 	to the maximum contextCSN value of the underlying
>>> 	database.
>>>
>>> #2 seems ideal from an efficiency standpoint, although it
>>> differs from the algorithm the current code appears to be
>>> intended to implement.
>>
>> We should have gone with #2. The problem scenario:
>
> The current code (in HEAD) covers the vast majority of
> cases, but still has a hole which can come up quite often
> in MMR + refreshOnly scenarios. The scenario:
>
>    1: provider 1, provider 2, and consumer all in sync. SID 1 CSN
>       is a few minutes old, SID 2 is a few hours old. Both sessionlogs
>       are empty with sl_mincsn set to the newest (provider 1) CSN value
>    2: stop consumer
>    3: write on provider 2
>    4: start consumer
>
> The provider will see that the consumer is out of date
> wrt the SID 2 CSN and that the old value of the SID 2 CSN
> predates sl_mincsn, so it will skip the session log.
>
> I think the following approach will cover this case:
>
> 	* sl_mincsn holds a CSN value for each SID

Yeah, looks like we were headed there anyway.

> 	* in syncprov_op_search, mincsn is compared to
> 	the sl_mincsn value with the same SID

The mincsn variable is no longer useful as-is. We need to compare the vector 
of CSNs from the cookie with the vector of CSNs in the provider. If any CSN in 
the consumer is older than the matching sl_mincsn then the log must be skipped.

At this point there are a bunch of redundant O(N^2) walkthrus of the CSNs 
anyway, so getting rid of mincsn would be a good simplification. Since there 
may be many SIDs it would be better to sort both vectors and then walk thru 
them sequentially.

> 	* when expiring log entries in in syncprov_add_slog,
> 	the sl_mincsn value with the same SID as the expired
> 	entry's CSN is set to the maximum of the two values.

Yes.

> If there is no matching SID in sl_mincsn, then:
>
> 	* in syncprov_op_search, this implies that the
> 	mincsn's SID has been added to the provider's
> 	configuration since startup or that the first
> 	change recorded by that SID occurred since startup,
> 	and thus the sessionlog contains sufficient
> 	information
>
> 	* in syncprov_add_slog, sl_mincsn will need to
> 	be expanded to hold the new SID's CSN
>
> Thoughts?

Sounds right.

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