Issue 6686 - VLV overlay fails to handle multiple search operations per LDAP connection
Summary: VLV overlay fails to handle multiple search operations per LDAP connection
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-27 12:32 UTC by sebastien.bahloul@gmail.com
Modified: 2014-08-01 21:04 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description sebastien.bahloul@gmail.com 2010-10-27 12:32:00 UTC
Full_Name: Sebastien Bahloul
Version: HEAD
OS: Linux RHEL 5
URL: 
Submission from: (NULL) (109.197.176.10)


Hi,

I think there is a bug / limitation inside VLV implementation : it can not
handle multiple search operation on a single connection. The first operation
succeeds and next operations fail with the following message :

LDAP: error code 51 - Other sort requests already in progress

My first guess was to remove / comment the following condition in sssvlv.c:757
:

vc->vc_context != so->so_vcontext

But I think this issue is more global. This implementation seems to be able to
only handle a single VLV context per connection. 

If I am right, this is related with the indexing method of sort_conns structure
which seems to be based only on the connection id. I suggest to implement a
double indexing array by connection id / VLV context id. 

Regards,
Comment 1 Howard Chu 2010-10-27 13:03:36 UTC
sebastien.bahloul@gmail.com wrote:
> Full_Name: Sebastien Bahloul
> Version: HEAD
> OS: Linux RHEL 5
> URL:
> Submission from: (NULL) (109.197.176.10)
>
>
> Hi,
>
> I think there is a bug / limitation inside VLV implementation : it can not
> handle multiple search operation on a single connection. The first operation
> succeeds and next operations fail with the following message :
>
> LDAP: error code 51 - Other sort requests already in progress

> But I think this issue is more global. This implementation seems to be able to
> only handle a single VLV context per connection.

Correct, that is by design.

> If I am right, this is related with the indexing method of sort_conns structure
> which seems to be based only on the connection id. I suggest to implement a
> double indexing array by connection id / VLV context id.

I have no interest in extending this. It would require much more overhead to 
protect the slapd from getting overloaded by too many such requests.

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

Comment 2 sebastien.bahloul@gmail.com 2010-10-27 13:31:05 UTC
Hi Howard,

Le mercredi 27 octobre 2010 15:03:36, Howard Chu a écrit :
> sebastien.bahloul@gmail.com wrote:
> > Full_Name: Sebastien Bahloul
> > Version: HEAD
> > OS: Linux RHEL 5
> > URL:
> > Submission from: (NULL) (109.197.176.10)
> > 
> > 
> > Hi,
> > 
> > I think there is a bug / limitation inside VLV implementation : it can
> > not handle multiple search operation on a single connection. The first
> > operation succeeds and next operations fail with the following message :
> > 
> > LDAP: error code 51 - Other sort requests already in progress
> > 
> > But I think this issue is more global. This implementation seems to be
> > able to only handle a single VLV context per connection.
> 
> Correct, that is by design.
> 
> > If I am right, this is related with the indexing method of sort_conns
> > structure which seems to be based only on the connection id. I suggest
> > to implement a double indexing array by connection id / VLV context id.
> 
> I have no interest in extending this. It would require much more overhead
> to protect the slapd from getting overloaded by too many such requests.

In fact my suggestion was a question : I would like to contribute a patch to 
add support for this way of using VLV. Is this feature extension as a double 
indexed array an acceptable implementation from your point of vue ? To avoid 
encountering such performance issues, I would add a new sssvlv parameter that 
would authorize by default only a single VLV search request per connection.

Regards,
-- 
Sebastien Bahloul
@: sebastien.bahloul@gmail.com

