From 277d33715c45d7f8f2c0ad09f08136cf6ef12cea Mon Sep 17 00:00:00 2001 From: Jonathan Budzenski Date: Mon, 24 Sep 2018 11:48:03 -0500 Subject: [PATCH] [ping()] determine ping timeout at runtime (#679) * [ping()] determine ping timeout at runtime * generate * single line * remove pingTimeout param * [docs] Add note about pingTimeout priority * remove default ping requestTimeout test * add test --- docs/configuration.asciidoc | 2 +- scripts/generate/js_api.js | 4 ---- src/lib/apis/0_90.js | 1 - src/lib/apis/1_7.js | 1 - src/lib/apis/2_4.js | 1 - src/lib/apis/5_0.js | 1 - src/lib/apis/5_1.js | 1 - src/lib/apis/5_2.js | 1 - src/lib/apis/5_3.js | 1 - src/lib/apis/5_4.js | 1 - src/lib/apis/5_5.js | 1 - src/lib/apis/5_6.js | 1 - src/lib/apis/6_0.js | 1 - src/lib/apis/6_1.js | 1 - src/lib/apis/6_2.js | 1 - src/lib/apis/6_3.js | 1 - src/lib/apis/6_x.js | 20 +++++--------------- src/lib/apis/master.js | 20 +++++--------------- src/lib/transport.js | 7 +++++++ test/unit/specs/client.js | 9 --------- test/unit/specs/transport.js | 22 ++++++++++++++++++++++ 21 files changed, 40 insertions(+), 58 deletions(-) diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index b1897eaf5..a6d701005 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -136,7 +136,7 @@ Default::: `30000` Default::: `60000` -`pingTimeout`[[config-ping-timeout]]:: `Number` -- Milliseconds that a ping request can take before timing out. +`pingTimeout`[[config-ping-timeout]]:: `Number` -- Milliseconds that a ping request can take before timing out. Takes precedence over `requestTimeout` for ping requests. Default::: `3000` diff --git a/scripts/generate/js_api.js b/scripts/generate/js_api.js index e72b630d7..50c67de75 100644 --- a/scripts/generate/js_api.js +++ b/scripts/generate/js_api.js @@ -253,10 +253,6 @@ module.exports = function (branch, done) { spec.bulkBody = true; } - if (name === 'ping') { - spec.requestTimeout = 3000; - } - var urls = _.difference(def.url.paths, overrides.aliases[name]); var urlSignatures = []; urls = _.map(urls, function (url) { diff --git a/src/lib/apis/0_90.js b/src/lib/apis/0_90.js index 6ca8a2469..2cbd98ea4 100644 --- a/src/lib/apis/0_90.js +++ b/src/lib/apis/0_90.js @@ -2890,7 +2890,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/1_7.js b/src/lib/apis/1_7.js index d61cd16d8..146506968 100644 --- a/src/lib/apis/1_7.js +++ b/src/lib/apis/1_7.js @@ -5193,7 +5193,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/2_4.js b/src/lib/apis/2_4.js index 34b03c7df..cdbf4c628 100644 --- a/src/lib/apis/2_4.js +++ b/src/lib/apis/2_4.js @@ -5133,7 +5133,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/5_0.js b/src/lib/apis/5_0.js index d35f30122..a13dcaac7 100644 --- a/src/lib/apis/5_0.js +++ b/src/lib/apis/5_0.js @@ -5563,7 +5563,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/5_1.js b/src/lib/apis/5_1.js index 300c33abc..972f874bb 100644 --- a/src/lib/apis/5_1.js +++ b/src/lib/apis/5_1.js @@ -5691,7 +5691,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/5_2.js b/src/lib/apis/5_2.js index fb3c9a34c..4bce7f17c 100644 --- a/src/lib/apis/5_2.js +++ b/src/lib/apis/5_2.js @@ -5692,7 +5692,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/5_3.js b/src/lib/apis/5_3.js index 1f910db73..8c47f8661 100644 --- a/src/lib/apis/5_3.js +++ b/src/lib/apis/5_3.js @@ -5667,7 +5667,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/5_4.js b/src/lib/apis/5_4.js index d12613584..9e84e3217 100644 --- a/src/lib/apis/5_4.js +++ b/src/lib/apis/5_4.js @@ -5854,7 +5854,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/5_5.js b/src/lib/apis/5_5.js index 4fa4102b7..4308506f0 100644 --- a/src/lib/apis/5_5.js +++ b/src/lib/apis/5_5.js @@ -5858,7 +5858,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/5_6.js b/src/lib/apis/5_6.js index 70f428538..3e687a9df 100644 --- a/src/lib/apis/5_6.js +++ b/src/lib/apis/5_6.js @@ -5936,7 +5936,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/6_0.js b/src/lib/apis/6_0.js index 335d3e000..89dc457db 100644 --- a/src/lib/apis/6_0.js +++ b/src/lib/apis/6_0.js @@ -5553,7 +5553,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/6_1.js b/src/lib/apis/6_1.js index 0144889c0..4b0046cef 100644 --- a/src/lib/apis/6_1.js +++ b/src/lib/apis/6_1.js @@ -5597,7 +5597,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/6_2.js b/src/lib/apis/6_2.js index bb2035235..50b854492 100644 --- a/src/lib/apis/6_2.js +++ b/src/lib/apis/6_2.js @@ -5581,7 +5581,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/6_3.js b/src/lib/apis/6_3.js index 0edaa39e9..989d0acd4 100644 --- a/src/lib/apis/6_3.js +++ b/src/lib/apis/6_3.js @@ -5571,7 +5571,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/6_x.js b/src/lib/apis/6_x.js index edcfbc481..043ba5faf 100644 --- a/src/lib/apis/6_x.js +++ b/src/lib/apis/6_x.js @@ -2700,25 +2700,11 @@ api.indices = namespace(); * * @param {Object} params - An object with parameters used to carry out this action * @param {<>} params.index - The name of the index to scope the operation - * @param {<>} params.preferLocal - With `true`, specify that a local shard should be used if available, with `false`, use a random shard (default: true) - * @param {<>} [params.format=detailed] - Format of the output */ api.indices.prototype.analyze = ca({ params: { index: { type: 'string' - }, - preferLocal: { - type: 'boolean', - name: 'prefer_local' - }, - format: { - type: 'enum', - 'default': 'detailed', - options: [ - 'detailed', - 'text' - ] } }, urls: [ @@ -3391,6 +3377,7 @@ api.indices.prototype.forcemerge = ca({ * @param {<>} [params.expandWildcards=open] - Whether wildcard expressions should get expanded to open or closed indices (default: open) * @param {<>} params.flatSettings - Return settings in flat format (default: false) * @param {<>} params.includeDefaults - Whether to return all default setting for each of the indices. + * @param {<>} params.masterTimeout - Specify timeout for connection to master * @param {<>, <>, <>} params.index - A comma-separated list of index names */ api.indices.prototype.get = ca({ @@ -3425,6 +3412,10 @@ api.indices.prototype.get = ca({ type: 'boolean', 'default': false, name: 'include_defaults' + }, + masterTimeout: { + type: 'time', + name: 'master_timeout' } }, url: { @@ -5598,7 +5589,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/apis/master.js b/src/lib/apis/master.js index 4858c1631..258ef5250 100644 --- a/src/lib/apis/master.js +++ b/src/lib/apis/master.js @@ -2761,25 +2761,11 @@ api.indices = namespace(); * * @param {Object} params - An object with parameters used to carry out this action * @param {<>} params.index - The name of the index to scope the operation - * @param {<>} params.preferLocal - With `true`, specify that a local shard should be used if available, with `false`, use a random shard (default: true) - * @param {<>} [params.format=detailed] - Format of the output */ api.indices.prototype.analyze = ca({ params: { index: { type: 'string' - }, - preferLocal: { - type: 'boolean', - name: 'prefer_local' - }, - format: { - type: 'enum', - 'default': 'detailed', - options: [ - 'detailed', - 'text' - ] } }, urls: [ @@ -3442,6 +3428,7 @@ api.indices.prototype.forcemerge = ca({ * @param {<>} [params.expandWildcards=open] - Whether wildcard expressions should get expanded to open or closed indices (default: open) * @param {<>} params.flatSettings - Return settings in flat format (default: false) * @param {<>} params.includeDefaults - Whether to return all default setting for each of the indices. + * @param {<>} params.masterTimeout - Specify timeout for connection to master * @param {<>, <>, <>} params.index - A comma-separated list of index names */ api.indices.prototype.get = ca({ @@ -3476,6 +3463,10 @@ api.indices.prototype.get = ca({ type: 'boolean', 'default': false, name: 'include_defaults' + }, + masterTimeout: { + type: 'time', + name: 'master_timeout' } }, url: { @@ -5662,7 +5653,6 @@ api.ping = ca({ url: { fmt: '/' }, - requestTimeout: 3000, method: 'HEAD' }); diff --git a/src/lib/transport.js b/src/lib/transport.js index 2efdb20e1..052015512 100644 --- a/src/lib/transport.js +++ b/src/lib/transport.js @@ -37,6 +37,7 @@ function Transport(config) { // setup requestTimeout default self.requestTimeout = config.hasOwnProperty('requestTimeout') ? config.requestTimeout : 30000; + self.pingTimeout = config.hasOwnProperty('pingTimeout') ? config.pingTimeout : 3000; if (config.hasOwnProperty('defer')) { self.defer = config.defer; @@ -203,6 +204,12 @@ Transport.prototype.request = function (params, cb) { requestTimeout = params.requestTimeout; } + const pingRequest = params.path === '/' && params.method === 'HEAD'; + if (pingRequest) { + const requestParam = params.hasOwnProperty('requestTimeout') && params.requestTimeout; + requestTimeout = requestParam || this.pingTimeout; + } + params.req = { method: params.method, path: params.path || '/', diff --git a/test/unit/specs/client.js b/test/unit/specs/client.js index c16b09488..06ca8b1ca 100644 --- a/test/unit/specs/client.js +++ b/test/unit/specs/client.js @@ -81,13 +81,4 @@ describe('Client instances creation', function () { client.transport.log.error(new Error()); }); }); - - describe('#ping', function () { - it('sets the default requestTimeout to 3000', function () { - stub(client.transport, 'request'); - client.ping(); - expect(client.transport.request.callCount).to.be(1); - expect(client.transport.request.lastCall.args[0].requestTimeout).to.be(3000); - }); - }); }); diff --git a/test/unit/specs/transport.js b/test/unit/specs/transport.js index 13798679e..ae3af0de3 100644 --- a/test/unit/specs/transport.js +++ b/test/unit/specs/transport.js @@ -788,6 +788,28 @@ describe('Transport Class', function () { }); }); + it('inherits the pingTimeout from the transport', function () { + var clock = sinon.useFakeTimers('setTimeout', 'clearTimeout'); + stub.autoRelease(clock); + var tran = new Transport({ + requestTimeout: 4000, + pingTimeout: 5000 + }); + + var prom = tran.request({ + path: '/', + method: 'HEAD' + }); + // disregard promise, prevent bluebird's warnings + prom.then(_.noop, _.noop); + + expect(_.size(clock.timers)).to.eql(1); + _.each(clock.timers, function (timer, id) { + expect(timer.callAt).to.eql(5000); + clearTimeout(id); + }); + }); + _.each([false, 0, null], function (falsy) { it('skips the timeout when it is ' + falsy, function () {