Evgeny Kotkov wrote on Fri, Jan 29, 2016 at 17:37:28 +0300:
> Daniel Shahaf <danielsh_at_apache.org> writes:
>
> > Well, it isn't consensus, but there's an old fallback we can use:
> > pick an agreed-upon third party and let him decide.
> >
> > So let's ask a mod_dav/mod_dav_svn dev for his opinion and do what he says.
>
> It seems like we still don't have a consensus here. I still say we should
> just go with (2) and have the problem solved. But since we were unable to
> agree on that, this issue is now tracked as SVN-4616:
>
> https://issues.apache.org/jira/browse/SVN-4616
I was merely trying to help find a find a better solution by reviewing
a technical proposal that had been brought up to the list. It was never
my intention to cause the issue to be left open indefinitely.
What can we do to make progress on fixing this bug?
> One thing worth adding is that there's also a problem with how we log actions
> from mod_dav_svn callbacks.
>
> Within a callback, we lack the information about the state of the operation.
> As one example, deadprops.c:db_first_name() calls dav_svn__operational_log().
> While handling <allprop/> depth 1 PROPFIND requests, db_first_name() is called
> once per every walked entry. Every subsequent call rewrites previously set
> r->subprocess_env values. Hence, the actual log entry is determined by the
> last walked entry, and that's incorrect. And all these values are allocated
> in the request pool, thus triggering unbounded memory usage.
>
> I think that this part of the problem can't be solved properly without
> reimplementing the PROPFIND in mod_dav_svn.
What does "properly solved" mean to you? I expect a 'propget
--depth=files $URL' to result in one operational log entry for $URL (and
no operational log entries for the children).
For the short term, we can simply skip setting the "SVN-ACTION" value
and friends, if a value is already set. That will stop the O(#dirents)
memory usage. We can do this in the next patch release.
For the slightly longer term, perhaps we should look into moving the
dav_svn__opertional_log() call from db_first_name() to mod_dav_svn/repos.c:walk():
that function seems like a more logical place to log a "I am doing
a propget --depth=files" operation from. (Can this be done without
extending the mod_dav↔walker API?)
Cheers,
Daniel
P.S. I may have limited net access next week, but I'll follow this
thread as much as I can.
Received on 2016-01-30 08:47:42 CET