Randomize retry penalties to prevent thundering herd type issues (#606)

The retry timeout values were using a fixed calculation which could cause multiple simultaneous queries to timeout and retry at the exact same time.  If a DNS server is throttling requests, this could cause the issue to never self-resolve due to all requests recurring at the same instance again.

This PR also creates a maximum timeout option to make sure the random value selected does not exceed this value.

Fix By: Ignat (@Kontakter)

docs/ares_init_options.3

@@ -28,6 +28,7 @@ struct ares_options {
   int timeout; /* in seconds or milliseconds, depending on options */
   int tries;
   int ndots;
+  int maxtimeout; /* in milliseconds */
   unsigned short udp_port;
   unsigned short tcp_port;
   int socket_send_buffer_size;
@@ -102,6 +103,11 @@ queried for "as is" prior to querying for it with the default domain
 extensions appended.  The default value is 1 unless set otherwise by
 resolv.conf or the RES_OPTIONS environment variable.
 .TP 18
+.B ARES_OPT_MAXTIMEOUTMS
+.B int \fImaxtimeout\fP;
+.br
+The upper bound for timeout between sequential retry attempts.
+.TP 18
 .B ARES_OPT_UDP_PORT
 .B unsigned short \fIudp_port\fP;
 .br

include/ares.h

@@ -194,6 +194,7 @@ typedef enum {
 #define ARES_OPT_RESOLVCONF      (1 << 17)
 #define ARES_OPT_HOSTS_FILE      (1 << 18)
 #define ARES_OPT_UDP_MAX_QUERIES (1 << 19)
+#define ARES_OPT_MAXTIMEOUTMS    (1 << 20)
 
 /* Nameinfo flag values */
 #define ARES_NI_NOFQDN        (1 << 0)
@@ -287,6 +288,7 @@ struct ares_options {
   int            timeout; /* in seconds or milliseconds, depending on options */
   int            tries;
   int            ndots;
+  int            maxtimeout; /* in milliseconds */
   unsigned short udp_port;
   unsigned short tcp_port;
   int            socket_send_buffer_size;

src/lib/ares_options.c

@@ -126,6 +126,10 @@ int ares_save_options(ares_channel_t *channel, struct ares_options *options,
     options->ndots = (int)channel->ndots;
   }
 
+  if (channel->optmask & ARES_OPT_MAXTIMEOUTMS) {
+    options->maxtimeout = (int)channel->maxtimeout;
+  }
+
   if (channel->optmask & ARES_OPT_UDP_PORT) {
     options->udp_port = ntohs(channel->udp_port);
   }
@@ -280,6 +284,10 @@ ares_status_t ares__init_by_options(ares_channel_t            *channel,
     channel->ndots = (size_t)options->ndots;
   }
 
+  if (optmask & ARES_OPT_MAXTIMEOUTMS) {
+    channel->maxtimeout = (size_t)options->maxtimeout;
+  }
+
   if (optmask & ARES_OPT_ROTATE) {
     channel->rotate = ARES_TRUE;
   }

src/lib/ares_private.h

@@ -252,6 +252,7 @@ struct ares_channeldata {
   size_t               timeout; /* in milliseconds */
   size_t               tries;
   size_t               ndots;
+  size_t               maxtimeout; /* in milliseconds */
   ares_bool_t          rotate;
   unsigned short       udp_port;                   /* stored in network order */
   unsigned short       tcp_port;                   /* stored in network order */
@@ -276,7 +277,7 @@ struct ares_channeldata {
    * failures, followed by the configuration order if failures are equal. */
   ares__slist_t       *servers;
 
-  /* random state to use when generating new ids */
+  /* random state to use when generating new ids and generating retry penalties */
   ares_rand_state     *rand_state;
 
   /* All active queries in a single list */

src/lib/ares_process.c

@@ -37,6 +37,9 @@
 #ifdef NETWARE
 #  include <sys/filio.h>
 #endif
+#ifdef HAVE_STDINT_H
+#  include <stdint.h>
+#endif
 
 #include <assert.h>
 #include <fcntl.h>
@@ -795,7 +798,7 @@ static ares_status_t ares__append_tcpbuf(struct server_state *server,
   return ares__buf_append(server->tcp_send, query->qbuf, query->qlen);
 }
 
-static size_t ares__retry_penalty(struct query *query)
+static size_t ares__calc_query_timeout(struct query *query)
 {
   const ares_channel_t *channel  = query->channel;
   size_t                timeplus = channel->timeout;
@@ -823,6 +826,32 @@ static size_t ares__retry_penalty(struct query *query)
     timeplus <<= shift;
   }
 
+  if (channel->maxtimeout && timeplus > channel->maxtimeout)
+    timeplus = channel->maxtimeout;
+
+  /* Add some jitter to the retry timeout.
+   *
+   * Jitter is needed in situation when resolve requests are performed
+   * simultaneously from multiple hosts and DNS server throttle these requests.
+   * Adding randomness allows to avoid synchronisation of retries.
+   *
+   * Value of timeplus adjusted randomly to the range [0.5 * timeplus, timeplus].
+   */
+  if (query->try_count > 0) {
+    unsigned short r;
+    float delta_multiplier;
+
+    ares__rand_bytes(channel->rand_state, (unsigned char *)&r, sizeof(r));
+    delta_multiplier = ((float)r / USHRT_MAX) * 0.5f;
+    timeplus -= (size_t)((float)timeplus * delta_multiplier);
+
+    // With shift that doubles each iteration and constant 0.5 above this situation
+    // is impossible, but we want explicitly guarantee that timeplus
+    // is greater or equal to timeout specified in channel options.
+    if (timeplus < channel->timeout)
+      timeplus = channel->timeout;
+  }
+
   return timeplus;
 }
 
@@ -949,7 +978,7 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now)
     }
   }
 
-  timeplus = ares__retry_penalty(query);
+  timeplus = ares__calc_query_timeout(query);
 
   /* Keep track of queries bucketed by timeout, so we can process
    * timeout events quickly.

test/ares-test-init.cc

@@ -85,6 +85,8 @@ TEST_F(LibraryTest, OptionsChannelInit) {
   opts.ndots = 4;
   optmask |= ARES_OPT_NDOTS;
   opts.udp_port = 54;
+  optmask |= ARES_OPT_MAXTIMEOUTMS;
+  opts.maxtimeout = 10000;
   optmask |= ARES_OPT_UDP_PORT;
   opts.tcp_port = 54;
   optmask |= ARES_OPT_TCP_PORT;
@@ -130,6 +132,7 @@ TEST_F(LibraryTest, OptionsChannelInit) {
   EXPECT_EQ(opts.timeout, opts2.timeout);
   EXPECT_EQ(opts.tries, opts2.tries);
   EXPECT_EQ(opts.ndots, opts2.ndots);
+  EXPECT_EQ(opts.maxtimeout, opts2.maxtimeout);
   EXPECT_EQ(opts.udp_port, opts2.udp_port);
   EXPECT_EQ(opts.tcp_port, opts2.tcp_port);
   EXPECT_EQ(1, opts2.nservers);  // Truncated by ARES_FLAG_PRIMARY