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.