[SSSD] [PATCH] Restart services with a delay in case they are restarted too often
Pavel Březina
pbrezina at redhat.com
Mon Nov 19 12:29:39 UTC 2012
On 11/18/2012 06:09 PM, Jakub Hrozek wrote:
> On Fri, Nov 16, 2012 at 11:41:50AM +0100, Pavel Březina wrote:
>> On 11/15/2012 07:51 PM, Jakub Hrozek wrote:
>>> In case a service is restarted while the DP is not ready yet, it gets
>>> restarted again immediatelly, which means the DP might still not be
>>> ready. The allowed number of restarts is then depleted quickly.
>>>
>>> This patch changes the restart mechanism such that the first restart
>>> happens immediatelly, the second is scheduled after 2 second, then 4
>>> etc..
>>>
>>> https://fedorahosted.org/sssd/ticket/1528
>>
>> Hi,
>>
>>> + /* restarts are schedule after 0, 2, 4, ...seconds */
>>> + tv = tevent_timeval_current_ofs((svc->restarts << 1), 0);
>>> + te = tevent_add_timer(svc->mt_ctx->ev, svc, tv, mt_svc_restart, svc);
>>
>> I would consider setting 4 seconds as a maximum delay instead of
>> going to infinity. Any opinions?
>
> I agree. I wanted to make the delays self-configurable but setting an
> upper limit is the right thing to do.
>
>>
>>> +static void mt_svc_restart(struct tevent_context *ev,
>>> + struct tevent_timer *te,
>>> + struct timeval t, void *ptr)
>>> +{
>>> + struct mt_svc *svc;
>>> +
>>> + svc = talloc_get_type(ptr, struct mt_svc);
>>> + if (svc == NULL) {
>>> + return;
>>> + }
>>> +
>>> + DEBUG(SSSDBG_TRACE_FUNC,
>>> + ("Scheduling a service for restart %d\n", svc->restarts));
>>
>> It should say svc->restarts + 1. Can you also add a service name to the
>> message?
>
> Done and done. Thank you for the review.
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
> + /* 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;
}
More information about the sssd-devel
mailing list