From 834e36dd54eb83a8265d7db3b54d9ce536950d67 Mon Sep 17 00:00:00 2001
From: Duncan Eastoe <duncan.eastoe@att.com>
Date: Tue, 21 Jul 2020 17:11:33 +0100
Subject: [PATCH 1/4] data_provider_be: Configurable max offline time

The offline_timeout period determines the minimum time between
attempts of a data provider to go back online, if it is offline due
to eg. unreachable servers. Each time this check fails there is a
backoff factor applied meaning there can be up to 60 minutes between
these attempts.

Here we introduce the offline_timeout_max option which allows the
the maximum period between attempts to be defined in the configuration,
instead of the default 60 minutes; therefore providing more
flexibility.

Setting offline_timeout_max to 0 disables the backoff functionality.
---
 src/confdb/confdb.h              |  1 +
 src/config/SSSDConfigTest.py     |  2 ++
 src/config/cfg_rules.ini         |  1 +
 src/config/etc/sssd.api.conf     |  1 +
 src/man/sssd.conf.5.xml          | 28 +++++++++++++++++++++++++---
 src/providers/data_provider_be.c | 25 ++++++++++++++++++++++++-
 6 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index c96896da5d..b7fad6285a 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -243,6 +243,7 @@
 #define CONFDB_DOMAIN_PWD_EXPIRATION_WARNING "pwd_expiration_warning"
 #define CONFDB_DOMAIN_REFRESH_EXPIRED_INTERVAL "refresh_expired_interval"
 #define CONFDB_DOMAIN_OFFLINE_TIMEOUT "offline_timeout"
+#define CONFDB_DOMAIN_OFFLINE_TIMEOUT_MAX "offline_timeout_max"
 #define CONFDB_DOMAIN_SUBDOMAIN_INHERIT "subdomain_inherit"
 #define CONFDB_DOMAIN_CACHED_AUTH_TIMEOUT "cached_auth_timeout"
 #define CONFDB_DOMAIN_TYPE "domain_type"
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 73c78d263f..323be5ed3c 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -593,6 +593,7 @@ def testListOptions(self):
             'max_id',
             'timeout',
             'offline_timeout',
+            'offline_timeout_max',
             'command',
             'enumerate',
             'cache_credentials',
@@ -969,6 +970,7 @@ def testRemoveProvider(self):
             'max_id',
             'timeout',
             'offline_timeout',
+            'offline_timeout_max',
             'command',
             'enumerate',
             'cache_credentials',
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 2874ea048b..159f49140e 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -369,6 +369,7 @@ option = timeout
 option = enumerate
 option = subdomain_enumerate
 option = offline_timeout
+option = offline_timeout_max
 option = cache_credentials
 option = cache_credentials_minimal_first_factor_length
 option = use_fully_qualified_names
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 035c33cad8..bbe0d8cc0e 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -166,6 +166,7 @@ timeout = int, None, false
 enumerate = bool, None, false
 subdomain_enumerate = str, None, false
 offline_timeout = int, None, false
+offline_timeout_max = int, None, false
 cache_credentials = bool, None, false
 cache_credentials_minimal_first_factor_length = int, None, false
 use_fully_qualified_names = bool, None, false
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 874a09c494..c50567b37a 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -783,15 +783,37 @@
                         </para>
                         <para>
                             Note that the maximum length of each interval
-                            is currently limited to one hour. If the
-                            calculated length of new_interval is greater
-                            than an hour, it will be forced to one hour.
+                            is defined by offline_timeout_max, which defaults
+                            to one hour. If the calculated length of new_interval
+                            is greater than offline_timeout_max, it will be forced
+                            to the offline_timeout_max value.
                         </para>
                         <para>
                             Default: 60
                         </para>
                     </listitem>
                 </varlistentry>
+                <varlistentry>
+                    <term>offline_timeout_max (integer)</term>
+                    <listitem>
+                        <para>
+                            Controls by how much the time between attempts to go
+                            online can be incremented following unsuccessful
+                            attempts to go online.
+                        </para>
+                        <para>
+                            A value of 0 disables the incrementing behaviour.
+                        </para>
+                        <para>
+                            Although a value between 0 and offline_timeout may be
+                            specified, it has the effect of overriding the
+                            offline_timeout value so is of little use.
+                        </para>
+                        <para>
+                            Default: 3600
+                        </para>
+                    </listitem>
+                </varlistentry>
                 <varlistentry>
                     <term>responder_idle_timeout</term>
                     <listitem>
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 95665332a7..ab47702900 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -51,6 +51,8 @@
 #define ONLINE_CB_RETRY 3
 #define ONLINE_CB_RETRY_MAX_DELAY 4
 
+#define OFFLINE_TIMEOUT_MAX_DEFAULT 3600
+
 /* sssd.service */
 static errno_t
 data_provider_res_init(TALLOC_CTX *mem_ctx,
@@ -110,9 +112,29 @@ static int get_offline_timeout(struct be_ctx *ctx)
     return offline_timeout;
 }
 
