be_ptask: let backoff affect only period

With this patch the first and enabled delay values are respected.

Reviewed-by: Michal Židek <mzidek@redhat.com>

3 files changed, 154 insertions, 32 deletions

diff --git a/src/providers/dp_ptask.c b/src/providers/dp_ptask.c
index 3540579e8..b5ab79b1d 100644
--- a/src/providers/dp_ptask.c
+++ b/src/providers/dp_ptask.c
@@ -28,13 +28,21 @@
 #include "providers/dp_ptask_private.h"
 #include "providers/dp_ptask.h"
 
+#define backoff_allowed(ptask) (ptask->max_backoff != 0)
+
 enum be_ptask_schedule {
     BE_PTASK_SCHEDULE_FROM_NOW,
     BE_PTASK_SCHEDULE_FROM_LAST
 };
 
+enum be_ptask_delay {
+    BE_PTASK_FIRST_DELAY,
+    BE_PTASK_ENABLED_DELAY,
+    BE_PTASK_PERIOD
+};
+
 static void be_ptask_schedule(struct be_ptask *task,
-                              time_t delay,
+                              enum be_ptask_delay delay_type,
                               enum be_ptask_schedule from);
 
 static int be_ptask_destructor(void *pvt)
@@ -86,7 +94,7 @@ static void be_ptask_timeout(struct tevent_context *ev,
     DEBUG(SSSDBG_OP_FAILURE, "Task [%s]: timed out\n", task->name);
 
     talloc_zfree(task->req);
-    be_ptask_schedule(task, task->period, BE_PTASK_SCHEDULE_FROM_NOW);
+    be_ptask_schedule(task, BE_PTASK_PERIOD, BE_PTASK_SCHEDULE_FROM_NOW);
 }
 
 static void be_ptask_done(struct tevent_req *req);
@@ -106,7 +114,8 @@ static void be_ptask_execute(struct tevent_context *ev,
         DEBUG(SSSDBG_TRACE_FUNC, "Back end is offline\n");
         switch (task->offline) {
         case BE_PTASK_OFFLINE_SKIP:
-            be_ptask_schedule(task, task->period, BE_PTASK_SCHEDULE_FROM_NOW);
+            be_ptask_schedule(task, BE_PTASK_PERIOD,
+                              BE_PTASK_SCHEDULE_FROM_NOW);
             return;
         case BE_PTASK_OFFLINE_DISABLE:
             /* This case is normally handled by offline callback but we
@@ -131,7 +140,7 @@ static void be_ptask_execute(struct tevent_context *ev,
         DEBUG(SSSDBG_OP_FAILURE, "Task [%s]: failed to execute task, "
               "will try again later\n", task->name);
 
-        be_ptask_schedule(task, task->period, BE_PTASK_SCHEDULE_FROM_NOW);
+        be_ptask_schedule(task, BE_PTASK_PERIOD, BE_PTASK_SCHEDULE_FROM_NOW);
         return;
     }
 
@@ -150,7 +159,8 @@ static void be_ptask_execute(struct tevent_context *ev,
             DEBUG(SSSDBG_OP_FAILURE, "Task [%s]: failed to set timeout, "
                   "the task will be rescheduled\n", task->name);
 
-            be_ptask_schedule(task, task->period, BE_PTASK_SCHEDULE_FROM_NOW);
+            be_ptask_schedule(task, BE_PTASK_PERIOD,
+                              BE_PTASK_SCHEDULE_FROM_NOW);
         }
     }
 
@@ -172,39 +182,47 @@ static void be_ptask_done(struct tevent_req *req)
         DEBUG(SSSDBG_TRACE_FUNC, "Task [%s]: finished successfully\n",
                                   task->name);
 
-        be_ptask_schedule(task, task->period, BE_PTASK_SCHEDULE_FROM_LAST);
+        be_ptask_schedule(task, BE_PTASK_PERIOD, BE_PTASK_SCHEDULE_FROM_LAST);
         break;
     default:
         DEBUG(SSSDBG_OP_FAILURE, "Task [%s]: failed with [%d]: %s\n",
                                   task->name, ret, sss_strerror(ret));
 
-        be_ptask_schedule(task, task->period, BE_PTASK_SCHEDULE_FROM_NOW);
+        be_ptask_schedule(task, BE_PTASK_PERIOD, BE_PTASK_SCHEDULE_FROM_NOW);
         break;
     }
 }
 
 static void be_ptask_schedule(struct be_ptask *task,
-                              time_t delay,
+                              enum be_ptask_delay delay_type,
                               enum be_ptask_schedule from)
 {
     struct timeval tv;
+    time_t delay = 0;
 
     if (!task->enabled) {
         DEBUG(SSSDBG_TRACE_FUNC, "Task [%s]: disabled\n", task->name);
         return;
     }
 
-    if (task->allow_backoff) {
-        if (task->backoff_delay == 0) {
-            task->backoff_delay = task->period;
-        } else {
-            task->backoff_delay = (task->backoff_delay * 2 > task->max_backoff)
-                                  ? task->max_backoff
-                                  : task->backoff_delay * 2;
-            delay = task->backoff_delay;
+    switch (delay_type) {
+    case BE_PTASK_FIRST_DELAY:
+        delay = task->first_delay;
+        break;
+    case BE_PTASK_ENABLED_DELAY:
+        delay = task->enabled_delay;
+        break;
+    case BE_PTASK_PERIOD:
+        delay = task->period;
+
+        if (backoff_allowed(task) && task->period * 2 <= task->max_backoff) {
+            /* double the period for the next execution */
+            task->period *= 2;
         }
+        break;
     }
 
