On Wed 14 Nov 2012 06:39:06 PM EST, Paul B. Henson wrote:
On 11/14/2012 1:41 PM, Stephen Gallagher wrote:
Minor: Please use the new SSSDBG macros in confdb_get_domain_internal().
You don't need to update the existing code, but all new code should use
the macros. See util.h for a listing of them.
Done.
../src/providers/ldap/sdap.h:481:5: note: expected 'const char **' but
argument is of type 'char **'
Fixed.
You need to add 'ignore_group_members' to the tuple of expected values
in the test in SSSDConfigTest.py
D'oh, I actually had an open editor buffer with that change in it that
it seems I lost track of and never saved 8-/.
Please indent the rest of the ?: structure so that it's clear that
member_filter : NULL is part of the above. &state->attrs, NULL should be
on the next line. It'll read easier that way.
Done.
!state->dom->ignore_group_members with a comment as to what that means?
The if-then-else construct is a bit confusing there.
Good suggestion, I guess I had just had ternaries on my mind from the
previous use :).
nsssrv_cmd.c has added a tab at the beginning of the line
if (!dom->ignore_group_members) {
Oops, I try to play chameleon when working on other people's code
bases but I guess my editor slipped that one past me.
I've attached an updated patch that I believe addresses all of your
issues, let me know if I missed something or anything else needs a
tuneup.
Thanks...
I've run some tests on this today. With the option unset or False,
behavior appears to be properly unmodified from the existing code
(Good). I ran a simple performance test on a moderately complex AD
environment I already had set up.
That environment has 3000 users that are a members of a series of
nested groups. I have the following baseline for performance (on a
completely empty cache):
[root@sgallagh520 db]# time id kurashima@adtest
uid=201145(kurashima) gid=200513(domain users) groups=200513(domain
users),204697(abovetopgroup),201108(topgroup),201109(nestedgroup),204699(aboveeventhat),204695(uppergroup),201107(sssdgroup),204698(aboveabovetop)
real 0m1.795s
user 0m0.002s
sys 0m0.009s
Changing only the ignore_group_members output, this becomes:
[root@sgallagh520 db]# time id kurashima@adtest
uid=201145(kurashima) gid=200513(domain users) groups=200513(domain
users),204697(abovetopgroup),201108(topgroup),201109(nestedgroup),204699(aboveeventhat),204695(uppergroup),201107(sssdgroup),204698(aboveabovetop)
real 0m0.441s
user 0m0.002s
sys 0m0.007s
This is obviously already a significant enhancement, and of course the
difference will be more pronounced for much larger environments. I'm
prepared to give this an ack, with one comment to whoever pushes the
patch upstream: please reflow the changes in nsssrv_cmd.c's
fill_grent() that are now exceeding the 80-character line limit due to
the new indentation.
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
