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?
Regards
--
Petr^4 Čech