On 05.11.2016 17:51, Hans van Kranenburg wrote:
On 11/05/2016 05:15 PM, Daniel Shahaf wrote:
Hans van Kranenburg wrote on Sat, Nov 05, 2016 at 16:39:44 +0100:
On 11/05/2016 11:44 AM, Stefan Fuhrmann wrote:
Looking at your workaround, I think the cleanest solution would be to
introduce an AuthzSVNDisableCache option.
I'm not sure a knob to _disable_ caching is the right approach. So long
as using the cache trades off correctness for performance, shouldn't the
cache be off by default, with a knob to enable it? I.e., shouldn't the
authz code be conservative and default to prefering correctness over
performance?
Ack.
The opt-out approach would be the one that does not break
existing known working setups. Otherwise, opt-in might be
the better choice.
I also guess that the cache invalidation change will have to lead
to deprecation/removal of the option again shortly thereafter, causing
extra cruft in the code.
Sure. OTOH, in other parts of Subversion, being able to
disable certain caches has come in handy in the past for a
variety of - often unforeseen - reasons. Thus, the option
would not become entirely pointless.
I don't see noticable performance issues at all while reparsing the
little files every time right now (how big/complex is the average authz
file?)
I remember generated authz files > 10MB. That translates
into 100s of ms parser overhead per e.g. GET request.
on large operations, consisting of many http requests to do
checkouts/commits etc. Especially not compared to using htgroup, which
feels like it gets into some O(2^n) horror when you put a user into too
many groups.
Well, I think the current authz group resolution can also
be expensive if you carelessly nest groups.
As to changing the value of AuthzSVNDisableCache: the test framework
doesn't let you do that; it presumes an httpd is already running. But
the test suite could launch the server with cache enabled, and provide
a harness option to run with cache disabled, similar to the handling of
the 'short-circuit' option.
(I assume the parsed file's cache lifetime is per-connection in svnserve too)
No, svnserve has a global cache for authz. Correctly
controlling object/pool lifetime had been a major issue.
-- Stefan^2.
