On (14/05/13 16:04), Jakub Hrozek wrote:
On Mon, May 13, 2013 at 05:08:44PM +0200, Lukas Slebodnik wrote:
> ehlo,
>
> Jakub, Pavel and me disscus about ticket
>
https://fedorahosted.org/sssd/ticket/1823
> "getgrnam / getgrgid for large user groups is too slow due to range retrieval
> functionality"
> We decided to add new option to turn the range retrieval with AD. Another
> solution could be to add new option to limit the number of group members
> processed.
>
> Any comments are welcomed.
>
> This commit adds new option ldap_disable_large_groups with default value FALSE.
> If this option is enabled, large groups(>1500) will not be retrieved and
> behaviour will be similar like was before commit ae8d047122c
> "LDAP: Handle very large Active Directory groups"
>
> LS
This is a good start, but I would prefer if the option name included
"range retrieval", for two reasons:
* the current option name would make the user think that the option
works with every provider
* in general, the range extension might not be limited to group
objects on the AD side.
What about "ldap_disable_range_retrieval" ?
Option renamed.
Similarly the parameter of sdap_parse_entry should be renamed,
sdap_parse_entry can be used to parse any generic entry, not just group
(it should have no knowledge about existence of groups).
I was also wondering if the check could be moved all the way to
sdap_parse_range() to keep the knowledge about range retrievals there.
Maybe introduce a new error code (or use something from errno) that would
be returned when the feature is off. If this is not possible because it
might break the flow of sdap_parse_entry(), then please #define the
";range" string and put it into sdap_range.h.
Can you also send a follow-up patch that removes the functions
sdap_parse_user() and sdap_parse_group(). Looks like dead code.
functions removed in first patch.
> --- a/src/man/sssd-ldap.5.xml
> +++ b/src/man/sssd-ldap.5.xml
> @@ -1201,6 +1201,25 @@
> </varlistentry>
>
> <varlistentry>
> + <term>ldap_disable_large_groups (boolean)</term>
> + <listitem>
> + <para>
> + Disable very large Active Directory groups.
> + </para>
> + <para>
> + This option has effect only, if LDAP server is
> + Active Directory
what about "...if SSSD is configured as an Active Directory client..." ?
Changed.
Updated patches are attached.
LS