Re: (ITS#6507) backglue and paged results issue

[masarati@aero.polimi.it]

> This is a multi-part message in MIME format.
> --------------050800080103050407050909
> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
> Content-Transfer-Encoding: 8bit
>
> Le 08/04/2010 13:49, jonathan@phillipoux.net a écrit :
>> On Wed, 7 Apr 2010 00:54:46 +0200 (CEST), masarati@aero.polimi.it wrote:
>>>> Le 06/04/2010 20:55, masarati@aero.polimi.it a écrit :
>>>>> When requesting paged results control on glued databases, if a
>>>>> database
>>>>> (either
>>>>> superior or subordinate) returns one page that ends with the last
>>>>> corresponding
>>>>> entry in that database, the resulting cookie is empty.  Thus, the
>> result
>>>>> returned to the client corresponds to the final result of a paged
>>>>> response.
>>>>> However, other databases may have returned data.  A fix is being
>>>>> designed.  p.
>>>>
>>>> Your fix for this problem works well in my tests.
>>>>
>>>> Under the same subject, another problem arises:
>>>>
>>>> Consider this :
>>>> - back null
>>>> ---- back ldap 1 returning 3 results to a search
>>>> ---- back ldap 2 returning 3 results to a search
>>>>
>>>> And you search "back null" with a page size of 2:
>>>> - The first response contains 2 results from "back ldap 1" and a
>>>> correct
>>>> cookie.
>>>> - The second response contains 1 result from "back ldap 1" and 1
>>>> result
>>>> from "back ldap 2", and another correct cookie.
>>>> - The third request, however, goes back to "back ldap 1", and returns
>>>> the same results as the first response.
>>>> - And so on... forever.
>>>>
>>>> As I mentioned, it seems that to solve this problem we would have to
>>>> tie
>>>> a paged results cookie to a particular backend under glue...
>>>
>>> Should work now.  The issue was related to the fact that unless the
>>> subordinate backends are local, the response to pgaed results has not
>> been
>>> parsed into the connection structure...
>>
>> Yes, much better now.
>>
>> Just one weird case left. Considering the same setup as described above
>> (back null with 2 back ldap glued underneath): a search with
>> pagedResults
>> and a page size of 3 (the exact number of results returned by each back
>> ldap), we get:
>> - a first page with the 3 results from back ldap 1
>> - a second page with 3 results from back ldap 2 (so far, so good)
>> - but then this last result also has a pagedResults cookie, and the
>> search
>> can go on and on, returning the same 2nd page.
>>
>> Presumably this is related to the patch allowing pagedResults to cross
>> two
>> databases. If the first returns an empty cookie (no more results), a new
>> cookie is added. However, this should not be done if the last database
>> returns no more results.
>
> It seems it just needs a simple exception for the last database. The
> attached patch works for me.
>
> Jonathan
> --
> --------------------------------------------------------------
> Jonathan Clarke - jonathan@phillipoux.net
> --------------------------------------------------------------
> Ldap Synchronization Connector (LSC) - http://lsc-project.org
> --------------------------------------------------------------
>
> --------------050800080103050407050909
> Content-Type: text/x-patch;
>  name="jcl-backglue-20100408.patch"
> Content-Transfer-Encoding: 7bit
> Content-Disposition: attachment;
>  filename="jcl-backglue-20100408.patch"
>
> Index: servers/slapd/backglue.c
> ===================================================================
> RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/backglue.c,v
> retrieving revision 1.153
> diff -a -u -r1.153 backglue.c
> --- servers/slapd/backglue.c	6 Apr 2010 20:04:58 -0000	1.153
> +++ servers/slapd/backglue.c	8 Apr 2010 15:39:34 -0000
> @@ -559,6 +559,7 @@
>
>  						/* Check whether the cookie is empty,
>  						 * and give remaining databases a chance
> +						 * unless this is already the last database
>  						 */
>  						if ( op->o_bd != gi->gi_n[0].gn_be ) {
>  							int		c;
> @@ -581,7 +582,7 @@
>  								tag = ber_scanf( ber, "{im}", &size, &cookie );
>  								assert( tag != LBER_ERROR );
>
> -								if ( BER_BVISEMPTY( &cookie ) ) {
> +								if ( BER_BVISEMPTY( &cookie ) && i != 0 ) {
>  									if ( btmp == b0 ) {
>  										op->o_conn->c_pagedresults_state.ps_be = gi->gi_n[gi->gi_nodes
> - 1].gn_be;

Yes, thanks.  I already fixed it almost the same way.  p.