aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRagnis Armus <ragnis@armus.ee>2019-12-08 22:07:57 +0200
committerRagnis Armus <ragnis@armus.ee>2019-12-11 23:03:18 +0200
commit81a660243f5a54fcf6e4961901436adf9b7b11e8 (patch)
tree18586b098835de750a1a955891728a5abb16cfd4
parentb6c1012035f6631085b32cc993c8d4d860c4c077 (diff)
Do not block during renewal when possible
-rw-r--r--lib/agent.js21
-rw-r--r--lib/agent.test.js74
2 files changed, 92 insertions, 3 deletions
diff --git a/lib/agent.js b/lib/agent.js
index d2ac62c..53559f0 100644
--- a/lib/agent.js
+++ b/lib/agent.js
@@ -77,7 +77,7 @@ class Target {
resolve() {
return this.renewIfNeeded().then(() => {
- if (this.maxStaleAt && this.maxStaleAt < this.getTime()) {
+ if (this.isTooStale()) {
throw this.createError(this.err || errMaxStale);
}
@@ -97,10 +97,25 @@ class Target {
* @return {Promise}
*/
renewIfNeeded() {
+ let promise;
+
if (this.getTime() > this.renewNextAfter) {
- return this.renew();
+ promise = this.renew();
+
+ if (this.addrs !== undefined && !this.isTooStale()) {
+ // We have stale data that's still good enough. Don't wait for
+ // the renewal to complete.
+ promise = Promise.resolve();
+ }
+ } else {
+ promise = Promise.resolve();
}
- return Promise.resolve();
+
+ return promise;
+ }
+
+ isTooStale() {
+ return this.maxStaleAt !== undefined && this.maxStaleAt < this.getTime();
}
renew() {
diff --git a/lib/agent.test.js b/lib/agent.test.js
index 147d0df..21b7077 100644
--- a/lib/agent.test.js
+++ b/lib/agent.test.js
@@ -1,4 +1,5 @@
const assert = require('assert');
+const { EventEmitter } = require('events');
const dns = require('dns');
const { Target } = require('./agent');
@@ -8,6 +9,15 @@ const assertRejects = async (func, errback) => func().then(
errback,
);
+function defer() {
+ const deferred = {};
+ deferred.promise = new Promise((resolve, reject) => {
+ deferred.resolve = resolve;
+ deferred.reject = reject;
+ });
+ return deferred;
+}
+
describe('Target', function () {
describe('#renew()', function () {
it('should not cause multiple parallel DNS queries', function () {
@@ -207,5 +217,69 @@ describe('Target', function () {
wait = target.getRetryWait(5);
assert(wait >= 30 && wait <= 31);
});
+
+ it('should not wait for renewal to complete when under maxStale limit', async function () {
+ let time = 10;
+ const target = new Target({ maxStale: 10 });
+
+ target.getTime = () => time;
+ target.resolver = async () => [
+ { name: 'foo.com', port: 80, ttl: 5 },
+ ];
+
+ // Perform the first successful renewal.
+ await target.resolve();
+
+ const deferred = defer();
+ let didRenew = 0;
+
+ target.resolver = () => {
+ didRenew++;
+ return deferred.promise;
+ };
+
+ // Data is now stale and renewal will need to be performed.
+ time = 16;
+ assert.strictEqual(String(await target.resolve()), 'foo.com:80');
+ assert.strictEqual(didRenew, 1);
+ assert.strictEqual(String(await target.resolve()), 'foo.com:80');
+ assert.strictEqual(didRenew, 1);
+
+ deferred.resolve([
+ { name: 'bar.com', port: 80, ttl: 5 },
+ ]);
+
+ assert.strictEqual(String(await target.resolve()), 'bar.com:80');
+ assert.strictEqual(didRenew, 1);
+ });
+
+ it('should not wait for renewal to complete when stale data is disallowed', async function () {
+ let time = 10;
+ const target = new Target({ maxStale: 0 });
+
+ target.getTime = () => time;
+ target.resolver = async () => [
+ { name: 'foo.com', port: 80, ttl: 5 },
+ ];
+
+ // Perform the first successful renewal.
+ await target.resolve();
+
+ // If "resolve()" rejects, then we can be sure that it waited for
+ // the renewal to finish.
+ target.renew = async () => {
+ throw new Error('did wait for renewal');
+ };
+
+ // Data is now stale and renewal will need to be performed.
+ time = 16;
+
+ await assertRejects(
+ () => target.resolve(),
+ (err) => {
+ assert.strictEqual(err.message, 'did wait for renewal');
+ },
+ );
+ });
});
});