URL: https://github.com/SSSD/sssd/pull/636 Author: pbrezina Title: #636: failover: tune up default timeouts Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/636/head:pr636 git checkout pr636
From 77236ad720a8ce4208fe284f7756789f79d06e90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Tue, 11 Jun 2019 13:49:13 +0200 Subject: [PATCH 1/5] man: fix description of dns_resolver_op_timeout --- src/man/include/failover.xml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/man/include/failover.xml b/src/man/include/failover.xml index cd6fd4d798..11ff86a388 100644 --- a/src/man/include/failover.xml +++ b/src/man/include/failover.xml @@ -77,7 +77,13 @@ </term> <listitem> <para> - How long would SSSD talk to a single DNS server. + Time in seconds to tell how long would SSSD try + to resolve single DNS query (e.g. resolution of a + hostname or an SRV record) before trying the next + hostname or discovery domain. + </para> + <para> + Default: 6 </para> </listitem> </varlistentry> From c6b7ac5fd855a655f0363d4a27ad877de9d1e9ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Tue, 11 Jun 2019 13:49:33 +0200 Subject: [PATCH 2/5] man: fix description of dns_resolver_timeout --- src/man/include/failover.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/man/include/failover.xml b/src/man/include/failover.xml index 11ff86a388..7b451d8315 100644 --- a/src/man/include/failover.xml +++ b/src/man/include/failover.xml @@ -98,6 +98,9 @@ include several steps, such as resolving DNS SRV queries or locating the site. </para> + <para> + Default: 6 + </para> </listitem> </varlistentry> </variablelist> From d2626347e7674356fc8500cf2c5ef421f096c133 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Tue, 11 Jun 2019 13:37:23 +0200 Subject: [PATCH 3/5] failover: add dns_resolver_server_timeout option --- src/config/SSSDConfig/__init__.py.in | 1 + src/config/SSSDConfigTest.py | 2 ++ src/config/cfg_rules.ini | 1 + src/config/etc/sssd.api.conf | 1 + src/man/include/failover.xml | 17 ++++++++++++++++- src/providers/data_provider.h | 1 + src/providers/data_provider_fo.c | 3 +++ src/resolv/async_resolv.c | 10 ++++++---- src/resolv/async_resolv.h | 2 +- src/tests/cmocka/test_fo_srv.c | 4 ++-- src/tests/cmocka/test_resolv_fake.c | 2 +- src/tests/fail_over-tests.c | 2 +- src/tests/resolv-tests.c | 2 +- 13 files changed, 37 insertions(+), 11 deletions(-) diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 9642fe6baf..2d1214e16b 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -171,6 +171,7 @@ option_strings = { 'entry_cache_timeout' : _('Entry cache timeout length (seconds)'), 'lookup_family_order' : _('Restrict or prefer a specific address family when performing DNS lookups'), 'account_cache_expiration' : _('How long to keep cached entries after last successful login (days)'), + 'dns_resolver_server_timeout' : _('How long should SSSD talk to single DNS server before trying next server (miliseconds)'), 'dns_resolver_timeout' : _('How long to wait for replies from DNS when resolving servers (seconds)'), 'dns_discovery_domain' : _('The domain part of service discovery DNS query'), 'override_gid' : _('Override GID value from the identity provider with this value'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index 727df71abf..82b1a97008 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -606,6 +606,7 @@ def testListOptions(self): 'refresh_expired_interval', 'lookup_family_order', 'account_cache_expiration', + 'dns_resolver_server_timeout', 'dns_resolver_timeout', 'dns_discovery_domain', 'dyndns_update', @@ -976,6 +977,7 @@ def testRemoveProvider(self): 'refresh_expired_interval', 'account_cache_expiration', 'lookup_family_order', + 'dns_resolver_server_timeout', 'dns_resolver_timeout', 'dns_discovery_domain', 'dyndns_update', diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 929e6149a7..a2efb3a677 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -367,6 +367,7 @@ option = account_cache_expiration option = pwd_expiration_warning option = filter_users option = filter_groups +option = dns_resolver_server_timeout option = dns_resolver_timeout option = dns_discovery_domain option = override_gid diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index c6d6690fb4..288b1cfe75 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -170,6 +170,7 @@ account_cache_expiration = int, None, false pwd_expiration_warning = int, None, false filter_users = list, str, false filter_groups = list, str, false +dns_resolver_server_timeout = int, None, false dns_resolver_timeout = int, None, false dns_discovery_domain = str, None, false override_gid = int, None, false diff --git a/src/man/include/failover.xml b/src/man/include/failover.xml index 7b451d8315..f2a01b933e 100644 --- a/src/man/include/failover.xml +++ b/src/man/include/failover.xml @@ -71,6 +71,20 @@ </citerefentry>, manual page. <variablelist> + <varlistentry> + <term> + dns_resolver_server_timeout + </term> + <listitem> + <para> + Time in milliseconds that sets how long would SSSD + talk to a single DNS server before trying next one. + </para> + <para> + Default: 2000 + </para> + </listitem> + </varlistentry> <varlistentry> <term> dns_resolver_op_timeout @@ -111,7 +125,8 @@ <quote>ldap_opt_timeout></quote> timeout should be set to a larger value than <quote>dns_resolver_timeout</quote> which in turn should be set to a larger value than - <quote>dns_resolver_op_timeout</quote>. + <quote>dns_resolver_op_timeout</quote> which should be larger + than <quote>dns_resolver_server_timeout</quote>. </para> </refsect2> </refsect1> diff --git a/src/providers/data_provider.h b/src/providers/data_provider.h index a0a21cc123..2d10dbb5bc 100644 --- a/src/providers/data_provider.h +++ b/src/providers/data_provider.h @@ -265,6 +265,7 @@ enum dp_res_opts { DP_RES_OPT_FAMILY_ORDER, DP_RES_OPT_RESOLVER_TIMEOUT, DP_RES_OPT_RESOLVER_OP_TIMEOUT, + DP_RES_OPT_RESOLVER_SERVER_TIMEOUT, DP_RES_OPT_DNS_DOMAIN, DP_RES_OPTS /* attrs counter */ diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c index 473b667e58..a7af3e2a54 100644 --- a/src/providers/data_provider_fo.c +++ b/src/providers/data_provider_fo.c @@ -833,6 +833,7 @@ static struct dp_option dp_res_default_opts[] = { { "lookup_family_order", DP_OPT_STRING, { "ipv4_first" }, NULL_STRING }, { "dns_resolver_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, { "dns_resolver_op_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, + { "dns_resolver_server_timeout", DP_OPT_NUMBER, { .number = 2000 }, NULL_NUMBER }, { "dns_discovery_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING }, DP_OPTION_TERMINATOR }; @@ -894,6 +895,8 @@ errno_t be_res_init(struct be_ctx *ctx) ret = resolv_init(ctx, ctx->ev, dp_opt_get_int(ctx->be_res->opts, DP_RES_OPT_RESOLVER_OP_TIMEOUT), + dp_opt_get_int(ctx->be_res->opts, + DP_RES_OPT_RESOLVER_SERVER_TIMEOUT), &ctx->be_res->resolv); if (ret != EOK) { talloc_zfree(ctx->be_res); diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c index 01d835ec9a..00b9531d49 100644 --- a/src/resolv/async_resolv.c +++ b/src/resolv/async_resolv.c @@ -60,8 +60,6 @@ #define DNS_RR_LEN(r) DNS__16BIT((r) + 8) #define DNS_RR_TTL(r) DNS__32BIT((r) + 4) -#define RESOLV_TIMEOUTMS 2000 - enum host_database default_host_dbs[] = { DB_FILES, DB_DNS, DB_SENTINEL }; struct fd_watch { @@ -83,6 +81,9 @@ struct resolv_ctx { /* Time in milliseconds before canceling a DNS request */ int timeout; + /* Time in milliseconds for communication with single DNS server. */ + int ares_timeout; + /* The timeout watcher periodically calls ares_process_fd() to check * if our pending requests didn't timeout. */ int pending_requests; @@ -423,7 +424,7 @@ recreate_ares_channel(struct resolv_ctx *ctx) */ options.sock_state_cb = fd_event; options.sock_state_cb_data = ctx; - options.timeout = RESOLV_TIMEOUTMS; + options.timeout = ctx->ares_timeout; /* Only affects ares_gethostbyname */ options.lookups = discard_const("f"); options.tries = 1; @@ -450,7 +451,7 @@ recreate_ares_channel(struct resolv_ctx *ctx) int resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, - int timeout, struct resolv_ctx **ctxp) + int timeout, int ares_timeout, struct resolv_ctx **ctxp) { int ret; struct resolv_ctx *ctx; @@ -467,6 +468,7 @@ resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, ctx->ev_ctx = ev_ctx; ctx->timeout = timeout; + ctx->ares_timeout = ares_timeout; ret = recreate_ares_channel(ctx); if (ret != EOK) { diff --git a/src/resolv/async_resolv.h b/src/resolv/async_resolv.h index 90ed037075..d83a7be447 100644 --- a/src/resolv/async_resolv.h +++ b/src/resolv/async_resolv.h @@ -52,7 +52,7 @@ struct resolv_ctx; int resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, - int timeout, struct resolv_ctx **ctxp); + int timeout, int ares_timeout, struct resolv_ctx **ctxp); void resolv_reread_configuration(struct resolv_ctx *ctx); diff --git a/src/tests/cmocka/test_fo_srv.c b/src/tests/cmocka/test_fo_srv.c index a11ebbb549..c13cf3a69b 100644 --- a/src/tests/cmocka/test_fo_srv.c +++ b/src/tests/cmocka/test_fo_srv.c @@ -49,7 +49,7 @@ struct resolv_ctx { /* mock resolver interface. The resolver test is separate */ int resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx, - int timeout, struct resolv_ctx **ctxp) + int timeout, int ares_timeout, struct resolv_ctx **ctxp) { *ctxp = talloc(mem_ctx, struct resolv_ctx); return EOK; @@ -230,7 +230,7 @@ static int test_fo_setup(void **state) assert_non_null(test_ctx->ctx); ret = resolv_init(test_ctx, test_ctx->ctx->ev, - TEST_RESOLV_TIMEOUT, &test_ctx->resolv); + TEST_RESOLV_TIMEOUT, 2000, &test_ctx->resolv); assert_non_null(test_ctx->resolv); memset(&fopts, 0, sizeof(fopts)); diff --git a/src/tests/cmocka/test_resolv_fake.c b/src/tests/cmocka/test_resolv_fake.c index 4cb3d40278..0f4011a39f 100644 --- a/src/tests/cmocka/test_resolv_fake.c +++ b/src/tests/cmocka/test_resolv_fake.c @@ -240,7 +240,7 @@ static int test_resolv_fake_setup(void **state) assert_non_null(test_ctx->ctx); ret = resolv_init(test_ctx, test_ctx->ctx->ev, - TEST_DEFAULT_TIMEOUT, &test_ctx->resolv); + TEST_DEFAULT_TIMEOUT, 2000, &test_ctx->resolv); assert_int_equal(ret, EOK); *state = test_ctx; diff --git a/src/tests/fail_over-tests.c b/src/tests/fail_over-tests.c index 5312b2772a..b2269ef3b2 100644 --- a/src/tests/fail_over-tests.c +++ b/src/tests/fail_over-tests.c @@ -73,7 +73,7 @@ setup_test(void) fail("Could not init tevent context"); } - ret = resolv_init(ctx, ctx->ev, 5, &ctx->resolv); + ret = resolv_init(ctx, ctx->ev, 5, 2000, &ctx->resolv); if (ret != EOK) { talloc_free(ctx); fail("Could not init resolv context"); diff --git a/src/tests/resolv-tests.c b/src/tests/resolv-tests.c index 4a2b3b9048..bc4cd7cc18 100644 --- a/src/tests/resolv-tests.c +++ b/src/tests/resolv-tests.c @@ -76,7 +76,7 @@ static int setup_resolv_test(int timeout, struct resolv_test_ctx **ctx) return EFAULT; } - ret = resolv_init(test_ctx, test_ctx->ev, timeout, &test_ctx->resolv); + ret = resolv_init(test_ctx, test_ctx->ev, timeout, 2000, &test_ctx->resolv); if (ret != EOK) { fail("Could not init resolv context"); talloc_free(test_ctx); From 9a9bf0e382e567be29569b7c2a0707fe7f3baab6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Tue, 11 Jun 2019 14:01:17 +0200 Subject: [PATCH 4/5] failover: change default timeouts --- src/man/include/failover.xml | 6 +++--- src/man/sssd-ldap.5.xml | 2 +- src/providers/ad/ad_opts.c | 2 +- src/providers/data_provider_fo.c | 4 ++-- src/providers/ipa/ipa_opts.c | 2 +- src/providers/ldap/ldap_opts.c | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/man/include/failover.xml b/src/man/include/failover.xml index f2a01b933e..288d91807a 100644 --- a/src/man/include/failover.xml +++ b/src/man/include/failover.xml @@ -81,7 +81,7 @@ talk to a single DNS server before trying next one. </para> <para> - Default: 2000 + Default: 1000 </para> </listitem> </varlistentry> @@ -97,7 +97,7 @@ hostname or discovery domain. </para> <para> - Default: 6 + Default: 2 </para> </listitem> </varlistentry> @@ -113,7 +113,7 @@ queries or locating the site. </para> <para> - Default: 6 + Default: 4 </para> </listitem> </varlistentry> diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml index f0bc82db5f..c205aea64d 100644 --- a/src/man/sssd-ldap.5.xml +++ b/src/man/sssd-ldap.5.xml @@ -1432,7 +1432,7 @@ StartTLS operation. </para> <para> - Default: 6 + Default: 8 </para> </listitem> </varlistentry> diff --git a/src/providers/ad/ad_opts.c b/src/providers/ad/ad_opts.c index 978c395ef2..3f7ec08b1d 100644 --- a/src/providers/ad/ad_opts.c +++ b/src/providers/ad/ad_opts.c @@ -65,7 +65,7 @@ struct dp_option ad_def_ldap_opts[] = { { "ldap_default_authtok", DP_OPT_BLOB, NULL_BLOB, NULL_BLOB }, { "ldap_search_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, { "ldap_network_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, - { "ldap_opt_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, + { "ldap_opt_timeout", DP_OPT_NUMBER, { .number = 8 }, NULL_NUMBER }, { "ldap_tls_reqcert", DP_OPT_STRING, { "hard" }, NULL_STRING }, { "ldap_user_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_user_search_scope", DP_OPT_STRING, { "sub" }, NULL_STRING }, diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c index a7af3e2a54..c634b8d49f 100644 --- a/src/providers/data_provider_fo.c +++ b/src/providers/data_provider_fo.c @@ -832,8 +832,8 @@ void _be_fo_set_port_status(struct be_ctx *ctx, static struct dp_option dp_res_default_opts[] = { { "lookup_family_order", DP_OPT_STRING, { "ipv4_first" }, NULL_STRING }, { "dns_resolver_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, - { "dns_resolver_op_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, - { "dns_resolver_server_timeout", DP_OPT_NUMBER, { .number = 2000 }, NULL_NUMBER }, + { "dns_resolver_op_timeout", DP_OPT_NUMBER, { .number = 3 }, NULL_NUMBER }, + { "dns_resolver_server_timeout", DP_OPT_NUMBER, { .number = 1000 }, NULL_NUMBER }, { "dns_discovery_domain", DP_OPT_STRING, NULL_STRING, NULL_STRING }, DP_OPTION_TERMINATOR }; diff --git a/src/providers/ipa/ipa_opts.c b/src/providers/ipa/ipa_opts.c index c38a7da0ed..7974cb8ea0 100644 --- a/src/providers/ipa/ipa_opts.c +++ b/src/providers/ipa/ipa_opts.c @@ -76,7 +76,7 @@ struct dp_option ipa_def_ldap_opts[] = { { "ldap_default_authtok", DP_OPT_BLOB, NULL_BLOB, NULL_BLOB }, { "ldap_search_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, { "ldap_network_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, - { "ldap_opt_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, + { "ldap_opt_timeout", DP_OPT_NUMBER, { .number = 8 }, NULL_NUMBER }, { "ldap_tls_reqcert", DP_OPT_STRING, { "hard" }, NULL_STRING }, { "ldap_user_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_user_search_scope", DP_OPT_STRING, { "sub" }, NULL_STRING }, diff --git a/src/providers/ldap/ldap_opts.c b/src/providers/ldap/ldap_opts.c index dc56f07125..616934a21e 100644 --- a/src/providers/ldap/ldap_opts.c +++ b/src/providers/ldap/ldap_opts.c @@ -36,7 +36,7 @@ struct dp_option default_basic_opts[] = { { "ldap_default_authtok", DP_OPT_BLOB, NULL_BLOB, NULL_BLOB }, { "ldap_search_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, { "ldap_network_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, - { "ldap_opt_timeout", DP_OPT_NUMBER, { .number = 6 }, NULL_NUMBER }, + { "ldap_opt_timeout", DP_OPT_NUMBER, { .number = 8 }, NULL_NUMBER }, { "ldap_tls_reqcert", DP_OPT_STRING, { "hard" }, NULL_STRING }, { "ldap_user_search_base", DP_OPT_STRING, NULL_STRING, NULL_STRING }, { "ldap_user_search_scope", DP_OPT_STRING, { "sub" }, NULL_STRING }, From adda0af4ec7508cb7ebdec98821fe7c3a5d42fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Mon, 8 Jul 2019 11:35:28 +0200 Subject: [PATCH 5/5] config: add dns_resolver_op_timeout to option list --- src/config/SSSDConfig/__init__.py.in | 1 + src/config/SSSDConfigTest.py | 2 ++ src/config/cfg_rules.ini | 1 + src/config/etc/sssd.api.conf | 1 + 4 files changed, 5 insertions(+) diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in index 2d1214e16b..ea79954104 100644 --- a/src/config/SSSDConfig/__init__.py.in +++ b/src/config/SSSDConfig/__init__.py.in @@ -172,6 +172,7 @@ option_strings = { 'lookup_family_order' : _('Restrict or prefer a specific address family when performing DNS lookups'), 'account_cache_expiration' : _('How long to keep cached entries after last successful login (days)'), 'dns_resolver_server_timeout' : _('How long should SSSD talk to single DNS server before trying next server (miliseconds)'), + 'dns_resolver_op_timeout' : _('How long should keep trying to resolve single DNS query (seconds)'), 'dns_resolver_timeout' : _('How long to wait for replies from DNS when resolving servers (seconds)'), 'dns_discovery_domain' : _('The domain part of service discovery DNS query'), 'override_gid' : _('Override GID value from the identity provider with this value'), diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py index 82b1a97008..95dfd677d0 100755 --- a/src/config/SSSDConfigTest.py +++ b/src/config/SSSDConfigTest.py @@ -607,6 +607,7 @@ def testListOptions(self): 'lookup_family_order', 'account_cache_expiration', 'dns_resolver_server_timeout', + 'dns_resolver_op_timeout', 'dns_resolver_timeout', 'dns_discovery_domain', 'dyndns_update', @@ -978,6 +979,7 @@ def testRemoveProvider(self): 'account_cache_expiration', 'lookup_family_order', 'dns_resolver_server_timeout', + 'dns_resolver_op_timeout', 'dns_resolver_timeout', 'dns_discovery_domain', 'dyndns_update', diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index a2efb3a677..30040b5950 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -368,6 +368,7 @@ option = pwd_expiration_warning option = filter_users option = filter_groups option = dns_resolver_server_timeout +option = dns_resolver_op_timeout option = dns_resolver_timeout option = dns_discovery_domain option = override_gid diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf index 288b1cfe75..4a069f2db2 100644 --- a/src/config/etc/sssd.api.conf +++ b/src/config/etc/sssd.api.conf @@ -171,6 +171,7 @@ pwd_expiration_warning = int, None, false filter_users = list, str, false filter_groups = list, str, false dns_resolver_server_timeout = int, None, false +dns_resolver_op_timeout = int, None, false dns_resolver_timeout = int, None, false dns_discovery_domain = str, None, false override_gid = int, None, false
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org