On 13.08.2014 10:52, Stefan Fuhrmann wrote:
>
> On Mon, Aug 11, 2014 at 8:02 PM, Branko Čibej <brane_at_wandisco.com
> <mailto:brane_at_wandisco.com>> wrote:
>
> On 11.08.2014 18:37, Stefan Fuhrmann wrote:
>> On Sun, Aug 10, 2014 at 7:05 PM, Branko Čibej <brane_at_wandisco.com
>> <mailto:brane_at_wandisco.com>> wrote:
>>
>> On 10.08.2014 18:57, Branko Čibej wrote:
>>
>>>>
>>>> FWIW, you should even have the get_memberships function
>>>> any more.
>>>
>>> To clarify: I plan to get rid of svn_authz_tng_t::groups
>>> at some point. That hash is completely redundant.
>>>
>>>
>>> Careful! This might get us into scalability troubles. But I
>>> reserve
>>> judgement until you actually came forth with an implementation.
>>
> The authz_ace_t::members contains exactly the same info, except
> that you don't have to look up the group name in
> svn_authz_tng_t::groups. Since it both your and my way require one
> hash lookup to determine if a (group) ACE pertains to a user,
> reversing the current groups hash to get memberships is just a
> waste of cycles. The groups hash in the authz structure actually
> only costs a bit of memory for the hash structure in the result
> pool, but otherwise it doesn't give any benefit and I don't
> believe there's any use for it. Note that I'm of course not
> copying hashes around; the group hashes, user and group names are
> all singletons.
>
>
> So, basically, you intend to change the ACE struct from
> "(user name)->rights" to "(hash of user names)->rights"
> with the hash actually being either the current per-group
> instance or an equally singleton per-user instance.
>
> That makes perfect sense. I had gotten the impression
> that you would expand each group right into multiple ACEs,
> one per user or so.
Eh, no, because that doesn't work in the presence of inverted ACEs. Take
a look at how svn_authz__ace_get_access works: we have to scan all the
ACEs in the ACL. I did consider using a hash as you proposed, but
couldn't figure out how to reliably represent inversions. Maybe with two
hashes ... which is going way overboard. This only makes sense if there
are authz files in the wild with lots and lots of per-user ACEs in each
rule; and we can simply advise users to create groups in that case. Or
go with the double-hash approach if and when it becomes necessary (it's
far more obscure than the current code, though).
The idea is that the ACEs are scanned when we generate the per (user,
repository) filtered authz structure, which is currently once per
connection; and and the svn_authtz(_tng)_t structure should eventually
outlive individual connections, so the structure should be optimized
towards fast and simple filtering and therefore the authz parser can and
should do extra work to have as much information as possible ready for
the filter (hence my suggestion that we pre-parse the rules themselves;
which I still think is a good idea, as long as we add predicate
functions for detecting prefix matches).
For example, checking the global rules in the authz struct is very fast
now that the parser does the heavy lifting in the second pass, and
should avoid having to create a filtered ruleset in many cases.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco | Realising the impossibilities of Big Data
e. brane_at_wandisco.com
Received on 2014-08-13 14:23:31 CEST