On 09/06/2016 04:17 PM, Justin Stephenson wrote:
>
>
> On 09/05/2016 10:20 AM, Petr Cech wrote:
>> On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:
>>> 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.
>
> Something like "it is useful for high-load SSSD environments" or "it
is
> useful for larger environments" may work - whichever you prefer.
>
> Kind regards,
> Justin Stephenson
Hello,
new version attached. Description is:
This option specifies the number of pre-forked
proxy children. It is useful for high-load SSSD
environments where sssd may run out of available
child slots, which would cause some issues due to
the requests being queued.
After hint from Michal I replaced introduces by specifies.
Is this version linguistic right?
This description looks good to me.
-Justin
>
> Regards
>
>
>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>