+    /* add random offset */
     if (task->random_offset != 0) {
         delay = delay + (rand_r(&task->ro_seed) % task->random_offset);
     }
@@ -274,9 +292,12 @@ errno_t be_ptask_create(TALLOC_CTX *mem_ctx,
     task->ev = be_ctx->ev;
     task->be_ctx = be_ctx;
     task->period = period;
+    task->orig_period = period;
+    task->first_delay = first_delay;
     task->enabled_delay = enabled_delay;
     task->random_offset = random_offset;
     task->ro_seed = time(NULL) * getpid();
+    task->max_backoff = max_backoff;
     task->timeout = timeout;
     task->offline = offline;
     task->send_fn = send_fn;
@@ -288,11 +309,6 @@ errno_t be_ptask_create(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    if (max_backoff != 0) {
-        task->max_backoff = max_backoff;
-        task->allow_backoff = true;
-    }
-
     task->enabled = true;
 
     talloc_set_destructor((TALLOC_CTX*)task, be_ptask_destructor);
@@ -316,7 +332,7 @@ errno_t be_ptask_create(TALLOC_CTX *mem_ctx,
 
     DEBUG(SSSDBG_TRACE_FUNC, "Periodic task [%s] was created\n", task->name);
 
-    be_ptask_schedule(task, first_delay, BE_PTASK_SCHEDULE_FROM_NOW);
+    be_ptask_schedule(task, BE_PTASK_FIRST_DELAY, BE_PTASK_SCHEDULE_FROM_NOW);
 
     if (_task != NULL) {
         *_task = task;
@@ -343,7 +359,7 @@ void be_ptask_enable(struct be_ptask *task)
     DEBUG(SSSDBG_TRACE_FUNC, "Task [%s]: enabling task\n", task->name);
 
     task->enabled = true;
-    be_ptask_schedule(task, task->enabled_delay, BE_PTASK_SCHEDULE_FROM_NOW);
+    be_ptask_schedule(task, BE_PTASK_ENABLED_DELAY, BE_PTASK_SCHEDULE_FROM_NOW);
 }
 
 /* Disable the task, but if a request already in progress, let it finish. */
@@ -353,10 +369,7 @@ void be_ptask_disable(struct be_ptask *task)
 
     talloc_zfree(task->timer);
     task->enabled = false;
-
-    if (task->allow_backoff) {
-        task->backoff_delay = 0;
-    }
+    task->period = task->orig_period;
 }
 
 void be_ptask_destroy(struct be_ptask **task)
diff --git a/src/providers/dp_ptask_private.h b/src/providers/dp_ptask_private.h
index 9c97fae7b..4144a3938 100644
--- a/src/providers/dp_ptask_private.h
+++ b/src/providers/dp_ptask_private.h
@@ -24,20 +24,20 @@
 struct be_ptask {
     struct tevent_context *ev;
     struct be_ctx *be_ctx;
-    time_t period;
+    time_t orig_period;
+    time_t first_delay;
     time_t enabled_delay;
     time_t random_offset;
     unsigned int ro_seed;
     time_t timeout;
-    bool allow_backoff;
     time_t max_backoff;
-    time_t backoff_delay;
     enum be_ptask_offline offline;
     be_ptask_send_t send_fn;
     be_ptask_recv_t recv_fn;
     void *pvt;
     const char *name;
 
+    time_t period;          /* computed period */
     time_t next_execution;  /* next time when the task is scheduled */
     time_t last_execution;  /* last time when send was called */
     struct tevent_req *req; /* active tevent request */
diff --git a/src/tests/cmocka/test_be_ptask.c b/src/tests/cmocka/test_be_ptask.c
index e06cc90c3..6155002ab 100644
--- a/src/tests/cmocka/test_be_ptask.c
+++ b/src/tests/cmocka/test_be_ptask.c
@@ -31,7 +31,7 @@
 #include "tests/common.h"
 
 #define DELAY  2
-#define PERIOD 10
+#define PERIOD 1
 
 #define new_test(test) \
     unit_test_setup_teardown(test_ ## test, test_setup, test_teardown)
@@ -702,6 +702,59 @@ void test_be_ptask_reschedule_timeout(void **state)
     assert_null(ptask);
 }
 
+void test_be_ptask_reschedule_backoff(void **state)
+{
+    struct test_ctx *test_ctx = (struct test_ctx *)(*state);
+    struct be_ptask *ptask = NULL;
+    time_t next_execution;
+    time_t now;
+    errno_t ret;
+
+    now = time(NULL);
+    ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0,
+                          BE_PTASK_OFFLINE_SKIP, PERIOD*2, test_be_ptask_send,
+                          test_be_ptask_recv, test_ctx, "Test ptask", &ptask);
+    assert_int_equal(ret, ERR_OK);
+    assert_non_null(ptask);
+    assert_non_null(ptask->timer);
+
+    /* first run */
+    next_execution = ptask->next_execution;
+
+    while (!test_ctx->done) {
+        tevent_loop_once(test_ctx->be_ctx->ev);
+    }
+
+    assert_true(now <= ptask->last_execution);
+    assert_true(now <= test_ctx->when);
+    assert_true(ptask->last_execution <= test_ctx->when);
+
+    assert_true(next_execution + PERIOD <= ptask->next_execution);
+    assert_int_equal(PERIOD*2, ptask->period);
+    assert_non_null(ptask->timer);
+
+    test_ctx->done = false;
+
+    /* second run */
+    now = time(NULL);
+    next_execution = ptask->next_execution;
+
+    while (!test_ctx->done) {
+        tevent_loop_once(test_ctx->be_ctx->ev);
+    }
+
+    assert_true(now + PERIOD <= ptask->last_execution);
+    assert_true(now + PERIOD <= test_ctx->when);
+    assert_true(ptask->last_execution <= test_ctx->when);
+
+    assert_true(next_execution + PERIOD*2 <= ptask->next_execution);
+    assert_int_equal(PERIOD*2, ptask->period);
+    assert_non_null(ptask->timer);
+
+    be_ptask_destroy(&ptask);
+    assert_null(ptask);
+}
+
 void test_be_ptask_get_period(void **state)
 {
     struct test_ctx *test_ctx = (struct test_ctx *)(*state);
@@ -809,6 +862,60 @@ void test_be_ptask_sync_reschedule_error(void **state)
     assert_null(ptask);
 }
 
+void test_be_ptask_sync_reschedule_backoff(void **state)
+{
+    struct test_ctx *test_ctx = (struct test_ctx *)(*state);
+    struct be_ptask *ptask = NULL;
+    time_t next_execution;
+    time_t now;
+    errno_t ret;
+
+    now = time(NULL);
+    ret = be_ptask_create_sync(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0,
+                               BE_PTASK_OFFLINE_SKIP, PERIOD*2,
+                               test_be_ptask_sync_error,
+                               test_ctx, "Test ptask", &ptask);
+    assert_int_equal(ret, ERR_OK);
+    assert_non_null(ptask);
+    assert_non_null(ptask->timer);
+
+    /* first run */
+    next_execution = ptask->next_execution;
+
+    while (!is_sync_ptask_finished(test_ctx, ptask)) {
+        tevent_loop_once(test_ctx->be_ctx->ev);
+    }
+
+    assert_true(now <= ptask->last_execution);
+    assert_true(now <= test_ctx->when);
+    assert_true(ptask->last_execution <= test_ctx->when);
+
+    assert_true(next_execution + PERIOD <= ptask->next_execution);
+    assert_int_equal(PERIOD*2, ptask->period);
+    assert_non_null(ptask->timer);
+
+    test_ctx->done = false;
+
+    /* second run */
+    now = time(NULL);
+    next_execution = ptask->next_execution;
+
+    while (!is_sync_ptask_finished(test_ctx, ptask)) {
+        tevent_loop_once(test_ctx->be_ctx->ev);
+    }
+
+    assert_true(now + PERIOD <= ptask->last_execution);
+    assert_true(now + PERIOD <= test_ctx->when);
+    assert_true(ptask->last_execution <= test_ctx->when);
+
+    assert_true(next_execution + PERIOD*2 <= ptask->next_execution);
+    assert_int_equal(PERIOD*2, ptask->period);
+    assert_non_null(ptask->timer);
+
+    be_ptask_destroy(&ptask);
+    assert_null(ptask);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -837,10 +944,12 @@ int main(int argc, const char *argv[])
         new_test(be_ptask_reschedule_null),
         new_test(be_ptask_reschedule_error),
         new_test(be_ptask_reschedule_timeout),
+        new_test(be_ptask_reschedule_backoff),
         new_test(be_ptask_get_period),
         new_test(be_ptask_create_sync),
         new_test(be_ptask_sync_reschedule_ok),
-        new_test(be_ptask_sync_reschedule_error)
+        new_test(be_ptask_sync_reschedule_error),
+        new_test(be_ptask_sync_reschedule_backoff)
     };
 
     /* Set debug level to invalid value so we can deside if -d 0 was used. */