You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2018/10/16 12:28:06 UTC
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Gentle ping :-). Did you find some time to have a look?
Regards
Rüdiger
On 09/25/2018 06:37 PM, Greg Stein wrote:
> We learned a lot about pool handling while writing Subversion, after I wrote that mod_dav code. There are definite some
> improvements to be made. I'm not surprised that a propfind can go nuts like that ... I'll review the change and take a
> look generally.
>
> h/t to DanielR for the pointer to this thread.
>
>
> On Tue, Sep 18, 2018 at 8:04 AM Ruediger Pluem <rpluem@apache.org <ma...@apache.org>> wrote:
>
> Pools are very tricky in mod_dav. Hence additional eyeballs are very much welcome here.
> As I only did testing with mod_dav_fs I would be keen to know if things still work with Subversion.
> So if someone from the Subversion guys is listening here: Having this tested with Subversion would be very welcome :-).
>
> Regards
>
> Rüdiger
>
> On 09/18/2018 02:58 PM, rpluem@apache.org <ma...@apache.org> wrote:
> > Author: rpluem
> > Date: Tue Sep 18 12:58:57 2018
> > New Revision: 1841225
> >
> > URL: http://svn.apache.org/viewvc?rev=1841225&view=rev
> > Log:
> > * Doing a PROPFIND on a large collection e.g. 50.000 elements can easily
> > consume 1 GB of memory as the subrequests and propdb pools are not
> > destroyed and cleared after each element was handled.
> > Do this now. There is one case in dav_get_props where elem->priv
> > lives longer then the propdb pool. In this case allocate from r->pool.
> > Furthermore also recycle propdb's which allows to clear the propdb's
> > pools instead of destroying them and creating them again.
> >
> > Modified:
> > httpd/httpd/trunk/modules/dav/main/props.c
> >
> > Modified: httpd/httpd/trunk/modules/dav/main/props.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/dav/main/props.c?rev=1841225&r1=1841224&r2=1841225&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/dav/main/props.c (original)
> > +++ httpd/httpd/trunk/modules/dav/main/props.c Tue Sep 18 12:58:57 2018
> > @@ -524,7 +524,21 @@ DAV_DECLARE(dav_error *)dav_open_propdb(
> > apr_array_header_t * ns_xlate,
> > dav_propdb **p_propdb)
> > {
> > - dav_propdb *propdb = apr_pcalloc(r->pool, sizeof(*propdb));
> > + dav_propdb *propdb = NULL;
> > + /*
> > + * Check if we have tucked away a previous propdb and reuse it.
> > + * Otherwise create a new one and tuck it away
> > + */
> > + apr_pool_userdata_get((void **)&propdb, "propdb", r->pool);
> > + if (!propdb) {
> > + propdb = apr_pcalloc(r->pool, sizeof(*propdb));
> > + apr_pool_userdata_setn(propdb, "propdb", NULL, r->pool);
> > + apr_pool_create(&propdb->p, r->pool);
> > + }
> > + else {
> > + /* Play safe and clear the pool of the reused probdb */
> > + apr_pool_clear(propdb->p);
> > + }
> >
> > *p_propdb = NULL;
> >
> > @@ -537,7 +551,6 @@ DAV_DECLARE(dav_error *)dav_open_propdb(
> > #endif
> >
> > propdb->r = r;
> > - apr_pool_create(&propdb->p, r->pool);
> > propdb->resource = resource;
> > propdb->ns_xlate = ns_xlate;
> >
> > @@ -562,10 +575,12 @@ DAV_DECLARE(void) dav_close_propdb(dav_p
> > (*propdb->db_hooks->close)(propdb->db);
> > }
> >
> > - /* Currently, mod_dav's pool usage doesn't allow clearing this pool. */
> > -#if 0
> > - apr_pool_destroy(propdb->p);
> > -#endif
> > + if (propdb->subreq) {
> > + ap_destroy_sub_req(propdb->subreq);
> > + propdb->subreq = NULL;
> > + }
> > +
> > + apr_pool_clear(propdb->p);
> > }
> >
> > DAV_DECLARE(dav_get_props_result) dav_get_allprops(dav_propdb *propdb,
> > @@ -815,7 +830,8 @@ DAV_DECLARE(dav_get_props_result) dav_ge
> > */
> >
> > if (elem->priv == NULL) {
> > - elem->priv = apr_pcalloc(propdb->p, sizeof(*priv));
> > + /* elem->priv outlives propdb->p. Hence use the request pool */
> > + elem->priv = apr_pcalloc(propdb->r->pool, sizeof(*priv));
> > }
> > priv = elem->priv;
> >
> >
> >
> >
>
AW: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
C2 General
> -----Ursprüngliche Nachricht-----
> Von: Joe Orton <jo...@redhat.com>
> Gesendet: Dienstag, 25. Juni 2019 13:59
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1841225 -
> /httpd/httpd/trunk/modules/dav/main/props.c
>
> On Tue, Jun 25, 2019 at 09:06:42AM +0000, Plüm, Rüdiger, Vodafone Group
> wrote:
> > > On Tue, Jun 25, 2019 at 08:49:08AM +0100, Joe Orton wrote:
> > > > Unless I am missing something the apr_dir_open/apr_dir_read API
> design
> > > > is always going to have memory use proportional to directory size
> > > > because apr_dir_read() duplicates the filename into the
> directory's
> > > > pool. We need an apr_dir_pread() or whatever which takes a pool
> > > > argument. :(
> > >
> > > OK, yes, this plus an iterpool in dav_fs_walker() fixes it.
> >
> > Only the iterpool in dav_fs_walker or plus the changes in APR
> (apr_dir_pread)?
>
> Yes, both - PoC attached but I'm not at all confident this is safe, esp
> in the recursion case.
>
> The test I was running before was only a "core" liveprop provided by
> mod_dav itself, so I also tested the deadprops case and that still seems
> to leak. sbrk() to increase the heap is getting triggered by
> apr_sdbm_open() every time I saw it, but apr_dbm_open() is being pass
> the scratchpool so I think it's not the cause.
I would agree, but can you please provide the stacktrace up to sbrk?
That would ease it for me to dive in :-).
>
> Running "dump_all_pools propdb->p" at this point gives me:
>
> Pool 'transaction' [0x24e1d88]: 7264/8192 free (1 blocks) allocator:
> 0x24d2b90 free blocks in allocator: 0 kiB
> Pool 'master_conn' [0x2504e28]: 1224/8192 free (1 blocks)
> allocator: 0x24d2b90 free blocks in allocator: 0 kiB
> Pool 'deferred_pool' [0x263ef18]: 8032/8192 free (1 blocks)
> allocator: 0x24d2b90 free blocks in allocator: 0 kiB
> Pool 'request' [0x2506e38]: 13672/1646592 free (201 blocks)
> allocator: 0x24d2b90 free blocks in allocator: 0 kiB
> Pool 'mod_dav_fs-walker' [0x2524648]: 8016/8192 free (1
> blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
> Pool 'mod_dav-scratch' [0x2508e48]: 7224/8192 free (1 blocks)
> allocator: 0x24d2b90 free blocks in allocator: 0 kiB
> Pool 'mod_lua-vm' [0x250ce68]: 6360/16384 free (2 blocks)
> allocator: 0x24d2b90 free blocks in allocator: 0 kiB
>
> [trimmed some whitespace only]
>
> I think this is implicating misuse of r->pool, that's grown rather
> large?
>
I would think so as well.
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Jun 25, 2019 at 09:06:42AM +0000, Plüm, Rüdiger, Vodafone Group wrote:
> > On Tue, Jun 25, 2019 at 08:49:08AM +0100, Joe Orton wrote:
> > > Unless I am missing something the apr_dir_open/apr_dir_read API design
> > > is always going to have memory use proportional to directory size
> > > because apr_dir_read() duplicates the filename into the directory's
> > > pool. We need an apr_dir_pread() or whatever which takes a pool
> > > argument. :(
> >
> > OK, yes, this plus an iterpool in dav_fs_walker() fixes it.
>
> Only the iterpool in dav_fs_walker or plus the changes in APR (apr_dir_pread)?
Yes, both - PoC attached but I'm not at all confident this is safe, esp
in the recursion case.
The test I was running before was only a "core" liveprop provided by
mod_dav itself, so I also tested the deadprops case and that still seems
to leak. sbrk() to increase the heap is getting triggered by
apr_sdbm_open() every time I saw it, but apr_dbm_open() is being pass
the scratchpool so I think it's not the cause.
Running "dump_all_pools propdb->p" at this point gives me:
Pool 'transaction' [0x24e1d88]: 7264/8192 free (1 blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
Pool 'master_conn' [0x2504e28]: 1224/8192 free (1 blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
Pool 'deferred_pool' [0x263ef18]: 8032/8192 free (1 blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
Pool 'request' [0x2506e38]: 13672/1646592 free (201 blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
Pool 'mod_dav_fs-walker' [0x2524648]: 8016/8192 free (1 blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
Pool 'mod_dav-scratch' [0x2508e48]: 7224/8192 free (1 blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
Pool 'mod_lua-vm' [0x250ce68]: 6360/16384 free (2 blocks) allocator: 0x24d2b90 free blocks in allocator: 0 kiB
[trimmed some whitespace only]
I think this is implicating misuse of r->pool, that's grown rather
large?
Regards, Joe
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Stefan Eissing <st...@greenbytes.de>.
Nice work!
> Am 25.06.2019 um 10:44 schrieb Joe Orton <jo...@redhat.com>:
>
> <ps-trunk.png>
AW: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
C2 General
> -----Ursprüngliche Nachricht-----
> Von: Joe Orton <jo...@redhat.com>
> Gesendet: Dienstag, 25. Juni 2019 10:45
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1841225 -
> /httpd/httpd/trunk/modules/dav/main/props.c
>
> On Tue, Jun 25, 2019 at 08:49:08AM +0100, Joe Orton wrote:
> > Unless I am missing something the apr_dir_open/apr_dir_read API design
> > is always going to have memory use proportional to directory size
> > because apr_dir_read() duplicates the filename into the directory's
> > pool. We need an apr_dir_pread() or whatever which takes a pool
> > argument. :(
>
> OK, yes, this plus an iterpool in dav_fs_walker() fixes it.
Only the iterpool in dav_fs_walker or plus the changes in APR (apr_dir_pread)?
>
> I've been using the python "psrecord" (from pip) to check this rather
> than my traditional method of "eyeballing top", here are the charts for
> running 10x a PROPFIND with ~100K files, for
Great plots. Thanks for pointing to the tool.
>
> a) trunk - steep gradient, ~20mb consumed
> b) trunk plus the patch from my previous post, ~5mb?
> c) (b) plus iterpool in dav_fs_walker - flat
>
> so I think (b) is fine to commit but with the recursion in dav_fs_walker
> I am not at all sure this stuff is safe yet, will need some more
> testing.
The recursion stuff could be tricky. Probably a separate (sub-)pool for apr_dir_open
on each recursion level and separate iterpools on each recursion level?
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Jun 25, 2019 at 08:49:08AM +0100, Joe Orton wrote:
> Unless I am missing something the apr_dir_open/apr_dir_read API design
> is always going to have memory use proportional to directory size
> because apr_dir_read() duplicates the filename into the directory's
> pool. We need an apr_dir_pread() or whatever which takes a pool
> argument. :(
OK, yes, this plus an iterpool in dav_fs_walker() fixes it.
I've been using the python "psrecord" (from pip) to check this rather
than my traditional method of "eyeballing top", here are the charts for
running 10x a PROPFIND with ~100K files, for
a) trunk - steep gradient, ~20mb consumed
b) trunk plus the patch from my previous post, ~5mb?
c) (b) plus iterpool in dav_fs_walker - flat
so I think (b) is fine to commit but with the recursion in dav_fs_walker
I am not at all sure this stuff is safe yet, will need some more
testing.
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/24/2019 07:04 PM, Joe Orton wrote:
> On Fri, Feb 08, 2019 at 08:07:57AM +0100, Ruediger Pluem wrote:
>> On 11/13/2018 10:26 AM, Joe Orton wrote:
>>> On Mon, Nov 12, 2018 at 08:26:48AM +0100, Ruediger Pluem wrote:
>>>> The discussion died a little bit, because of the other issue (frequent
>>>> writeev calls). I know that the liveprops issue is not fixed yet, but
>>>> I guess it makes sense if you commit the patch you posted here
>>>> already.
>>>
>>> Sorry, I was looking at this again and the repro case I thought worked
>>> was still showing leaks, so I got stuck. Will come back to it hopefully
>>> later this week.
>>>
>>
>> Going through the STATUS of 2.4.x I got aware that this died a little bit again.
>> Any new ideas in the meantime?
>
> I spent another half an afternoon looking at this.
>
> I'm trying a PROPFIND/depth: 1 for *just* DAV: getcontenttype - and I
> still see steadily increasing memory use during the response with trunk
> or trunk plus my patch.
>
> If I break in sbrk the memory allocation which triggers a heap expansion
> is within some r->pool for the subrequest, so I suppose the bug is
> around subreq handling somehow, but it's far from obvious.
Thanks for spending time on this again. By coincidence I stumbled across it
recently again on a system that has Webdav enabled folders with a lot of files in (e.g. 10,000).
Over a short time the only process of this webserver instance with 10 threads, which does
nothing else but serving this stuff via WebDav rose to several GB.
Doing a dump_all_pools showed that all pools and allocator free lists looked well (MaxMemFree
set to 2048 globally).
I was able to limit the memory consumption over time to sane values by adding the following environment
variables:
export MALLOC_MMAP_THRESHOLD_=8192
export MALLOC_ARENA_MAX=3
The system is running RedHat 7.5. So my conclusion was that the glibc was not able to free the memory
again when it was allocated via sbrk, probably because other blocks that remained in use were added
to the top of the heap. OTOH memory also does not seem to get reused with the next request which
is kind of weird given that APR requests pages size aligned blocks which should reduce fragmentation.
Regards
Rüdiger
Re: AW: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/27/2019 10:45 PM, William A Rowe Jr wrote:
> On Thu, Jun 27, 2019 at 2:06 PM Ruediger Pluem <rpluem@apache.org <ma...@apache.org>> wrote:
>
>
> On 06/25/2019 10:39 AM, Plüm, Rüdiger, Vodafone Group wrote:
> >
> > Another related question: Do you think it would be beneficial if we replace
> > the apr_psprintf with apr_pstrcat in props.c as long as they only concat strings without
> > further formatting?
>
> Anyone any opinion on this?
>
>
> I would expect apr_pstrcat to be more performant, wise change.
Done in r1862270.
Regards
Rüdiger
Re: AW: svn commit: r1841225 - /httpd/httpd/trunk/modules/dav/main/props.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, Jun 27, 2019 at 2:06 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 06/25/2019 10:39 AM, Plüm, Rüdiger, Vodafone Group wrote:
> >
> > Another related question: Do you think it would be beneficial if we
> replace
> > the apr_psprintf with apr_pstrcat in props.c as long as they only concat
> strings without
> > further formatting?
>
> Anyone any opinion on this?
>
I would expect apr_pstrcat to be more performant, wise change.
Re: AW: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/25/2019 10:39 AM, Plüm, Rüdiger, Vodafone Group wrote:
>
>
>
>
> Another related question: Do you think it would be beneficial if we replace
> the apr_psprintf with apr_pstrcat in props.c as long as they only concat strings without
> further formatting?
>
>
Anyone any opinion on this?
Regards
Rüdiger
AW: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
C2 General
> -----Ursprüngliche Nachricht-----
> Von: Joe Orton <jo...@redhat.com>
> Gesendet: Dienstag, 25. Juni 2019 09:49
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1841225 -
> /httpd/httpd/trunk/modules/dav/main/props.c
>
> On Mon, Jun 24, 2019 at 09:54:21PM +0200, Ruediger Pluem wrote:
> > On 06/24/2019 07:04 PM, Joe Orton wrote:
> > > #11 0x00007f8110ce6448 in dav_do_prop_subreq (propdb=0x100cfc0) at
> props.c:331
> >
> > What if you go here and do a
> >
> > dump_all_pools propdb->p
> >
> > and do it again when you hit the next sbrk and the next one and so on?
> Something suspicious?
>
> Well I did this and immediately noticed something suspicious on the
> previous line, e_uri is allocated from resource->pool (=r->pool) rather
> than propbb->p, so, yet again the precision accuracy of your insights is
Very good catch. Great that you found it.
> beyond belief Ruediger :) :)
>
> This fixed a significant leak, but r->pool is still growing. I think
> the remaining problem is use of r->pool in dav_fs_walker, i.e. is
> mod_dav_fs specific.
Do you think it is now good enough to commit at least as a first step?
>
> Unless I am missing something the apr_dir_open/apr_dir_read API design
> is always going to have memory use proportional to directory size
> because apr_dir_read() duplicates the filename into the directory's
> pool. We need an apr_dir_pread() or whatever which takes a pool
> argument. :(
In order to fix this leak, I agree that we would need this.
The pool is also used for apr_stat inside apr_dir_read, but I fail to see
how it consumes memory.
Another related question: Do you think it would be beneficial if we replace
the apr_psprintf with apr_pstrcat in props.c as long as they only concat strings without
further formatting?
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jun 24, 2019 at 09:54:21PM +0200, Ruediger Pluem wrote:
> On 06/24/2019 07:04 PM, Joe Orton wrote:
> > #11 0x00007f8110ce6448 in dav_do_prop_subreq (propdb=0x100cfc0) at props.c:331
>
> What if you go here and do a
>
> dump_all_pools propdb->p
>
> and do it again when you hit the next sbrk and the next one and so on? Something suspicious?
Well I did this and immediately noticed something suspicious on the
previous line, e_uri is allocated from resource->pool (=r->pool) rather
than propbb->p, so, yet again the precision accuracy of your insights is
beyond belief Ruediger :) :)
This fixed a significant leak, but r->pool is still growing. I think
the remaining problem is use of r->pool in dav_fs_walker, i.e. is
mod_dav_fs specific.
Unless I am missing something the apr_dir_open/apr_dir_read API design
is always going to have memory use proportional to directory size
because apr_dir_read() duplicates the filename into the directory's
pool. We need an apr_dir_pread() or whatever which takes a pool
argument. :(
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/24/2019 07:04 PM, Joe Orton wrote:
> On Fri, Feb 08, 2019 at 08:07:57AM +0100, Ruediger Pluem wrote:
>> On 11/13/2018 10:26 AM, Joe Orton wrote:
>>> On Mon, Nov 12, 2018 at 08:26:48AM +0100, Ruediger Pluem wrote:
>>>> The discussion died a little bit, because of the other issue (frequent
>>>> writeev calls). I know that the liveprops issue is not fixed yet, but
>>>> I guess it makes sense if you commit the patch you posted here
>>>> already.
>>>
>>> Sorry, I was looking at this again and the repro case I thought worked
>>> was still showing leaks, so I got stuck. Will come back to it hopefully
>>> later this week.
>>>
>>
>> Going through the STATUS of 2.4.x I got aware that this died a little bit again.
>> Any new ideas in the meantime?
>
>
> If I break in sbrk the memory allocation which triggers a heap expansion
> is within some r->pool for the subrequest, so I suppose the bug is
> around subreq handling somehow, but it's far from obvious.
>
> #0 0x00007f8111b60130 in sbrk () from /lib64/libc.so.6
> #1 0x00007f8111af54cd in __default_morecore () from /lib64/libc.so.6
> #2 0x00007f8111af18a3 in sysmalloc () from /lib64/libc.so.6
> #3 0x00007f8111af2a3f in _int_malloc () from /lib64/libc.so.6
> #4 0x00007f8111af3b8f in malloc () from /lib64/libc.so.6
> #5 0x00007f8111c7cd34 in allocator_alloc (in_size=8152, allocator=0xfd69f0) at memory/unix/apr_pools.c:411
> #6 apr_pool_create_ex (newpool=newpool@entry=0x7ffe8dc303f8, parent=0x16fe6b8, abort_fn=0x440d90 <abort_on_oom>,
> abort_fn@entry=0x0, allocator=0xfd69f0, allocator@entry=0x0) at memory/unix/apr_pools.c:1079
> #7 0x0000000000461a4a in ap_location_walk (r=r@entry=0x16fe730) at request.c:1487
> #8 0x0000000000462164 in ap_process_request_internal (r=0x16fe730) at request.c:238
> #9 0x0000000000462cc8 in ap_sub_req_method_uri (method=method@entry=0x47a6a0 "GET",
> new_uri=0x16fc6a8 "/modules/dav/file.b626.txt", r=0x100afb0, next_filter=next_filter@entry=0x0) at request.c:2346
> #10 0x0000000000462d03 in ap_sub_req_lookup_uri (new_uri=<optimized out>, r=<optimized out>,
> next_filter=next_filter@entry=0x0) at request.c:2359
> #11 0x00007f8110ce6448 in dav_do_prop_subreq (propdb=0x100cfc0) at props.c:331
What if you go here and do a
dump_all_pools propdb->p
and do it again when you hit the next sbrk and the next one and so on? Something suspicious?
> #12 0x00007f8110ce665d in dav_insert_coreprop (propdb=propdb@entry=0x100cfc0, propid=<optimized out>,
> name=0x1014e18 "getcontenttype", what=what@entry=DAV_PROP_INSERT_VALUE, phdr=phdr@entry=0x7ffe8dc30550,
> inserted=inserted@entry=0x7ffe8dc30548) at props.c:390
> #13 0x00007f8110ce73b5 in dav_insert_liveprop (what=DAV_PROP_INSERT_VALUE, elem=0x1014da8, elem=0x1014da8,
> inserted=0x7ffe8dc30548, phdr=0x7ffe8dc30550, propdb=0x100cfc0) at props.c:457
> #14 dav_get_props (propdb=0x100cfc0, doc=<optimized out>) at props.c:871
> #15 0x00007f8110cdf7c9 in dav_propfind_walker (wres=0x7ffe8dc307b8, calltype=<optimized out>) at mod_dav.c:2055
> #16 0x00007f8110be4885 in dav_fs_walker (fsctx=fsctx@entry=0x7ffe8dc307b0, depth=depth@entry=1) at repos.c:1600
> #17 0x00007f8110be4df9 in dav_fs_internal_walk (params=0x7ffe8dc30a50, depth=1, is_move=0, root_dst=0x0,
> response=0x7ffe8dc30a48) at repos.c:1844
> #18 0x00007f8110ce083b in dav_method_propfind (r=r@entry=0x100afb0) at mod_dav.c:2192
> #19 0x00007f8110ce37f8 in dav_handler (r=0x100afb0) at mod_dav.c:4814
>
>
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Feb 08, 2019 at 08:07:57AM +0100, Ruediger Pluem wrote:
> On 11/13/2018 10:26 AM, Joe Orton wrote:
> > On Mon, Nov 12, 2018 at 08:26:48AM +0100, Ruediger Pluem wrote:
> >> The discussion died a little bit, because of the other issue (frequent
> >> writeev calls). I know that the liveprops issue is not fixed yet, but
> >> I guess it makes sense if you commit the patch you posted here
> >> already.
> >
> > Sorry, I was looking at this again and the repro case I thought worked
> > was still showing leaks, so I got stuck. Will come back to it hopefully
> > later this week.
> >
>
> Going through the STATUS of 2.4.x I got aware that this died a little bit again.
> Any new ideas in the meantime?
I spent another half an afternoon looking at this.
I'm trying a PROPFIND/depth: 1 for *just* DAV: getcontenttype - and I
still see steadily increasing memory use during the response with trunk
or trunk plus my patch.
If I break in sbrk the memory allocation which triggers a heap expansion
is within some r->pool for the subrequest, so I suppose the bug is
around subreq handling somehow, but it's far from obvious.
#0 0x00007f8111b60130 in sbrk () from /lib64/libc.so.6
#1 0x00007f8111af54cd in __default_morecore () from /lib64/libc.so.6
#2 0x00007f8111af18a3 in sysmalloc () from /lib64/libc.so.6
#3 0x00007f8111af2a3f in _int_malloc () from /lib64/libc.so.6
#4 0x00007f8111af3b8f in malloc () from /lib64/libc.so.6
#5 0x00007f8111c7cd34 in allocator_alloc (in_size=8152, allocator=0xfd69f0) at memory/unix/apr_pools.c:411
#6 apr_pool_create_ex (newpool=newpool@entry=0x7ffe8dc303f8, parent=0x16fe6b8, abort_fn=0x440d90 <abort_on_oom>,
abort_fn@entry=0x0, allocator=0xfd69f0, allocator@entry=0x0) at memory/unix/apr_pools.c:1079
#7 0x0000000000461a4a in ap_location_walk (r=r@entry=0x16fe730) at request.c:1487
#8 0x0000000000462164 in ap_process_request_internal (r=0x16fe730) at request.c:238
#9 0x0000000000462cc8 in ap_sub_req_method_uri (method=method@entry=0x47a6a0 "GET",
new_uri=0x16fc6a8 "/modules/dav/file.b626.txt", r=0x100afb0, next_filter=next_filter@entry=0x0) at request.c:2346
#10 0x0000000000462d03 in ap_sub_req_lookup_uri (new_uri=<optimized out>, r=<optimized out>,
next_filter=next_filter@entry=0x0) at request.c:2359
#11 0x00007f8110ce6448 in dav_do_prop_subreq (propdb=0x100cfc0) at props.c:331
#12 0x00007f8110ce665d in dav_insert_coreprop (propdb=propdb@entry=0x100cfc0, propid=<optimized out>,
name=0x1014e18 "getcontenttype", what=what@entry=DAV_PROP_INSERT_VALUE, phdr=phdr@entry=0x7ffe8dc30550,
inserted=inserted@entry=0x7ffe8dc30548) at props.c:390
#13 0x00007f8110ce73b5 in dav_insert_liveprop (what=DAV_PROP_INSERT_VALUE, elem=0x1014da8, elem=0x1014da8,
inserted=0x7ffe8dc30548, phdr=0x7ffe8dc30550, propdb=0x100cfc0) at props.c:457
#14 dav_get_props (propdb=0x100cfc0, doc=<optimized out>) at props.c:871
#15 0x00007f8110cdf7c9 in dav_propfind_walker (wres=0x7ffe8dc307b8, calltype=<optimized out>) at mod_dav.c:2055
#16 0x00007f8110be4885 in dav_fs_walker (fsctx=fsctx@entry=0x7ffe8dc307b0, depth=depth@entry=1) at repos.c:1600
#17 0x00007f8110be4df9 in dav_fs_internal_walk (params=0x7ffe8dc30a50, depth=1, is_move=0, root_dst=0x0,
response=0x7ffe8dc30a48) at repos.c:1844
#18 0x00007f8110ce083b in dav_method_propfind (r=r@entry=0x100afb0) at mod_dav.c:2192
#19 0x00007f8110ce37f8 in dav_handler (r=0x100afb0) at mod_dav.c:4814
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 11/13/2018 10:26 AM, Joe Orton wrote:
> On Mon, Nov 12, 2018 at 08:26:48AM +0100, Ruediger Pluem wrote:
>> The discussion died a little bit, because of the other issue (frequent
>> writeev calls). I know that the liveprops issue is not fixed yet, but
>> I guess it makes sense if you commit the patch you posted here
>> already.
>
> Sorry, I was looking at this again and the repro case I thought worked
> was still showing leaks, so I got stuck. Will come back to it hopefully
> later this week.
>
Going through the STATUS of 2.4.x I got aware that this died a little bit again.
Any new ideas in the meantime?
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Nov 12, 2018 at 08:26:48AM +0100, Ruediger Pluem wrote:
> The discussion died a little bit, because of the other issue (frequent
> writeev calls). I know that the liveprops issue is not fixed yet, but
> I guess it makes sense if you commit the patch you posted here
> already.
Sorry, I was looking at this again and the repro case I thought worked
was still showing leaks, so I got stuck. Will come back to it hopefully
later this week.
Re: svn commit: r1841225 - /httpd/httpd/trunk/modules/dav/main/props.c
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Nov 12, 2018 at 1:26 AM Ruediger Pluem <rp...@apache.org> wrote:
>...
> The discussion died a little bit, because of the other issue (frequent
> writeev calls).
> I know that the liveprops issue is not fixed yet, but I guess it makes
> sense
> if you commit the patch you posted here already.
>
My time has been getting consumed by Infra. ... I hope to revisit one day,
but it looks like Joe is throwing in with some additional eyeballs.
Cheers,
-g
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/18/2018 01:45 PM, Joe Orton wrote:
> On Thu, Oct 18, 2018 at 11:09:13AM +0200, Ruediger Pluem wrote:
>> On 10/17/2018 07:47 PM, Joe Orton wrote:
>>> On Wed, Oct 17, 2018 at 03:32:34PM +0100, Joe Orton wrote:
>>>> I see constant memory use for a simple PROPFIND/depth:1 for the
>>>> attached, though I'm not sure this is sufficient to repro the problem
>>>> you saw before.
>>
>> Thanks for having a look. My test case was opening a large directory (about 50,000 files)
>> with doplhin under RedHat 6. Memory usage remains constant after the patch. So patch
>> seems to make sense.
>
> OK thanks. I've tested this works to fix the memory consumption in that
> case (both as committed in trunk and with my change to it), when the
> PROPFIND is for *only* dead properties.
>
> There is still more work to make memory use constant for liveprops.
> mod_dav_svn has done this already I assume, since the scratchpool was
> added to fix this issue with SVN IIRC. So it should be possible with
> some effort for mod_dav_fs too, I will hopefully return to that
> sometime.
The discussion died a little bit, because of the other issue (frequent writeev calls).
I know that the liveprops issue is not fixed yet, but I guess it makes sense
if you commit the patch you posted here already.
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Oct 18, 2018 at 11:09:13AM +0200, Ruediger Pluem wrote:
> On 10/17/2018 07:47 PM, Joe Orton wrote:
> > On Wed, Oct 17, 2018 at 03:32:34PM +0100, Joe Orton wrote:
> >> I see constant memory use for a simple PROPFIND/depth:1 for the
> >> attached, though I'm not sure this is sufficient to repro the problem
> >> you saw before.
>
> Thanks for having a look. My test case was opening a large directory (about 50,000 files)
> with doplhin under RedHat 6. Memory usage remains constant after the patch. So patch
> seems to make sense.
OK thanks. I've tested this works to fix the memory consumption in that
case (both as committed in trunk and with my change to it), when the
PROPFIND is for *only* dead properties.
There is still more work to make memory use constant for liveprops.
mod_dav_svn has done this already I assume, since the scratchpool was
added to fix this issue with SVN IIRC. So it should be possible with
some effort for mod_dav_fs too, I will hopefully return to that
sometime.
> Any idea if we could hit a similar issue with the two other remaining callers of dav_open_propdb?
>
> dav_gen_supported_live_props
> dav_method_proppatch
The specific issue here is in the "walker" function iterations - it was
long known this can consume all your RAM for "Depth: infinity" walks,
but I guess nobody ever thought about it too hard for Depth: 1. LABEL,
PROPFIND and LOCK/UNLOCK may all be affected I think.
Regards, Joe
Re: svn commit: r1841225 - /httpd/httpd/trunk/modules/dav/main/props.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Oct 18, 2018 at 11:29 AM Ruediger Pluem <rp...@apache.org> wrote:
> >
> > The reason is ap_request_core_filter. It iterates over the brigade and hands over each bucket alone to
> > ap_core_output_filter. IMHO a bug.
>
> How about the attached patch for fixing?
+1, nice way to address it!
Thanks,
Yann.
Re: ap_request_core_filter issues
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/23/2018 03:52 PM, Yann Ylavic wrote:
> On Tue, Oct 23, 2018 at 3:18 PM Plüm, Rüdiger, Vodafone Group
> <ru...@vodafone.com> wrote:
>>
>>> -----Ursprüngliche Nachricht-----
>>> Von: Yann Ylavic <yl...@gmail.com>
>>>
>>> I tried to implement that in the attached patch, WDYT?
>>
>> 1. Why removing the META_BUCKET check when reading buckets of length -1? I don't see any sense in
>> doing an apr_bucket_read on a META_BUCKET.
>
> META usually have a length of 0, not -1 (so we shouldn't read them either).
> We pretty much everywhere detect morphing buckets with -1 only, META
> or not, and honestly if a META has length == -1 we probably should
> consider it's a morphing META and let read figure out :)
You got me. I should have validated my assumption from memory. All good.
>
>> 2. Apart from 1. I think the flow is correct, but I am a little bit worried if calling ap_filter_reinstate_brigade
>> inside the loop iterating over the brigade has a too large performance impact as ap_filter_reinstate_brigade
>> iterates itself over the brigade. So we are in O(n^2) with n being the size of the brigade. But to be honest I have
>> no good idea yet how to do without. The only thing I can think of is to duplicate and merge the logic of ap_filter_reinstate_brigade
>> inside our loop to avoid this as we could gather the stats needed for deciding up to which bucket we need to write while
>> doing our iteration of the brigade.
>
> Yeah, I have a version which checks only for
> (APR_BUCKET_IS_FLUSH(bucket) || cumulative_tmp_bb_len >=
> ap_get_core_module_config()->flush_max_threshold), but I thought I
> would not open code in the first version to show the idea...
Fair enough. The first patch made the idea more clear, but this one is the one that should be used for performance reasons.
>
> Ideally we'd have another ap_filter_xxx helper which could take a
> bucket (and offset) to start, and also account for f->next(s)'
> *bytes_in_brigade if asked to...
Sounds sensible.
Regards
Rüdiger
Re: ap_request_core_filter issues
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 23, 2018 at 3:52 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> Yeah, I have a version which checks only for
> (APR_BUCKET_IS_FLUSH(bucket) || cumulative_tmp_bb_len >=
> ap_get_core_module_config()->flush_max_threshold)
See attached.
Re: ap_request_core_filter issues
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Oct 23, 2018 at 3:18 PM Plüm, Rüdiger, Vodafone Group
<ru...@vodafone.com> wrote:
>
> > -----Ursprüngliche Nachricht-----
> > Von: Yann Ylavic <yl...@gmail.com>
> >
> > I tried to implement that in the attached patch, WDYT?
>
> 1. Why removing the META_BUCKET check when reading buckets of length -1? I don't see any sense in
> doing an apr_bucket_read on a META_BUCKET.
META usually have a length of 0, not -1 (so we shouldn't read them either).
We pretty much everywhere detect morphing buckets with -1 only, META
or not, and honestly if a META has length == -1 we probably should
consider it's a morphing META and let read figure out :)
> 2. Apart from 1. I think the flow is correct, but I am a little bit worried if calling ap_filter_reinstate_brigade
> inside the loop iterating over the brigade has a too large performance impact as ap_filter_reinstate_brigade
> iterates itself over the brigade. So we are in O(n^2) with n being the size of the brigade. But to be honest I have
> no good idea yet how to do without. The only thing I can think of is to duplicate and merge the logic of ap_filter_reinstate_brigade
> inside our loop to avoid this as we could gather the stats needed for deciding up to which bucket we need to write while
> doing our iteration of the brigade.
Yeah, I have a version which checks only for
(APR_BUCKET_IS_FLUSH(bucket) || cumulative_tmp_bb_len >=
ap_get_core_module_config()->flush_max_threshold), but I thought I
would not open code in the first version to show the idea...
Ideally we'd have another ap_filter_xxx helper which could take a
bucket (and offset) to start, and also account for f->next(s)'
*bytes_in_brigade if asked to...
Regards,
Yann.
>
>
> Regards
>
> Rüdiger
>
AW: ap_request_core_filter issues
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
> -----Ursprüngliche Nachricht-----
> Von: Yann Ylavic <yl...@gmail.com>
> Gesendet: Dienstag, 23. Oktober 2018 13:09
> An: httpd-dev <de...@httpd.apache.org>
> Betreff: Re: ap_request_core_filter issues
>
> On Thu, Oct 18, 2018 at 1:52 PM Joe Orton <jo...@redhat.com> wrote:
> >
> > On Thu, Oct 18, 2018 at 12:51:06PM +0200, Ruediger Pluem wrote:
> > > >>>
> > > >>> Curiously inefficient writev use when stracing the process,
> though,
> > > >>> dunno what's going on there (trunk/prefork):
> > > >>>
> > > >>> writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
> > > >>> writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
> > > >>> writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"...,
> iov_len=7820}], 1) = 7820
> > > >>> writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"...,
> iov_len=248}], 1) = 248
> > > >>
> > > >> The reason is ap_request_core_filter. It iterates over the
> brigade and hands over each bucket alone to
> > > >> ap_core_output_filter. IMHO a bug.
> > > >
> > > > How about the attached patch for fixing?
> > >
> > > Scratch that. I guess it is much more complex.
> >
> > Hmm, well it works for me and looks correct, it batches up those the
> > writev(,,1) calls as expected, but it seems YMMV :)
>
> After a closer look into this, Rüdiger's patch seems indeed incomplete.
> What we possibly want is keep buckets in tmp_bb until it's time to
> pass it down the chain, per ap_filter_reinstate_brigade() logic (i.e.
> FLUSH bucket, size limit, ...).
>
> I tried to implement that in the attached patch, WDYT?
1. Why removing the META_BUCKET check when reading buckets of length -1? I don't see any sense in
doing an apr_bucket_read on a META_BUCKET.
2. Apart from 1. I think the flow is correct, but I am a little bit worried if calling ap_filter_reinstate_brigade
inside the loop iterating over the brigade has a too large performance impact as ap_filter_reinstate_brigade
iterates itself over the brigade. So we are in O(n^2) with n being the size of the brigade. But to be honest I have
no good idea yet how to do without. The only thing I can think of is to duplicate and merge the logic of ap_filter_reinstate_brigade
inside our loop to avoid this as we could gather the stats needed for deciding up to which bucket we need to write while
doing our iteration of the brigade.
Regards
Rüdiger
Re: ap_request_core_filter issues
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Oct 18, 2018 at 1:52 PM Joe Orton <jo...@redhat.com> wrote:
>
> On Thu, Oct 18, 2018 at 12:51:06PM +0200, Ruediger Pluem wrote:
> > >>>
> > >>> Curiously inefficient writev use when stracing the process, though,
> > >>> dunno what's going on there (trunk/prefork):
> > >>>
> > >>> writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
> > >>> writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
> > >>> writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"..., iov_len=7820}], 1) = 7820
> > >>> writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"..., iov_len=248}], 1) = 248
> > >>
> > >> The reason is ap_request_core_filter. It iterates over the brigade and hands over each bucket alone to
> > >> ap_core_output_filter. IMHO a bug.
> > >
> > > How about the attached patch for fixing?
> >
> > Scratch that. I guess it is much more complex.
>
> Hmm, well it works for me and looks correct, it batches up those the
> writev(,,1) calls as expected, but it seems YMMV :)
After a closer look into this, Rüdiger's patch seems indeed incomplete.
What we possibly want is keep buckets in tmp_bb until it's time to
pass it down the chain, per ap_filter_reinstate_brigade() logic (i.e.
FLUSH bucket, size limit, ...).
I tried to implement that in the attached patch, WDYT?
>
> I notice the function is doing unconditional blocking reads without
> flushing first - rule 10 of the golden rules of output filters! --
The patch also addresses this.
Regards,
Yann.
ap_request_core_filter issues
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Oct 18, 2018 at 12:51:06PM +0200, Ruediger Pluem wrote:
> >>>
> >>> Curiously inefficient writev use when stracing the process, though,
> >>> dunno what's going on there (trunk/prefork):
> >>>
> >>> writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
> >>> writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
> >>> writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"..., iov_len=7820}], 1) = 7820
> >>> writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"..., iov_len=248}], 1) = 248
> >>
> >> The reason is ap_request_core_filter. It iterates over the brigade and hands over each bucket alone to
> >> ap_core_output_filter. IMHO a bug.
> >
> > How about the attached patch for fixing?
>
> Scratch that. I guess it is much more complex.
Hmm, well it works for me and looks correct, it batches up those the
writev(,,1) calls as expected, but it seems YMMV :)
I notice the function is doing unconditional blocking reads without
flushing first - rule 10 of the golden rules of output filters! --
status = apr_bucket_read(bucket, &data, &size, APR_BLOCK_READ);
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/18/2018 11:29 AM, Ruediger Pluem wrote:
>
>
> On 10/18/2018 11:09 AM, Ruediger Pluem wrote:
>>
>>
>> On 10/17/2018 07:47 PM, Joe Orton wrote:
>>> On Wed, Oct 17, 2018 at 03:32:34PM +0100, Joe Orton wrote:
>>>> I see constant memory use for a simple PROPFIND/depth:1 for the
>>>> attached, though I'm not sure this is sufficient to repro the problem
>>>> you saw before.
>
>>>
>>> Curiously inefficient writev use when stracing the process, though,
>>> dunno what's going on there (trunk/prefork):
>>>
>>> writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
>>> writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
>>> writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"..., iov_len=7820}], 1) = 7820
>>> writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"..., iov_len=248}], 1) = 248
>>>
>>>
>>
>> The reason is ap_request_core_filter. It iterates over the brigade and hands over each bucket alone to
>> ap_core_output_filter. IMHO a bug.
>
> How about the attached patch for fixing?
Scratch that. I guess it is much more complex.
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/18/2018 11:09 AM, Ruediger Pluem wrote:
>
>
> On 10/17/2018 07:47 PM, Joe Orton wrote:
>> On Wed, Oct 17, 2018 at 03:32:34PM +0100, Joe Orton wrote:
>>> I see constant memory use for a simple PROPFIND/depth:1 for the
>>> attached, though I'm not sure this is sufficient to repro the problem
>>> you saw before.
>>
>> Curiously inefficient writev use when stracing the process, though,
>> dunno what's going on there (trunk/prefork):
>>
>> writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
>> writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
>> writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"..., iov_len=7820}], 1) = 7820
>> writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"..., iov_len=248}], 1) = 248
>>
>>
>
> The reason is ap_request_core_filter. It iterates over the brigade and hands over each bucket alone to
> ap_core_output_filter. IMHO a bug.
How about the attached patch for fixing?
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/17/2018 07:47 PM, Joe Orton wrote:
> On Wed, Oct 17, 2018 at 03:32:34PM +0100, Joe Orton wrote:
>> I see constant memory use for a simple PROPFIND/depth:1 for the
>> attached, though I'm not sure this is sufficient to repro the problem
>> you saw before.
Thanks for having a look. My test case was opening a large directory (about 50,000 files)
with doplhin under RedHat 6. Memory usage remains constant after the patch. So patch
seems to make sense.
Any idea if we could hit a similar issue with the two other remaining callers of dav_open_propdb?
dav_gen_supported_live_props
dav_method_proppatch
>
> I needed to also remove the new apr_pool_clear() there. Is the repro
> case for this something other than depth:1 PROPFIND? Am seing constant
> memory use with
>
> $ mkdir t/htdocs/modules/dav/
> $ (cd t/htdocs/modules/dav/;
> seq 1 100000 | sed 's/^/file.b/;s/$/.txt/' | xargs -n100 touch )
> $ ./t/TEST -start
>
> and then run a PROPFIND against /modules/dav/
>
> Curiously inefficient writev use when stracing the process, though,
> dunno what's going on there (trunk/prefork):
>
> writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
> writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
> writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"..., iov_len=7820}], 1) = 7820
> writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"..., iov_len=248}], 1) = 248
>
>
The reason is ap_request_core_filter. It iterates over the brigade and hands over each bucket alone to
ap_core_output_filter. IMHO a bug.
Regards
Rüdiger
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Oct 17, 2018 at 03:32:34PM +0100, Joe Orton wrote:
> I see constant memory use for a simple PROPFIND/depth:1 for the
> attached, though I'm not sure this is sufficient to repro the problem
> you saw before.
I needed to also remove the new apr_pool_clear() there. Is the repro
case for this something other than depth:1 PROPFIND? Am seing constant
memory use with
$ mkdir t/htdocs/modules/dav/
$ (cd t/htdocs/modules/dav/;
seq 1 100000 | sed 's/^/file.b/;s/$/.txt/' | xargs -n100 touch )
$ ./t/TEST -start
and then run a PROPFIND against /modules/dav/
Curiously inefficient writev use when stracing the process, though,
dunno what's going on there (trunk/prefork):
writev(46, [{iov_base="\r\n", iov_len=2}], 1) = 2
writev(46, [{iov_base="1f84\r\n", iov_len=6}], 1) = 6
writev(46, [{iov_base="<D:lockdiscovery/>\n<D:getcontent"..., iov_len=7820}], 1) = 7820
writev(46, [{iov_base="<D:supportedlock>\n<D:lockentry>\n"..., iov_len=248}], 1) = 248
Re: svn commit: r1841225 -
/httpd/httpd/trunk/modules/dav/main/props.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Oct 16, 2018 at 02:28:06PM +0200, Ruediger Pluem wrote:
> Gentle ping :-). Did you find some time to have a look?
"I'm not Greg, but..."
This seems to be duplicating the ctx->scratchpool which was added for
use in dav_propfind_walker specifically to solve this kind of problem, I
think?
I see constant memory use for a simple PROPFIND/depth:1 for the
attached, though I'm not sure this is sufficient to repro the problem
you saw before.
I think the patch makes sense anyway, though I don't remember why that
dav_close_propdb() wasn't destroying the pool before, hopefully Greg
does.
Regards, Joe