On Mon, Nov 19, 2012 at 05:04:43PM +0100, Pavel Březina wrote:
On 11/19/2012 03:55 PM, Jakub Hrozek wrote:
>On Mon, Nov 19, 2012 at 01:29:39PM +0100, Pavel Březina wrote:
>>Hi,
>>nitpicks...
>>
>>>+/* TODO: get the restart related values from config */
>>>+#define MONITOR_RESTART_CNT_RESET 30
>>
>>I had some troubles interpreting this macro. Can you change the name in
>>such way that it is more clear that we are resetting restart counter
>>after 30 seconds? Maybe
>>MONITOR_RESET_RESTART_COUNTER_INTERVAL
>>
>
>That's too long, but I provided a new name, hopefully it's more
>readable.
>
>>>+ /* restarts are schedule after 0, 2, 4 seconds */
>>>+ restart_delay = svc->restarts << 1;
>>>+ restart_delay = (restart_delay > MONITOR_MAX_RESTART_DELAY) ? \
>>>+ MONITOR_MAX_RESTART_DELAY : \
>>>+ restart_delay;
>>
>>I think this is easier to read:
>>
>>/* restarts are schedule after 0, 2, 4 seconds */
>>restart_delay = svc->restarts << 1;
>>if (restart_delay > MONITOR_MAX_RESTART_DELAY) {
>> restart_delay = MONITOR_MAX_RESTART_DELAY;
>>}
>
>Right :-) thanks.
>
>New patch is attached.
Ack.
Pushed to master and sssd-1-9