Petr,
On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech <pcech(a)redhat.com> wrote:
> On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:
>>
>> Petr,
>>
>> I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
>> 50. However, you haven't update the value on sssd.conf.5.xml:
>>
>> diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
>> index
>>
ae291e0fc8f2f9afabcdf32f18a5ec12252bbbbf..1bf3e799047d9c722487be8657bbee5cfd479cdd
>> 100644
>> --- a/src/man/sssd.conf.5.xml
>> +++ b/src/man/sssd.conf.5.xml
>> @@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
>> </listitem>
>> </varlistentry>
>>
>> + <varlistentry>
>> + <term>proxy_max_children (integer)</term>
>> + <listitem>
>> + <para>
>> + The number of preforked proxy children.
>> + </para>
>> + <para>
>> + Default: 50
>> here: ^^^
>>
>> + </para>
>> + </listitem>
>> + </varlistentry>
>> +
>> </variablelist>
>> </para>
>>
>> Apart from this minor the patch seems to be following everything that
>> was requested during the review process. However, I'm not comfortable
>> with the text used to describe the new option, so adding there a bit
>> more information would be super. Like, I don't know what's the
>> influence of the preforked proxy children to the rest of the code
>> (probably because I'm a newbie here ;-)), but would be nice to have it
>> clear in the documentation (for newbies like myself ;-)).
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>
>
> Hi Fabiano,
>
> thanks for code review. I fixed the default value in man page and I
> reformulated description. Is it better?
>
> Regards
>
> --
> Petr^4 Čech
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>
Looking at the changes in the manual ...
+ In busy environments it is possible sssd
runs out of
+ available child slots and starts queuing requests
+ in proxy mode. This option introduces the number of
+ preforked proxy children.
I personally go for something like:
"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".
*: Not sure whether busy environments is something clear for everyone ...
IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?
Best Regards,
--
Fabiano Fidêncio
Fabiano,
I took your suggestion, thanks. I don't know right term for 'busy
environments'. I will be glad if native speaker help me with the right
formulation of description.
Regards
--
Petr^4 Čech