[SSSD] [PATCH] Exit offline mode only if server is available.
Michal Židek
mzidek at redhat.com
Wed Jul 30 14:59:44 UTC 2014
On 07/29/2014 03:29 PM, Jakub Hrozek wrote:
>
> So far I only read the patches, no testing yet, see some comments
> inline:
>
>> From 0724c54b3e4ff73b091afed86122cdf8dbc746ea Mon Sep 17 00:00:00 2001
>> From: Michal Zidek <mzidek at redhat.com>
>> Date: Mon, 7 Jul 2014 16:46:18 +0200
>> Subject: [PATCH 1/3] ptask: Allow adding random_offset to scheduled execution
>> time
>
> ACK
>
>> From bf9d6aebda21397278b5a3f782efaa1a538992cb Mon Sep 17 00:00:00 2001
>> From: Michal Zidek <mzidek at redhat.com>
>> Date: Mon, 7 Jul 2014 17:45:45 +0200
>> Subject: [PATCH 2/3] ptask: Add backoff feature to the ptask api.
>
> I would prefer if the backoff was a parameter to the create function,
> creating would better be atomic.
Ok.
>
>> From fd85d23e9721293adaaaba9927947d31ab68da8a Mon Sep 17 00:00:00 2001
>> From: Michal Zidek <mzidek at redhat.com>
>> Date: Thu, 3 Jul 2014 12:49:20 +0200
>> Subject: [PATCH 3/3] Exit offline mode only if server is available.
>>
>> This patch adds periodic check to test if
>> backend can exit offline mode and only
>> marks backend as not offline if server
>> for the service is available.
>>
>> Prior to this patch we marked backend as not
>> offline if the offline_timeout was reached
>> without checking for the server availability
>> and when the next request failed again
>> we switched back to the offline mode.
>> This caused significant slowdowns in some
>> edge cases.
>>
>> Fixes:
>> https://fedorahosted.org/sssd/ticket/2355
>> ---
>> src/providers/data_provider_be.c | 68 ++++++++++++++++++++++++++++++----------
>> src/providers/dp_backend.h | 5 +++
>> 2 files changed, 56 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
>> index 4c3eaa6..76b7f82 100644
>> --- a/src/providers/data_provider_be.c
>> +++ b/src/providers/data_provider_be.c
>> @@ -450,35 +450,68 @@ static void be_queue_next_request(struct be_req *be_req, enum bet_type type)
>>
>> bool be_is_offline(struct be_ctx *ctx)
>> {
>> - time_t now = time(NULL);
>> - int offline_timeout;
>> - int ret;
>> + return ctx->offstat.offline;
>> +}
>>
>> - /* check if we are past the offline blackout timeout */
>> - ret = confdb_get_int(ctx->cdb, ctx->conf_path,
>> - CONFDB_DOMAIN_OFFLINE_TIMEOUT, 60,
>> - &offline_timeout);
>> - if (ret != EOK) {
>> - DEBUG(SSSDBG_MINOR_FAILURE,
>> - "Failed to get offline_timeout from confdb. "
>> - "Using default value (60 seconds)\n");
>> - offline_timeout = 60;
>> - }
>> +static void check_if_online(struct be_ctx *ctx);
>>
>> - if (ctx->offstat.went_offline + offline_timeout < now) {
>> - ctx->offstat.offline = false;
>> - }
>> +errno_t
>> +try_to_go_online(TALLOC_CTX *mem_ctx,
>> + struct tevent_context *ev,
>> + struct be_ctx *be_ctx,
>> + struct be_ptask *be_ptask,
>> + void *be_ctx_void)
>> +{
>
> Can you make this function static?
Done.
>
>> + struct be_ctx *ctx = (struct be_ctx*) be_ctx_void;
>>
>> - return ctx->offstat.offline;
>> + check_if_online(ctx);
>> + return EOK;
>> }
>>
>> void be_mark_offline(struct be_ctx *ctx)
>> {
>> + int offline_timeout;
>> DEBUG(SSSDBG_TRACE_INTERNAL, "Going offline!\n");
>>
>> ctx->offstat.went_offline = time(NULL);
>> ctx->offstat.offline = true;
>> ctx->run_online_cb = true;
>> + errno_t ret;
>> +
>> + if (ctx->online_check_ptask_initialized == false) {
>
> Can you use just:
> if (ctx->check_if_online_ptask == NULL)
>
> ?
>
> If not, then instead of another structure member, isn't it more readable
> to add a new function be_ptask_is_enabled() and then you could call:
> if (ctx->check_if_online_ptask &&
> be_ptask_is_enabled(ctx->check_if_online_ptask)) {
> }
>
> The andvantage is that you don't have to keep track of anything
> separately and all the state info is kept in the ptask itself.
Ok. I'll go with the NULL check then.
New patches are attached.
Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ptask-Allow-adding-random_offset-to-scheduled-execut.patch
Type: text/x-patch
Size: 6703 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140730/89d12980/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-ptask-Add-backoff-feature-to-the-ptask-api.patch
Type: text/x-patch
Size: 7051 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140730/89d12980/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Exit-offline-mode-only-if-server-is-available.patch
Type: text/x-patch
Size: 4806 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140730/89d12980/attachment-0002.bin>
More information about the sssd-devel
mailing list