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.
Received on 2016-11-06 13:47:25 CET