Comment 3 Howard Chu 2010-10-29 10:11:45 UTC
Sebastien Bahloul wrote:
> Hi Howard,
>
> Le mercredi 27 octobre 2010 15:03:36, Howard Chu a écrit :
>> sebastien.bahloul@gmail.com wrote:
>>> Full_Name: Sebastien Bahloul
>>> Version: HEAD
>>> OS: Linux RHEL 5
>>> URL:
>>> Submission from: (NULL) (109.197.176.10)
>>>
>>>
>>> Hi,
>>>
>>> I think there is a bug / limitation inside VLV implementation : it can
>>> not handle multiple search operation on a single connection. The first
>>> operation succeeds and next operations fail with the following message :
>>>
>>> LDAP: error code 51 - Other sort requests already in progress
>>>
>>> But I think this issue is more global. This implementation seems to be
>>> able to only handle a single VLV context per connection.
>>
>> Correct, that is by design.
>>
>>> If I am right, this is related with the indexing method of sort_conns
>>> structure which seems to be based only on the connection id. I suggest
>>> to implement a double indexing array by connection id / VLV context id.
>>
>> I have no interest in extending this. It would require much more overhead
>> to protect the slapd from getting overloaded by too many such requests.
>
> In fact my suggestion was a question : I would like to contribute a patch to
> add support for this way of using VLV. Is this feature extension as a double
> indexed array an acceptable implementation from your point of vue ? To avoid
> encountering such performance issues, I would add a new sssvlv parameter that
> would authorize by default only a single VLV search request per connection.

I guess as long as you observe the svi_max config parameter, there's no reason 
not to adopt your suggestion. Feel free to followup here with your patch for 
review.

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

Comment 4 Raphael Ouazana 2010-11-05 17:39:09 UTC
Hi !
Le Ven 29 octobre 2010 11:12, hyc@symas.com a écrit :
> I guess as long as you observe the svi_max config parameter, there's no
> reason
> not to adopt your suggestion. Feel free to followup here with your patch
> for
> review.

Here is the patch:
ftp://ftp.openldap.org/incoming/raphael-ouazana-101105-sssvlv-multiple-requests-per-conn.patch

It applies to HEAD of today. If you need it, I can send you a patch for
RE24 too.

The patch includes the modification of the man page.

Legal notice:
This patch file is derived from OpenLDAP Software. All of the
modifications to
OpenLDAP Software represented in this following patch were developed by
Raphael
Ouazana raphael.ouazana@linagora.com. These modifications are not subject to
any
license of Linagora.

The attached modifications to OpenLDAP Software are subject to the following
notice:
Copyright 2010 Raphael Ouazana, Linagora
Redistribution and use in source and binary forms, with or without
modification,
are permitted only as authorized by the OpenLDAP Public License.

Regards,
Raphaël Ouazana


Comment 5 Raphael Ouazana 2010-11-25 14:59:22 UTC
Hi,

Here is a new version of the patch:
ftp://ftp.openldap.org/incoming/raphael-ouazana-101125-sssvlv-multiple-requests-per-conn.patch

It fixes some segfaults that the previous patch introduced if you use
pagedResults or sss instead of VLV.

It also fixes a segfault in the original sssvlv code when making two
successive requests on the same connexion (can be triggered by
ldapsearch).

Finally here is an update of the copyright notice too:

Legal notice:
This patch file is derived from OpenLDAP Software. All of the
modifications to
OpenLDAP Software represented in this following patch were developed by
Raphael
Ouazana raphael.ouazana@linagora.com and Sebastien Bahloul
sbahloul@linagora.com. These modifications are not subject to
any
license of Linagora.

The attached modifications to OpenLDAP Software are subject to the following
notice:
Copyright 2010 Raphael Ouazana, Linagora
Copyright 2010 Sebastien Bahloul, Linagora
Redistribution and use in source and binary forms, with or without
modification,
are permitted only as authorized by the OpenLDAP Public License.

Regards,
Raphaël Ouazana






Comment 6 Howard Chu 2010-12-23 13:30:36 UTC
changed notes
changed state Open to Test
moved from Incoming to Software Enhancements
Comment 7 Quanah Gibson-Mount 2011-01-04 11:18:03 UTC
changed notes
changed state Test to Release
Comment 8 Quanah Gibson-Mount 2011-02-14 12:35:13 UTC
changed notes
changed state Release to Closed
Comment 9 OpenLDAP project 2014-08-01 21:04:54 UTC
added to HEAD
added to RE24