MANTA-5525 Reduce SRV no-records retry TTL from 3600s to 60s

CHANGES.adoc

@@ -6,6 +6,12 @@ toc::[]
 
 ## v2.x
 
+### v2.10.4
+
+Maintenance release.
+
+- Reduce SRV no-records retry TTL from 3600s to 60s
+
 ### v2.10.3
 
 Maintenance release. Switch away from git:// URLs.

lib/resolver.js

@@ -250,13 +250,15 @@ function CueBallDNSResolver(options) {
 	mod_assert.optionalNumber(options.defaultPort, 'options.defaultPort');
 
 	mod_assert.optionalBool(options._isBootstrap, 'options._isBootstrap');
+	mod_assert.optionalNumber(options.srvTTL, 'options.srvTTL');
 
 	this.r_uuid = mod_uuid.v4();
 	this.r_resolvers = options.resolvers || [];
 	this.r_domain = options.domain;
 	this.r_service = options.service || '_http._tcp';
 	this.r_maxres = options.maxDNSConcurrency || 3;
 	this.r_defport = options.defaultPort || 80;
+	this.r_srvNoRecordsTTL = (options.srvTTL != null) ? options.srvTTL : 60;
 	this.r_isBootstrap = false;
 	if (options._isBootstrap === true)
 		this.r_isBootstrap = true;
@@ -601,7 +603,7 @@ CueBallDNSResolver.prototype.state_srv_try = function (S) {
 			} ];
 
 			var d = new Date();
-			var ttl = 60 * 60;
+			var ttl;
 
 			if (err.code === 'NOTIMP') {
 				/*
@@ -610,32 +612,35 @@ CueBallDNSResolver.prototype.state_srv_try = function (S) {
 				 * doesn't support SRV at all and this isn't
 				 * likely to change quickly.
 				 */
+				ttl = 60 * 60;
 				self.r_log.info('SRV got NOTIMP for %s; ' +
 				    'retry in %d seconds', self.r_service, ttl);
 			} else {
 
 				/*
-				 * No SRV records (NXDOMAIN, NODATA, etc).  By
-				 * default, we'll wait 60 minutes before trying
-				 * again: most likely, there *are* no SRV
-				 * records to be had.
-				 *
-				 * However, if binder gave us an SOA TTL
-				 * (currently only implemented for NODATA),
-				 * we'll use that.  This is basically the same
-				 * TTL we get for AAAA/A responses.
+				 * No SRV records (NXDOMAIN, NODATA, etc). Use the SOA TTL from
+				 * the DNS response if available (currently only implemented for
+				 * NODATA by binder). Otherwise fall back to r_srvNoRecordsTTL
+				 * (default 60s, configurable via the srvTTL constructor
+				 * option).
+				 * The previous default of 3600s created a large vulnerability
+				 * window: if A record resolution also fails transiently during
+				 * this period, the connection pool enters "failed" state and
+				 * cannot recover until the SRV timer expires -- turning a brief
+				 * DNS blip into a prolonged outage.
 				 */
 
 				if (err.ttl) {
 					ttl = err.ttl;
 				} else {
+					ttl = self.r_srvNoRecordsTTL;
 					self.r_log.trace('no TTL in %s for ' +
-					    '%s; falling back to 60m retry',
-					    err.name, self.r_service);
+					                 '%s; falling back to %ds retry', err.name,
+                                     self.r_service, ttl);
 				}
 
 				self.r_log.info('no SRV records for %s; ' +
-				    'retry in %d seconds', self.r_service, ttl);
+				                'retry in %d seconds', self.r_service, ttl);
 			}
 
 			d.setTime(d.getTime() + ttl * 1000);

package.json

@@ -1,6 +1,6 @@
 {
   "name": "cueball",
-  "version": "2.10.3",
+  "version": "2.10.4",
   "description": "manage a pool of connections to a multi-node service where nodes are listed in DNS",
   "main": "lib/index.js",
   "dependencies": {