3333. [bug] Setting resolver-query-timeout too low can cause

                        named to not recover if it looses connectivity.
                        [RT #29623]

diff --git a/CHANGES b/CHANGES
index 47a2198dfd3e8b311bfa240e5f0c0a749c728fd6..f0469c6d8382e0ab75c34537e2e42cb6aa8e52a9 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+3333.  [bug]           Setting resolver-query-timeout too low can cause
+                       named to not recover if it looses connectivity.
+                       [RT #29623]
+
 3332.  [bug]           Re-use cached DS rrsets if possible. [RT 29446]
 
 3331.  [security]      dns_rdataslab_fromrdataset could produce bad

diff --git a/bin/named/config.c b/bin/named/config.c
index eb1eef35046a71c191b80a095abb38726d237b6a..79fc539d1f6c8020ea509f6fa523ed526ee06099 100644 (file)
--- a/bin/named/config.c
+++ b/bin/named/config.c
@@ -89,7 +89,7 @@ options {\n\
 #endif
 "\
        recursive-clients 1000;\n\
-       resolver-query-timeout 30;\n\
+       resolver-query-timeout 10;\n\
        rrset-order { order random; };\n\
        serial-queries 20;\n\
        serial-query-rate 20;\n\

diff --git a/bin/tests/resolver/t_resolver.c b/bin/tests/resolver/t_resolver.c
index 3d611b60cb47496ca001a47bb825831b307d2243..27a18ac71401117eb87b6d02f972b8bb9cbb7e89 100644 (file)
--- a/bin/tests/resolver/t_resolver.c
+++ b/bin/tests/resolver/t_resolver.c
@@ -154,11 +154,11 @@ test_dns_resolver_settimeout(void) {
        t_info("The default timeout is %d second%s\n", default_timeout,
               (default_timeout == 1 ? "" : "s"));
 
-       dns_resolver_settimeout(resolver, default_timeout - 1);
+       dns_resolver_settimeout(resolver, default_timeout + 1);
        timeout = dns_resolver_gettimeout(resolver);
        t_info("The new timeout is %d second%s\n", timeout,
               (timeout == 1 ? "" : "s"));
-       test_result = (timeout == default_timeout - 1) ? T_PASS : T_FAIL;
+       test_result = (timeout == default_timeout + 1) ? T_PASS : T_FAIL;
 
        destroy_resolver(&resolver);
        teardown();

diff --git a/doc/arm/Bv9ARM-book.xml b/doc/arm/Bv9ARM-book.xml
index 252f18bb02896df732e0982e6f599503d95dd058..48119cb5d2d50fc3ad51fb61ce0440c687f2d0bf 100644 (file)
--- a/doc/arm/Bv9ARM-book.xml
+++ b/doc/arm/Bv9ARM-book.xml
@@ -7474,8 +7474,8 @@ options {
               <listitem>
                 <para>
                  The amount of time the resolver will spend attempting
-                 to resolve a recursive query before failing.  The
-                 default is <literal>10</literal> and the maximum is
+                 to resolve a recursive query before failing.  The default
+                 and minimum is <literal>10</literal> and the maximum is
                  <literal>30</literal>.  Setting it to <literal>0</literal>
                  will result in the default being used.
                 </para>

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 48dcedf5a0d884bf2c1b19b27b01f2620be6d565..f6ce93bdd4b613cf80c47ead83ff9addf669156a 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -106,8 +106,21 @@
 #define QTRACE(m)
 #endif
 
+#define US_PER_SEC 1000000U
+/*
+ * The maximum time we will wait for a single query.
+ */
+#define MAX_SINGLE_QUERY_TIMEOUT 9U
+#define MAX_SINGLE_QUERY_TIMEOUT_US (MAX_SINGLE_QUERY_TIMEOUT*US_PER_SEC)
+
+/*
+ * We need to allow a individual query time to complete / timeout.
+ */
+#define MINIMUM_QUERY_TIMEOUT (MAX_SINGLE_QUERY_TIMEOUT + 1U)
+
+/* The default time in seconds for the whole query to live. */
 #ifndef DEFAULT_QUERY_TIMEOUT
-#define DEFAULT_QUERY_TIMEOUT 10  /* The default time in seconds for the whole query to live. */
+#define DEFAULT_QUERY_TIMEOUT MINIMUM_QUERY_TIMEOUT
 #endif
 
 #ifndef MAXIMUM_QUERY_TIMEOUT
@@ -827,8 +840,8 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp,
                         */
                        INSIST(no_response);
                        rtt = query->addrinfo->srtt + 200000;
-                       if (rtt > 10000000)
-                               rtt = 10000000;
+                       if (rtt > MAX_SINGLE_QUERY_TIMEOUT_US)
+                               rtt = MAX_SINGLE_QUERY_TIMEOUT_US;
                        /*
                         * Replace the current RTT with our value.
                         */
@@ -1343,12 +1356,18 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) {
                us = (800000 << (fctx->restarts - 2));
 
        /*
-        * Double the round-trip time.
+        * Add a fudge factor to the expected rtt based on the current
+        * estimate.
         */
-       rtt *= 2;
+       if (rtt < 50000)
+               rtt += 50000;
+       else if (rtt < 100000)
+               rtt += 100000;
+       else
+               rtt += 200000;
 
        /*
-        * Always wait for at least the doubled round-trip time.
+        * Always wait for at least the expected rtt.
         */
        if (us < rtt)
                us = rtt;
@@ -1356,11 +1375,11 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) {
        /*
         * But don't ever wait for more than 10 seconds.
         */
-       if (us > 10000000)
-               us = 10000000;
+       if (us > MAX_SINGLE_QUERY_TIMEOUT_US)
+               us = MAX_SINGLE_QUERY_TIMEOUT_US;
 
-       seconds = us / 1000000;
-       us -= seconds * 1000000;
+       seconds = us / US_PER_SEC;
+       us -= seconds * US_PER_SEC;
        isc_interval_set(&fctx->interval, seconds, us * 1000);
 }
 
@@ -1382,6 +1401,11 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
        task = res->buckets[fctx->bucketnum].task;
 
        srtt = addrinfo->srtt;
+
+       /*
+        * A forwarder needs to make multiple queries. Give it at least
+        * a second to do these in.
+        */
        if (ISFORWARDER(addrinfo) && srtt < 1000000)
                srtt = 1000000;
 
@@ -8191,8 +8215,8 @@ dns_resolver_logfetch(dns_fetch_t *fetch, isc_log_t *lctx,
                              "timeout:%u,lame:%u,neterr:%u,badresp:%u,"
                              "adberr:%u,findfail:%u,valfail:%u]",
                              __FILE__, fctx->exitline, fctx->info,
-                             fctx->duration / 1000000,
-                             fctx->duration % 1000000,
+                             fctx->duration / US_PER_SEC,
+                             fctx->duration % US_PER_SEC,
                              isc_result_totext(fctx->result),
                              isc_result_totext(fctx->vresult), domainbuf,
                              fctx->referrals, fctx->restarts,
@@ -8797,6 +8821,8 @@ dns_resolver_settimeout(dns_resolver_t *resolver, unsigned int seconds) {
                seconds = DEFAULT_QUERY_TIMEOUT;
        if (seconds > MAXIMUM_QUERY_TIMEOUT)
                seconds = MAXIMUM_QUERY_TIMEOUT;
+       if (seconds < MINIMUM_QUERY_TIMEOUT)
+               seconds =  MINIMUM_QUERY_TIMEOUT;
 
        resolver->query_timeout = seconds;
 }