Add offline_timeout_max option to control offline interval backoff #5252
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
elkoniu
reviewed
Jul 22, 2020
deastoe
force-pushed
the
offline-timeout-max
branch
from
eebb19e
to
403e287
Compare
elkoniu
reviewed
Aug 3, 2020
src/providers/data_provider_be.c
Outdated
| DEBUG(SSSDBG_CONF_SETTINGS, | ||
| "Failed to get offline_timeout_max from confdb. " | ||
| "Will use 3600 seconds.\n"); | ||
| offline_timeout_max = 3600; |
There was a problem hiding this comment.
I think that changing those hardcoded "60" and "3600" everywhere may be a good candidate for next contribution.
They may look better as a some function local const or global define if it is worht it. Right now in future when someone change 60s to lets say 90s will have to go around multiple lines to fix it.
There was a problem hiding this comment.
Sounds good, I've added some defines at the top of the file. Apologies for the delay getting back to this PR.
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.
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.
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.
Replace hardcoded default value of 60 in a couple of places.
deastoe
force-pushed
the
offline-timeout-max
branch
from
403e287
to
8f5c6a2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Additionally fix two issues observed in be_ptask scheduling: