feat: use lru to aovid oom in dns cache httpclient (eggjs#961)

config/config.default.js

@@ -239,6 +239,8 @@ module.exports = appInfo => {
     maxSockets: Infinity,
     maxFreeSockets: 256,
     enableDNSCache: false,
+    dnsCacheMaxLength: 1000,
+    dnsCacheMaxAge: 10000,
   };
 
   /**

docs/source/zh-cn/core/httpclient.md

@@ -267,9 +267,14 @@ exports.httpclient = {
   maxSockets: Infinity,
   // 最大空闲 socket 数
   maxFreeSockets: 256,
-  // 是否开启本地 DNS 缓存，默认关闭
-  // 一旦设置开启，则每个域名的 DNS 查询结果将在进程内缓存 10 秒
+  // 是否开启本地 DNS 缓存，默认关闭，开启后有两个特性
+  // 1. 所有的 DNS 查询都会默认优先使用缓存的，即使 DNS 查询错误也不影响应用
+  // 2. 对同一个域名，在 dnsCacheLookupInterval 的间隔内（默认 10s）只会查询一次
   enableDNSCache: false,
+  // 对同一个域名进行 DNS 查询的最小间隔时间
+  dnsCacheLookupInterval: 10000,
+  // DNS 同时缓存的最大域名数量，默认 1000
+  dnsCacheMaxLength: 1000,
 };
 ```
 

lib/core/dnscache_httpclient.js

@@ -1,16 +1,20 @@
 'use strict';
 
 const dns = require('dns');
+const LRU = require('ylru');
 const urlparse = require('url').parse;
 const urllib = require('urllib');
+const utility = require('utility');
+
+const IP_REGEX = /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/;
 
 class DNSCacheHttpClient extends urllib.HttpClient {
   constructor(options) {
     super(options);
 
     this.app = options.app;
-    this.dnsCacheMaxAge = 10000;
-    this.dnsCache = new Map();
+    this.dnsCacheLookupInterval = this.app.config.httpclient.dnsCacheLookupInterval;
+    this.dnsCache = new LRU(this.app.config.httpclient.dnsCacheMaxLength);
   }
 
   request(url, args, callback) {
@@ -62,24 +66,31 @@ class DNSCacheHttpClient extends urllib.HttpClient {
   _dnsLookup(url, args, callback) {
     const parsed = typeof url === 'string' ? urlparse(url) : url;
     // hostname must exists
-    const host = parsed.hostname;
+    const hostname = parsed.hostname;
+
+    // don't lookup when hostname is IP
+    if (hostname && IP_REGEX.test(hostname)) {
+      return callback(null, { url, args });
+    }
+
     args = args || {};
     args.headers = args.headers || {};
     // set host header is not exists
     if (!args.headers.host && !args.headers.Host) {
-      args.headers.host = host;
+      // host must combine with hostname:port, node won't use `parsed.host`
+      args.headers.host = parsed.port ? `${hostname}:${parsed.port}` : hostname;
     }
 
-    const record = this.dnsCache.get(host);
+    const record = this.dnsCache.get(hostname);
     const now = Date.now();
     if (record) {
-      const needUpdate = now - record.timestamp >= this.dnsCacheMaxAge;
+      const needUpdate = now - record.timestamp >= this.dnsCacheLookupInterval;
       if (needUpdate) {
         // make sure next request don't refresh dns query
         record.timestamp = now;
       }
       callback(null, {
-        url: this._formatDnsLookupUrl(host, url, record.ip),
+        url: this._formatDnsLookupUrl(hostname, url, record.ip),
         args,
       });
       if (!needUpdate) {
@@ -90,22 +101,22 @@ class DNSCacheHttpClient extends urllib.HttpClient {
       callback = null;
     }
 
-    dns.lookup(host, { family: 4 }, (err, address) => {
+    dns.lookup(hostname, { family: 4 }, (err, address) => {
       const logger = args.ctx ? args.ctx.coreLogger : this.app.coreLogger;
       if (err) {
-        logger.warn('[dnscache_httpclient] dns lookup error: %s(%s) => %s', host, url, err);
+        logger.warn('[dnscache_httpclient] dns lookup error: %s(%s) => %s', hostname, url, err);
         // no cache, return error
         return callback && callback(err);
       }
 
-      logger.info('[dnscache_httpclient] dns lookup success: %s(%s) => %s', host, url, address);
-      this.dnsCache.set(host, {
+      logger.info('[dnscache_httpclient] dns lookup success: %s(%s) => %s', hostname, url, address);
+      this.dnsCache.set(hostname, {
         timestamp: Date.now(),
         ip: address,
       });
 
       callback && callback(null, {
-        url: this._formatDnsLookupUrl(host, url, address),
+        url: this._formatDnsLookupUrl(hostname, url, address),
         args,
       });
     });
@@ -115,6 +126,7 @@ class DNSCacheHttpClient extends urllib.HttpClient {
     if (typeof url === 'string') {
       url = url.replace(host, address);
     } else {
+      url = utility.assign({}, url);
       url.hostname = url.hostname.replace(host, address);
       if (url.host) {
         url.host = url.host.replace(host, address);

package.json

@@ -43,7 +43,8 @@
     "mime-types": "^2.1.15",
     "sendmessage": "^1.1.0",
     "urllib": "^2.22.0",
-    "utility": "^1.12.0"
+    "utility": "^1.12.0",
+    "ylru": "^1.0.0"
   },
   "devDependencies": {
     "autod": "^2.8.0",

test/lib/core/dnscache_httpclient.test.js

@@ -10,12 +10,14 @@ const utils = require('../../utils');
 describe('test/lib/core/dnscache_httpclient.test.js', () => {
   let app;
   let url;
+  let host;
 
   before(function* () {
     app = utils.app('apps/dnscache_httpclient');
     yield app.ready();
     url = yield utils.startLocalServer();
     url = url.replace('127.0.0.1', 'localhost');
+    host = urlparse(url).host;
   });
 
   afterEach(mm.restore);
@@ -24,7 +26,7 @@ describe('test/lib/core/dnscache_httpclient.test.js', () => {
     yield request(app.callback())
       .get('/?url=' + encodeURIComponent(url + '/get_headers'))
       .expect(200)
-      .expect(/"host":"localhost"/);
+      .expect(/"host":"localhost:\d+"/);
     yield request(app.callback())
       .get('/?url=' + encodeURIComponent(url + '/get_headers') + '&host=localhost.foo.com')
       .expect(200)
@@ -46,31 +48,31 @@ describe('test/lib/core/dnscache_httpclient.test.js', () => {
     yield request(app.callback())
       .get('/?url=' + encodeURIComponent(url + '/get_headers'))
       .expect(200)
-      .expect(/"host":"localhost"/);
+      .expect(/"host":"localhost:\d+"/);
     // mock local cache expires and mock dns lookup throw error
     app.httpclient.dnsCache.get('localhost').timestamp = 0;
     mm.error(dns, 'lookup', 'mock dns lookup error');
     yield request(app.callback())
       .get('/?url=' + encodeURIComponent(url + '/get_headers'))
       .expect(200)
-      .expect(/"host":"localhost"/);
+      .expect(/"host":"localhost:\d+"/);
   });
 
   it('should app.curl work', function* () {
     const result = yield app.curl(url + '/get_headers', { dataType: 'json' });
     assert(result.status === 200);
-    assert(result.data.host === 'localhost');
+    assert(result.data.host === host);
 
     const result2 = yield app.httpclient.curl(url + '/get_headers', { dataType: 'json' });
     assert(result2.status === 200);
-    assert(result2.data.host === 'localhost');
+    assert(result2.data.host === host);
   });
 
   it('should callback style work', done => {
     app.httpclient.curl(url + '/get_headers', (err, data, res) => {
       data = JSON.parse(data);
       assert(res.status === 200);
-      assert(data.host === 'localhost');
+      assert(data.host === host);
       done();
     });
   });
@@ -88,7 +90,7 @@ describe('test/lib/core/dnscache_httpclient.test.js', () => {
       assert(!err);
       const data = JSON.parse(result.data);
       assert(result.res.status === 200);
-      assert(data.host === 'localhost');
+      assert(data.host === host);
       done();
     });
   });
@@ -104,27 +106,60 @@ describe('test/lib/core/dnscache_httpclient.test.js', () => {
   it('should app.curl work on lookup error', function* () {
     const result = yield app.curl(url + '/get_headers', { dataType: 'json' });
     assert(result.status === 200);
-    assert(result.data.host === 'localhost');
+    assert(result.data.host === host);
 
     // mock local cache expires and mock dns lookup throw error
     app.httpclient.dnsCache.get('localhost').timestamp = 0;
     mm.error(dns, 'lookup', 'mock dns lookup error');
     const result2 = yield app.httpclient.curl(url + '/get_headers', { dataType: 'json' });
     assert(result2.status === 200);
-    assert(result2.data.host === 'localhost');
+    assert(result2.data.host === host);
   });
 
   it('should app.curl(obj)', function* () {
     const obj = urlparse(url + '/get_headers');
     const result = yield app.curl(obj, { dataType: 'json' });
     assert(result.status === 200);
-    assert(result.data.host === 'localhost');
+    assert(result.data.host === host);
 
     const obj2 = urlparse(url + '/get_headers');
     // mock obj2.host
     obj2.host = null;
     const result2 = yield app.curl(obj2, { dataType: 'json' });
     assert(result2.status === 200);
-    assert(result2.data.host === 'localhost');
+    assert(result2.data.host === host);
+  });
+
+  it('should dnsCacheMaxLength work', function* () {
+    mm.data(dns, 'lookup', '127.0.0.1');
+
+    // reset lru cache
+    mm(app.httpclient.dnsCache, 'max', 1);
+    mm(app.httpclient.dnsCache, 'size', 0);
+    mm(app.httpclient.dnsCache, 'cache', new Map());
+    mm(app.httpclient.dnsCache, '_cache', new Map());
+
+    let obj = urlparse(url + '/get_headers');
+    let result = yield app.curl(obj, { dataType: 'json' });
+    assert(result.status === 200);
+    assert(result.data.host === host);
+
+    assert(app.httpclient.dnsCache.get('localhost'));
+
+    obj = urlparse(url.replace('localhost', 'another.com') + '/get_headers');
+    result = yield app.curl(obj, { dataType: 'json' });
+    assert(result.status === 200);
+    assert(result.data.host === obj.host);
+
+    assert(!app.httpclient.dnsCache.get('localhost'));
+    assert(app.httpclient.dnsCache.get('another.com'));
+  });
+
+  it('should not cache ip', function* () {
+    const obj = urlparse(url.replace('localhost', '127.0.0.1') + '/get_headers');
+    const result = yield app.curl(obj, { dataType: 'json' });
+    assert(result.status === 200);
+    assert(result.data.host === obj.host);
+    assert(!app.httpclient.dnsCache.get('127.0.0.1'));
   });
 });