From 70d477efb1a43b3e1750b4aca868dadc2ed435d5 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 18 Jun 2019 20:49:00 +0200 Subject: [PATCH 55/64] BE: Extend be_ptask_create() with control when to schedule next run after success Related: https://pagure.io/SSSD/sssd/issue/4012 be_ptask_create() used to always schedule the next periodical run "period" seconds after the previous run started. This is great for tasks that are short-lived like DNS updates because we know they will be executed really with the configured period. But the background refresh task can potentially take a very long time in which case the next run could have been scheduled almost immediately and as a result sssd_be would always be quite busy. It is better to have the option to schedule the next task period seconds after the last run has finished. This can lead to some inconsistency, but we can warn the admin about that. This patch so far does not change any of the existing calls to be_ptask_create(), just adds BE_PTASK_SCHEDULE_FROM_LAST as an additional parameter. Reviewed-by: Sumit Bose (cherry picked from commit 0fbc317ac7f1fe13cd41364c67db7d7a19d7d546) Reviewed-by: Sumit Bose --- src/providers/ad/ad_machine_pw_renewal.c | 4 +- src/providers/ad/ad_subdomains.c | 4 +- src/providers/be_ptask.c | 10 ++-- src/providers/be_ptask.h | 24 ++++++++- src/providers/be_ptask_private.h | 1 + src/providers/be_refresh.c | 4 +- src/providers/ipa/ipa_subdomains.c | 4 +- src/providers/ldap/ldap_id_enum.c | 1 + src/providers/ldap/sdap_sudo_shared.c | 8 ++- src/tests/cmocka/test_be_ptask.c | 62 ++++++++++++++++-------- 10 files changed, 89 insertions(+), 33 deletions(-) diff --git a/src/providers/ad/ad_machine_pw_renewal.c b/src/providers/ad/ad_machine_pw_renewal.c index 5b6ba26b7..47941dfbf 100644 --- a/src/providers/ad/ad_machine_pw_renewal.c +++ b/src/providers/ad/ad_machine_pw_renewal.c @@ -382,7 +382,9 @@ errno_t ad_machine_account_password_renewal_init(struct be_ctx *be_ctx, } ret = be_ptask_create(be_ctx, be_ctx, period, initial_delay, 0, 0, 60, - BE_PTASK_OFFLINE_DISABLE, 0, + BE_PTASK_OFFLINE_DISABLE, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, ad_machine_account_password_renewal_send, ad_machine_account_password_renewal_recv, renewal_data, diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c index e3e3d3ece..45a8fe0fc 100644 --- a/src/providers/ad/ad_subdomains.c +++ b/src/providers/ad/ad_subdomains.c @@ -2111,7 +2111,9 @@ errno_t ad_subdomains_init(TALLOC_CTX *mem_ctx, period = be_ctx->domain->subdomain_refresh_interval; ret = be_ptask_create(sd_ctx, be_ctx, period, 0, 0, 0, period, - BE_PTASK_OFFLINE_DISABLE, 0, + BE_PTASK_OFFLINE_DISABLE, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, ad_subdomains_ptask_send, ad_subdomains_ptask_recv, sd_ctx, "Subdomains Refresh", NULL); if (ret != EOK) { diff --git a/src/providers/be_ptask.c b/src/providers/be_ptask.c index c43351755..32d9a03ce 100644 --- a/src/providers/be_ptask.c +++ b/src/providers/be_ptask.c @@ -30,11 +30,6 @@ #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, @@ -182,7 +177,7 @@ static void be_ptask_done(struct tevent_req *req) DEBUG(SSSDBG_TRACE_FUNC, "Task [%s]: finished successfully\n", task->name); - be_ptask_schedule(task, BE_PTASK_PERIOD, BE_PTASK_SCHEDULE_FROM_LAST); + be_ptask_schedule(task, BE_PTASK_PERIOD, task->success_schedule_type); break; default: DEBUG(SSSDBG_OP_FAILURE, "Task [%s]: failed with [%d]: %s\n", @@ -268,6 +263,7 @@ errno_t be_ptask_create(TALLOC_CTX *mem_ctx, time_t random_offset, time_t timeout, enum be_ptask_offline offline, + enum be_ptask_schedule success_schedule_type, time_t max_backoff, be_ptask_send_t send_fn, be_ptask_recv_t recv_fn, @@ -300,6 +296,7 @@ errno_t be_ptask_create(TALLOC_CTX *mem_ctx, task->max_backoff = max_backoff; task->timeout = timeout; task->offline = offline; + task->success_schedule_type = success_schedule_type; task->send_fn = send_fn; task->recv_fn = recv_fn; task->pvt = pvt; @@ -470,6 +467,7 @@ errno_t be_ptask_create_sync(TALLOC_CTX *mem_ctx, ret = be_ptask_create(mem_ctx, be_ctx, period, first_delay, enabled_delay, random_offset, timeout, offline, + BE_PTASK_SCHEDULE_FROM_LAST, max_backoff, be_ptask_sync_send, be_ptask_sync_recv, ctx, name, _task); if (ret != EOK) { diff --git a/src/providers/be_ptask.h b/src/providers/be_ptask.h index 3b9755361..c23278e88 100644 --- a/src/providers/be_ptask.h +++ b/src/providers/be_ptask.h @@ -46,6 +46,19 @@ enum be_ptask_offline { BE_PTASK_OFFLINE_EXECUTE }; +/** + * Defines the starting point for scheduling a task + */ +enum be_ptask_schedule { + /* Schedule starting from now, typically this is used when scheduling + * relative to the finish time + */ + BE_PTASK_SCHEDULE_FROM_NOW, + /* Schedule relative to the start time of the task + */ + BE_PTASK_SCHEDULE_FROM_LAST +}; + typedef struct tevent_req * (*be_ptask_send_t)(TALLOC_CTX *mem_ctx, struct tevent_context *ev, @@ -75,6 +88,14 @@ typedef errno_t * The first execution is scheduled first_delay seconds after the task is * created. * + * Subsequent runs will be scheduled depending on the value of the + * success_schedule_type parameter: + * - BE_PTASK_SCHEDULE_FROM_NOW: period seconds from the finish time + * - BE_PTASK_SCHEDULE_FROM_LAST: period seconds from the last start time + * + * If the test fails, another run is always scheduled period seconds + * from the finish time. + * * If request does not complete in timeout seconds, it will be * cancelled and rescheduled to 'now + period'. * @@ -83,7 +104,7 @@ typedef errno_t * * The random_offset is maximum number of seconds added to the * expected delay. Set to 0 if no randomization is needed. - + * * If max_backoff is not 0 then the period is doubled * every time the task is scheduled. The maximum value of * period is max_backoff. The value of period will be reset to @@ -100,6 +121,7 @@ errno_t be_ptask_create(TALLOC_CTX *mem_ctx, time_t random_offset, time_t timeout, enum be_ptask_offline offline, + enum be_ptask_schedule success_schedule_type, time_t max_backoff, be_ptask_send_t send_fn, be_ptask_recv_t recv_fn, diff --git a/src/providers/be_ptask_private.h b/src/providers/be_ptask_private.h index 4144a3938..e89105f95 100644 --- a/src/providers/be_ptask_private.h +++ b/src/providers/be_ptask_private.h @@ -32,6 +32,7 @@ struct be_ptask { time_t timeout; time_t max_backoff; enum be_ptask_offline offline; + enum be_ptask_schedule success_schedule_type; be_ptask_send_t send_fn; be_ptask_recv_t recv_fn; void *pvt; diff --git a/src/providers/be_refresh.c b/src/providers/be_refresh.c index 5d86509bb..50b023c3d 100644 --- a/src/providers/be_refresh.c +++ b/src/providers/be_refresh.c @@ -156,7 +156,9 @@ errno_t be_refresh_ctx_init(struct be_ctx *be_ctx, refresh_interval = be_ctx->domain->refresh_expired_interval; if (refresh_interval > 0) { ret = be_ptask_create(be_ctx, be_ctx, refresh_interval, 30, 5, 0, - refresh_interval, BE_PTASK_OFFLINE_SKIP, 0, + refresh_interval, BE_PTASK_OFFLINE_SKIP, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, be_refresh_send, be_refresh_recv, ctx, "Refresh Records", NULL); if (ret != EOK) { diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c index 94365aaca..3a17c851d 100644 --- a/src/providers/ipa/ipa_subdomains.c +++ b/src/providers/ipa/ipa_subdomains.c @@ -3134,7 +3134,9 @@ errno_t ipa_subdomains_init(TALLOC_CTX *mem_ctx, period = be_ctx->domain->subdomain_refresh_interval; ret = be_ptask_create(sd_ctx, be_ctx, period, ptask_first_delay, 0, 0, period, - BE_PTASK_OFFLINE_DISABLE, 0, + BE_PTASK_OFFLINE_DISABLE, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, ipa_subdomains_ptask_send, ipa_subdomains_ptask_recv, sd_ctx, "Subdomains Refresh", NULL); if (ret != EOK) { diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c index 8832eb558..062185c55 100644 --- a/src/providers/ldap/ldap_id_enum.c +++ b/src/providers/ldap/ldap_id_enum.c @@ -99,6 +99,7 @@ errno_t ldap_setup_enumeration(struct be_ctx *be_ctx, 0, /* random offset */ period, /* timeout */ BE_PTASK_OFFLINE_SKIP, + BE_PTASK_SCHEDULE_FROM_LAST, 0, /* max_backoff */ send_fn, recv_fn, ectx, "enumeration", &sdom->enum_task); diff --git a/src/providers/ldap/sdap_sudo_shared.c b/src/providers/ldap/sdap_sudo_shared.c index 66b788702..982fdbf7f 100644 --- a/src/providers/ldap/sdap_sudo_shared.c +++ b/src/providers/ldap/sdap_sudo_shared.c @@ -90,7 +90,9 @@ sdap_sudo_ptask_setup_generic(struct be_ctx *be_ctx, * when offline. */ if (full > 0) { ret = be_ptask_create(be_ctx, be_ctx, full, delay, 0, 0, full, - BE_PTASK_OFFLINE_DISABLE, 0, + BE_PTASK_OFFLINE_DISABLE, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, full_send_fn, full_recv_fn, pvt, "SUDO Full Refresh", NULL); if (ret != EOK) { @@ -107,7 +109,9 @@ sdap_sudo_ptask_setup_generic(struct be_ctx *be_ctx, * when offline. */ if (smart > 0) { ret = be_ptask_create(be_ctx, be_ctx, smart, delay + smart, smart, 0, - smart, BE_PTASK_OFFLINE_DISABLE, 0, + smart, BE_PTASK_OFFLINE_DISABLE, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, smart_send_fn, smart_recv_fn, pvt, "SUDO Smart Refresh", NULL); if (ret != EOK) { diff --git a/src/tests/cmocka/test_be_ptask.c b/src/tests/cmocka/test_be_ptask.c index ca80b5442..45c41ed58 100644 --- a/src/tests/cmocka/test_be_ptask.c +++ b/src/tests/cmocka/test_be_ptask.c @@ -306,7 +306,8 @@ void test_be_ptask_create_einval_be(void **state) errno_t ret; ret = be_ptask_create(test_ctx, NULL, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, NULL, "Test ptask", &ptask); assert_int_equal(ret, EINVAL); assert_null(ptask); @@ -319,7 +320,8 @@ void test_be_ptask_create_einval_period(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, 0, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, NULL, "Test ptask", &ptask); assert_int_equal(ret, EINVAL); assert_null(ptask); @@ -332,7 +334,8 @@ void test_be_ptask_create_einval_send(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, NULL, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, NULL, test_be_ptask_recv, NULL, "Test ptask", &ptask); assert_int_equal(ret, EINVAL); assert_null(ptask); @@ -345,7 +348,8 @@ void test_be_ptask_create_einval_recv(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, NULL, NULL, "Test ptask", &ptask); assert_int_equal(ret, EINVAL); assert_null(ptask); @@ -358,7 +362,8 @@ void test_be_ptask_create_einval_name(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, NULL, NULL, &ptask); assert_int_equal(ret, EINVAL); assert_null(ptask); @@ -373,7 +378,8 @@ void test_be_ptask_create_no_delay(void **state) now = get_current_time(); ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -400,7 +406,8 @@ void test_be_ptask_create_first_delay(void **state) now = get_current_time(); ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, DELAY, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -425,7 +432,8 @@ void test_be_ptask_disable(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -449,7 +457,8 @@ void test_be_ptask_enable(void **state) now = get_current_time(); ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -481,7 +490,8 @@ void test_be_ptask_enable_delay(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, DELAY, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -520,7 +530,8 @@ void test_be_ptask_offline_skip(void **state) now = get_current_time(); ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -553,7 +564,9 @@ void test_be_ptask_offline_disable(void **state) will_return(be_add_offline_cb, test_ctx); ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_DISABLE, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_DISABLE, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -583,7 +596,9 @@ void test_be_ptask_offline_execute(void **state) mark_offline(test_ctx); ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_EXECUTE, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_EXECUTE, + BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -610,7 +625,8 @@ void test_be_ptask_reschedule_ok(void **state) now = get_current_time(); ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -641,7 +657,8 @@ void test_be_ptask_reschedule_null(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_null_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_null_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); @@ -668,7 +685,8 @@ void test_be_ptask_reschedule_error(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_error_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); @@ -695,7 +713,8 @@ void test_be_ptask_reschedule_timeout(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 1, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_timeout_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_timeout_send, test_be_ptask_error_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); @@ -732,7 +751,8 @@ void test_be_ptask_reschedule_backoff(void **state) now_first = get_current_time(); 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, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 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); @@ -786,7 +806,8 @@ void test_be_ptask_get_period(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, 0, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); @@ -806,7 +827,8 @@ void test_be_ptask_get_timeout(void **state) errno_t ret; ret = be_ptask_create(test_ctx, test_ctx->be_ctx, PERIOD, 0, 0, 0, TIMEOUT, - BE_PTASK_OFFLINE_SKIP, 0, test_be_ptask_send, + BE_PTASK_OFFLINE_SKIP, BE_PTASK_SCHEDULE_FROM_LAST, + 0, test_be_ptask_send, test_be_ptask_recv, test_ctx, "Test ptask", &ptask); assert_int_equal(ret, ERR_OK); assert_non_null(ptask); -- 2.20.1