+static int get_offline_timeout_max(struct be_ctx *ctx)
+{
+    int offline_timeout_max;
+    errno_t ret;
+
+    ret = confdb_get_int(ctx->cdb, ctx->conf_path,
+                         CONFDB_DOMAIN_OFFLINE_TIMEOUT_MAX,
+                         OFFLINE_TIMEOUT_MAX_DEFAULT,
+                         &offline_timeout_max);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_CONF_SETTINGS,
+              "Failed to get offline_timeout_max from confdb. "
+              "Will use %d seconds.\n", OFFLINE_TIMEOUT_MAX_DEFAULT);
+        offline_timeout_max = OFFLINE_TIMEOUT_MAX_DEFAULT;
+    }
+
+    return offline_timeout_max;
+}
+
 void be_mark_offline(struct be_ctx *ctx)
 {
     int offline_timeout;
+    int offline_timeout_max;
     errno_t ret;
 
     DEBUG(SSSDBG_TRACE_INTERNAL, "Going offline!\n");
@@ -126,11 +148,12 @@ void be_mark_offline(struct be_ctx *ctx)
         DEBUG(SSSDBG_TRACE_INTERNAL, "Initialize check_if_online_ptask.\n");
 
         offline_timeout = get_offline_timeout(ctx);
+        offline_timeout_max = get_offline_timeout_max(ctx);
 
         ret = be_ptask_create_sync(ctx, ctx,
                                    offline_timeout, offline_timeout,
                                    offline_timeout, 30, offline_timeout,
-                                   3600 /* max_backoff */,
+                                   offline_timeout_max,
                                    try_to_go_online,
                                    ctx, "Check if online (periodic)",
                                    BE_PTASK_OFFLINE_EXECUTE,

From 61aa8989a60e7feb7de2a9520bb53572f75294c3 Mon Sep 17 00:00:00 2001
From: Duncan Eastoe <duncan.eastoe@att.com>
Date: Tue, 21 Jul 2020 17:16:16 +0100
Subject: [PATCH 2/4] be_ptask: max_backoff may not be reached

If the incremented delay value was greater than max_backoff then
the previous delay was used, rather than using max_backoff as a
ceiling value.
---
 src/providers/be_ptask.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/be_ptask.c b/src/providers/be_ptask.c
index 319e44aa8f..3bf79f9c5d 100644
--- a/src/providers/be_ptask.c
+++ b/src/providers/be_ptask.c
@@ -216,9 +216,9 @@ static void be_ptask_schedule(struct be_ptask *task,
 
         delay = task->period;
 
-        if (backoff_allowed(task) && task->period * 2 <= task->max_backoff) {
+        if (backoff_allowed(task)) {
             /* double the period for the next execution */
-            task->period *= 2;
+            task->period = MIN(task->period * 2, task->max_backoff);
         }
         break;
     }

From 167485f64ea46a4a37e939deae81d4c114a9c481 Mon Sep 17 00:00:00 2001
From: Duncan Eastoe <duncan.eastoe@att.com>
Date: Tue, 21 Jul 2020 18:08:05 +0100
Subject: [PATCH 3/4] be_ptask: backoff not applied on first re-schedule

The task interval backoff is not applied on the first re-schedule
operation, since when scheduling the first run (BE_PTASK_FIRST_DELAY)
we do not calculate the backed off period for the next re-schedule.

Calculate the backed off period for the current scheduling operation,
rather than the next, to resolve this.
---
 src/providers/be_ptask.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/be_ptask.c b/src/providers/be_ptask.c
index 3bf79f9c5d..fb80909a0e 100644
--- a/src/providers/be_ptask.c
+++ b/src/providers/be_ptask.c
@@ -214,12 +214,12 @@ static void be_ptask_schedule(struct be_ptask *task,
             return;
         }
 
-        delay = task->period;
-
         if (backoff_allowed(task)) {
             /* double the period for the next execution */
             task->period = MIN(task->period * 2, task->max_backoff);
         }
+
+        delay = task->period;
         break;
     }
 

From 8f5c6a204f0220d13bc61e2a81cb8c6d3f5e02a8 Mon Sep 17 00:00:00 2001
From: Duncan Eastoe <duncan.eastoe@att.com>
Date: Fri, 4 Sep 2020 18:56:46 +0100
Subject: [PATCH 4/4] data_provider_be: Add OFFLINE_TIMEOUT_DEFAULT

Replace hardcoded default value of 60 in a couple of places.
---
 src/providers/data_provider_be.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index ab47702900..4c10d6b480 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -51,6 +51,7 @@
 #define ONLINE_CB_RETRY 3
 #define ONLINE_CB_RETRY_MAX_DELAY 4
 
+#define OFFLINE_TIMEOUT_DEFAULT 60
 #define OFFLINE_TIMEOUT_MAX_DEFAULT 3600
 
 /* sssd.service */
@@ -100,13 +101,14 @@ static int get_offline_timeout(struct be_ctx *ctx)
     int offline_timeout;
 
     ret = confdb_get_int(ctx->cdb, ctx->conf_path,
-                         CONFDB_DOMAIN_OFFLINE_TIMEOUT, 60,
+                         CONFDB_DOMAIN_OFFLINE_TIMEOUT,
+                         OFFLINE_TIMEOUT_DEFAULT,
                          &offline_timeout);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE,
               "Failed to get offline_timeout from confdb. "
-              "Will use 60 seconds.\n");
-        offline_timeout = 60;
+              "Will use %d seconds.\n", OFFLINE_TIMEOUT_DEFAULT);
+        offline_timeout = OFFLINE_TIMEOUT_DEFAULT;
     }
 
     return offline_timeout;
