Re: (ITS#5114) pcache cache results for searches that hit size/timelimit
by ando＠sys-net.it
rhafer(a)suse.de wrote:
> On Wednesday 29 August 2007 18:48, ando(a)sys-net.it wrote:
>> rhafer(a)suse.de wrote:
>>> But a malicous client can then just send requests with sizelimit 1. Those
>>> query will get cached and the database is of no real use anymore (IMO).
>> Well, in this case, the proxycache should either change the sizelimit
>> (and the timelimit) to unlimited, and deal with client-requested limits
>> locally,
> This seems like the nicest approach to solve the problem. But seems to be a
> bit more effort.
I'd go this way, yes.
> My suggest for a quick fix for this issue would be to just not cache queries
> that return one off the _LIMIT_EXCEEDED error codes. Probably with checking
> if it was a client-requested limit or a limit of the server side (in which
> case we could probably cache the results).
Then please make a clear statement in the code that it's a temporary
workaround, so it is easier to spot and remove, eventually.
>> or consider uncacheable those requests that specify a time or a
>> size limit.
> Well that would cause even those queries being uncachable that would not hit
> the requested limit. If I understand you correctly.
Right. Well, I understand if no limit was hit the request would become
cacheable. In this case, the approach you suggest would be fine.
p.
Ing. Pierangelo Masarati
OpenLDAP Core Team
SysNet s.r.l.
via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
---------------------------------------
Office: +39 02 23998309
Mobile: +39 333 4963172
Email: pierangelo.masarati(a)sys-net.it
---------------------------------------
15 years, 9 months
Re: (ITS#5114) pcache cache results for searches that hit size/timelimit
by rhafer＠suse.de
On Wednesday 29 August 2007 18:48, ando(a)sys-net.it wrote:
> rhafer(a)suse.de wrote:
> > But a malicous client can then just send requests with sizelimit 1. Those
> > query will get cached and the database is of no real use anymore (IMO).
>
> Well, in this case, the proxycache should either change the sizelimit
> (and the timelimit) to unlimited, and deal with client-requested limits
> locally,
This seems like the nicest approach to solve the problem. But seems to be a
bit more effort.
My suggest for a quick fix for this issue would be to just not cache queries
that return one off the _LIMIT_EXCEEDED error codes. Probably with checking
if it was a client-requested limit or a limit of the server side (in which
case we could probably cache the results).
> or consider uncacheable those requests that specify a time or a
> size limit.
Well that would cause even those queries being uncachable that would not hit
the requested limit. If I understand you correctly.
> On the contrary, they should be considered answerable if a
> corresponding request is cached, and the limits should be checked locally.
--
Ralf
15 years, 9 months
Re: (ITS#5112) memory leak in pcache overlay
by hyc＠symas.com
Pierangelo Masarati wrote:
> hyc(a)symas.com wrote:
>
>> Ah yes, that's correct. I misspoke - the DB is written when the cache has
>> received an LDAP_RESULT from the remote server. The client gets this result
>> when the cache has finished writing. We could move the DB write to a separate
>> cleanup handler instead of running it in the response handler, which would free
>> up the client sooner.
>
> I also noticed that
> <http://www.openldap.org/lists/openldap-devel/200708/msg00033.html>, and
> I wanted to do something similar, but considering Ralf's observations,
> multiple clients concurrently running the same search request would
> cause it to be cached multiple times, because until the first one is not
> cached the others will not find it. OTOH, letting the others know it's
> being cached, and making them wait until caching is done would imply
> some delay, which could be long based on how many entries are being
> cached. However, given the purpose of a proxy cache, and considering
> that so many identical searches are not likely to occur simultaneously,
> I'd prefer to queue requests that are being cached.
Agreed. Even with a delay it ought to be faster than making the remote search
again.
But this is probably not worth changing at this moment. Let's get 2.4.5 out first.
--
-- Howard Chu
Chief Architect, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
15 years, 9 months
Re: (ITS#5112) memory leak in pcache overlay
by ando＠sys-net.it
hyc(a)symas.com wrote:
> Ah yes, that's correct. I misspoke - the DB is written when the cache has
> received an LDAP_RESULT from the remote server. The client gets this result
> when the cache has finished writing. We could move the DB write to a separate
> cleanup handler instead of running it in the response handler, which would free
> up the client sooner.
I also noticed that
<http://www.openldap.org/lists/openldap-devel/200708/msg00033.html>, and
I wanted to do something similar, but considering Ralf's observations,
multiple clients concurrently running the same search request would
cause it to be cached multiple times, because until the first one is not
cached the others will not find it. OTOH, letting the others know it's
being cached, and making them wait until caching is done would imply
some delay, which could be long based on how many entries are being
cached. However, given the purpose of a proxy cache, and considering
that so many identical searches are not likely to occur simultaneously,
I'd prefer to queue requests that are being cached.
p.
Ing. Pierangelo Masarati
OpenLDAP Core Team
SysNet s.r.l.
via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
---------------------------------------
Office: +39 02 23998309
Mobile: +39 333 4963172
Email: pierangelo.masarati(a)sys-net.it
---------------------------------------
15 years, 9 months
Re: (ITS#5112) memory leak in pcache overlay
by hyc＠symas.com
rhafer(a)suse.de wrote:
> On Wednesday 29 August 2007 18:10, Howard Chu wrote:
>> Ralf Haferkamp wrote:
>>> On Wednesday 29 August 2007 17:36, Howard Chu wrote:
>>>> rhafer(a)suse.de wrote:
>>>>> Full_Name: Ralf Haferkamp
>>>>> Version: RE23, HEAD
>>>>>
>>>>> Valgrind gives me the log pasted below when I abort an ldapsearch
>>>>> command (CTRL-C) that is running against a back-ldap database that uses
>>>>> the pcache overlay.
>>>> Makes sense. We need to use the cleanup handler and free the saved query
>>>> info if op->o_abandon is set. Do you want to code this up?
>>> I am still working on that, but checking for op->o_abandon doesn't seem
>>> to be enough. E.g. if I abort the ldapsearch while slapd is updating its
>>> cache (all entries have been returned, but the final LDAP_RESULT to the
>>> client is still missing) a filter_free in the cleanup handler will
>>> invalidly free the filter.
>>>
>>> We somehow need to make sure that the query is only free'd if nothing has
>>> been written to the cache database.
>> I don't understand the problem. Nothing can get written to the DB until the
>> LDAP_RESULT has been sent. I.e., rs->sr_result == REP_RESULT.
> Hm, but the ldapsearch command only returns after the complete query result
> has been written to the cache (at least here it behaves so), which can take
> quite a while. So it seems to me that the client has not received the
> LDAP_RESULT.
Ah yes, that's correct. I misspoke - the DB is written when the cache has
received an LDAP_RESULT from the remote server. The client gets this result
when the cache has finished writing. We could move the DB write to a separate
cleanup handler instead of running it in the response handler, which would free
up the client sooner.
--
-- Howard Chu
Chief Architect, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
15 years, 9 months
Re: (ITS#5114) pcache cache results for searches that hit size/timelimit
by ando＠sys-net.it
rhafer(a)suse.de wrote:
> But a malicous client can then just send requests with sizelimit 1. Those
> query will get cached and the database is of no real use anymore (IMO).
Well, in this case, the proxycache should either change the sizelimit
(and the timelimit) to unlimited, and deal with client-requested limits
locally, or consider uncacheable those requests that specify a time or a
size limit. On the contrary, they should be considered answerable if a
corresponding request is cached, and the limits should be checked locally.
p.
Ing. Pierangelo Masarati
OpenLDAP Core Team
SysNet s.r.l.
via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
---------------------------------------
Office: +39 02 23998309
Mobile: +39 333 4963172
Email: pierangelo.masarati(a)sys-net.it
---------------------------------------
15 years, 9 months
Re: (ITS#5114) pcache cache results for searches that hit size/timelimit
by rhafer＠suse.de
On Wednesday 29 August 2007 18:36, Pierangelo Masarati wrote:
> hyc(a)symas.com wrote:
> >> pcache currently cache the results of search that hit server- or
> >> clientside size- or timelimits. That means that subsequent search will
> >> get the (incomplete) results from the cache. I guess pcache should only
> >> cache operations that returned LDAP_SUCCESS.
> >
> > Makes sense...
>
> Well, I agree for timelimit, but sizelimit might be questionable. In
> fact, the only reason not to cache searches ending in sizelimit exceeded
> is that the size limit may depend on the client's identity. But this is
> true in general also for access to entries and to entry data, but we
> don't cache based on the identity of the client, so data cached with one
> identity (set A) might differ from data that would be returned by
> another identity for the very same search (set B), and both the relative
> complement of A in B and of B in A may be not empty. So, if we accept
> this for ACLs (no differences between the results returned for requests
> with different identities) I don't see why we should differentiate with
> respect to the size limit.
But a malicous client can then just send requests with sizelimit 1. Those
query will get cached and the database is of no real use anymore (IMO).
--
Ralf
15 years, 9 months
Re: (ITS#5114) pcache cache results for searches that hit size/timelimit
by ando＠sys-net.it
hyc(a)symas.com wrote:
>> pcache currently cache the results of search that hit server- or clientside
>> size- or timelimits. That means that subsequent search will get the (incomplete)
>> results from the cache. I guess pcache should only cache operations that
>> returned LDAP_SUCCESS.
>
> Makes sense...
Well, I agree for timelimit, but sizelimit might be questionable. In
fact, the only reason not to cache searches ending in sizelimit exceeded
is that the size limit may depend on the client's identity. But this is
true in general also for access to entries and to entry data, but we
don't cache based on the identity of the client, so data cached with one
identity (set A) might differ from data that would be returned by
another identity for the very same search (set B), and both the relative
complement of A in B and of B in A may be not empty. So, if we accept
this for ACLs (no differences between the results returned for requests
with different identities) I don't see why we should differentiate with
respect to the size limit.
p.
Ing. Pierangelo Masarati
OpenLDAP Core Team
SysNet s.r.l.
via Dossi, 8 - 27100 Pavia - ITALIA
http://www.sys-net.it
---------------------------------------
Office: +39 02 23998309
Mobile: +39 333 4963172
Email: pierangelo.masarati(a)sys-net.it
---------------------------------------
15 years, 9 months
Re: (ITS#5112) memory leak in pcache overlay
by rhafer＠suse.de
On Wednesday 29 August 2007 18:10, Howard Chu wrote:
> Ralf Haferkamp wrote:
> > On Wednesday 29 August 2007 17:36, Howard Chu wrote:
> >> rhafer(a)suse.de wrote:
> >>> Full_Name: Ralf Haferkamp
> >>> Version: RE23, HEAD
> >>>
> >>> Valgrind gives me the log pasted below when I abort an ldapsearch
> >>> command (CTRL-C) that is running against a back-ldap database that uses
> >>> the pcache overlay.
> >>
> >> Makes sense. We need to use the cleanup handler and free the saved query
> >> info if op->o_abandon is set. Do you want to code this up?
> >
> > I am still working on that, but checking for op->o_abandon doesn't seem
> > to be enough. E.g. if I abort the ldapsearch while slapd is updating its
> > cache (all entries have been returned, but the final LDAP_RESULT to the
> > client is still missing) a filter_free in the cleanup handler will
> > invalidly free the filter.
> >
> > We somehow need to make sure that the query is only free'd if nothing has
> > been written to the cache database.
>
> I don't understand the problem. Nothing can get written to the DB until the
> LDAP_RESULT has been sent. I.e., rs->sr_result == REP_RESULT.
Hm, but the ldapsearch command only returns after the complete query result
has been written to the cache (at least here it behaves so), which can take
quite a while. So it seems to me that the client has not received the
LDAP_RESULT.
With the suggested fix I get this, if I abort the ldapsearch during that time,
as the op->o_abandon is set in cleanup handler:
==26949== Invalid free() / delete / delete[]
==26949== at 0x4C226DB: free
(in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==26949== by 0x508A819: ber_memfree_x (memory.c:152)
==26949== by 0x4AC9D7: slap_sl_free (sl_malloc.c:455)
==26949== by 0x44797D: filter_free_x (filter.c:526)
==26949== by 0x4478C9: filter_free_x (filter.c:509)
==26949== by 0x4479D9: filter_free (filter.c:538)
==26949== by 0x5A7794: free_query (pcache.c:1126)
==26949== by 0x5AF9B0: pcache_db_close (pcache.c:3282)
==26949== by 0x4C2F87: over_db_close (backover.c:192)
==26949== by 0x452658: backend_shutdown (backend.c:365)
==26949== by 0x47C03E: slap_shutdown (init.c:253)
==26949== by 0x422D65: main (main.c:943)
==26949== Address 0xA533A68 is 0 bytes inside a block of size 24 free'd
==26949== at 0x4C226DB: free
(in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==26949== by 0x508A819: ber_memfree_x (memory.c:152)
==26949== by 0x4AC9D7: slap_sl_free (sl_malloc.c:455)
==26949== by 0x44797D: filter_free_x (filter.c:526)
==26949== by 0x4478C9: filter_free_x (filter.c:509)
==26949== by 0x4479D9: filter_free (filter.c:538)
==26949== by 0x5A9F7F: pcache_op_cleanup (pcache.c:1886)
==26949== by 0x456A69: slap_cleanup_play (result.c:344)
==26949== by 0x4571DC: send_ldap_response (result.c:526)
==26949== by 0x457935: slap_send_ldap_result (result.c:632)
==26949== by 0x4F7AE4: ldap_back_search (search.c:528)
==26949== by 0x4C3FD0: overlay_op_walk (backover.c:652)
--
Ralf
15 years, 9 months