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;
}

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to