From 4ed4b8b35799983de63860f5e0457a079beff86f Mon Sep 17 00:00:00 2001 From: Stephen Hess Date: Wed, 18 Jan 2017 12:56:42 -0500 Subject: [PATCH] added retry support for ES requests that timeout --- app.js | 2 +- controller/search.js | 77 +++++++++++++++++++++++++++++++------------- package.json | 2 ++ routes/v1.js | 36 ++++++++++----------- 4 files changed, 76 insertions(+), 41 deletions(-) diff --git a/app.js b/app.js index 2dabb3c9..40c85964 100644 --- a/app.js +++ b/app.js @@ -23,7 +23,7 @@ var legacy = require('./routes/legacy'); legacy.addRoutes(app, peliasConfig.api); var v1 = require('./routes/v1'); -v1.addRoutes(app, peliasConfig.api); +v1.addRoutes(app, peliasConfig); /** ----------------------- error middleware ----------------------- **/ diff --git a/controller/search.js b/controller/search.js index 8c845938..eb324f10 100644 --- a/controller/search.js +++ b/controller/search.js @@ -1,8 +1,11 @@ +'use strict'; + var _ = require('lodash'); var service = { search: require('../service/search') }; var logger = require('pelias-logger').get('api'); var logging = require( '../helper/logging' ); +const retry = require('retry'); function setup( config, esclient, query ){ function controller( req, res, next ){ @@ -33,6 +36,23 @@ function setup( config, esclient, query ){ return next(); } + logger.debug( '[ES req]', cmd ); + + // options for retry + // default number of retries to 3 (seems reasonable) + // factor of 1 means that each retry attempt will esclient requestTimeout + const operationOptions = { + retries: _.get(esclient, 'transport.maxRetries', 3), + factor: 1, + minTimeout: _.get(esclient, 'transport.requestTimeout') + }; + + // prepend the config timeout since the total number of attempts is maxRetries+1 + const timeouts = [operationOptions.minTimeout].concat(retry.timeouts(operationOptions)); + + // setup a new operation + const operation = retry.operation(operationOptions); + // elasticsearch command var cmd = { index: config.indexName, @@ -40,31 +60,44 @@ function setup( config, esclient, query ){ body: renderedQuery.body }; - logger.debug( '[ES req]', cmd ); + operation.attempt((currentAttempt) => { + // query elasticsearch + service.search( esclient, cmd, function( err, docs, meta ){ + // returns true if the operation should be attempted again + // (handles bookkeeping of maxRetries) + if (operation.retry(err)) { + logger.info('request timed out, retrying'); + return; + } - // query elasticsearch - service.search( esclient, cmd, function( err, docs, meta ){ + // error handler + if( err ){ + if (_.isObject(err) && err.message) { + req.errors.push( err.message ); + } else { + req.errors.push( err ); + } + } + // set response data + else { + // log that a retry was successful + // most requests succeed on first attempt so this declutters log files + if (currentAttempt > 1) { + logger.info(`succeeded on retry ${currentAttempt-1}`); + } + + res.data = docs; + res.meta = meta || {}; + // store the query_type for subsequent middleware + res.meta.query_type = renderedQuery.type; - // error handler - if( err ){ - if (_.isObject(err) && err.message) { - req.errors.push( err.message ); - } else { - req.errors.push( err ); + logger.info(`[controller:search] [queryType:${renderedQuery.type}] [es_result_count:` + + (res.data && res.data.length ? res.data.length : 0)); } - } - // set response data - else { - res.data = docs; - res.meta = meta || {}; - // store the query_type for subsequent middleware - res.meta.query_type = renderedQuery.type; - - logger.info(`[controller:search] [queryType:${renderedQuery.type}] [es_result_count:` + - (res.data && res.data.length ? res.data.length : 0)); - } - logger.debug('[ES response]', docs); - next(); + logger.debug('[ES response]', docs); + next(); + }); + }); } diff --git a/package.json b/package.json index 0bf5d5c6..435c7e68 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,8 @@ "pelias-model": "4.4.0", "pelias-query": "8.11.0", "pelias-text-analyzer": "1.7.0", + "performance-now": "^2.0.0", + "retry": "^0.10.1", "stats-lite": "2.0.3", "through2": "^2.0.3" }, diff --git a/routes/v1.js b/routes/v1.js index 97892a58..97bf7849 100644 --- a/routes/v1.js +++ b/routes/v1.js @@ -68,22 +68,22 @@ function addRoutes(app, peliasConfig) { var routers = { index: createRouter([ - controllers.mdToHTML(peliasConfig, './public/apiDoc.md') + controllers.mdToHTML(peliasConfig.api, './public/apiDoc.md') ]), attribution: createRouter([ - controllers.mdToHTML(peliasConfig, './public/attribution.md') + controllers.mdToHTML(peliasConfig.api, './public/attribution.md') ]), search: createRouter([ sanitizers.search.middleware, middleware.calcSize(), // 3rd parameter is which query module to use, use fallback/geodisambiguation // first, then use original search strategy if first query didn't return anything - controllers.search(peliasConfig, esclient, queries.libpostal), + controllers.search(peliasConfig.api, esclient, queries.libpostal), sanitizers.search_fallback.middleware, - controllers.search(peliasConfig, esclient, queries.fallback_to_old_prod), + controllers.search(peliasConfig.api, esclient, queries.fallback_to_old_prod), postProc.trimByGranularity(), postProc.distances('focus.point.'), - postProc.confidenceScores(peliasConfig), + postProc.confidenceScores(peliasConfig.api), postProc.confidenceScoresFallback(), postProc.dedupe(), postProc.accuracy(), @@ -92,16 +92,16 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), structured: createRouter([ sanitizers.structured_geocoding.middleware, middleware.calcSize(), - controllers.search(peliasConfig, esclient, queries.structured_geocoding), + controllers.search(peliasConfig.api, esclient, queries.structured_geocoding), postProc.trimByGranularityStructured(), postProc.distances('focus.point.'), - postProc.confidenceScores(peliasConfig), + postProc.confidenceScores(peliasConfig.api), postProc.confidenceScoresFallback(), postProc.dedupe(), postProc.accuracy(), @@ -110,14 +110,14 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), autocomplete: createRouter([ sanitizers.autocomplete.middleware, - controllers.search(peliasConfig, esclient, queries.autocomplete), + controllers.search(peliasConfig.api, esclient, queries.autocomplete), postProc.distances('focus.point.'), - postProc.confidenceScores(peliasConfig), + postProc.confidenceScores(peliasConfig.api), postProc.dedupe(), postProc.accuracy(), postProc.localNamingConventions(), @@ -125,13 +125,13 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), reverse: createRouter([ sanitizers.reverse.middleware, middleware.calcSize(), - controllers.search(peliasConfig, esclient, queries.reverse), + controllers.search(peliasConfig.api, esclient, queries.reverse), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first @@ -143,13 +143,13 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), nearby: createRouter([ sanitizers.nearby.middleware, middleware.calcSize(), - controllers.search(peliasConfig, esclient, queries.reverse), + controllers.search(peliasConfig.api, esclient, queries.reverse), postProc.distances('point.'), // reverse confidence scoring depends on distance from origin // so it must be calculated first @@ -161,19 +161,19 @@ function addRoutes(app, peliasConfig) { postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), place: createRouter([ sanitizers.place.middleware, - controllers.place(peliasConfig, esclient), + controllers.place(peliasConfig.api, esclient), postProc.accuracy(), postProc.localNamingConventions(), postProc.renamePlacenames(), postProc.parseBoundingBox(), postProc.normalizeParentIds(), postProc.assignLabels(), - postProc.geocodeJSON(peliasConfig, base), + postProc.geocodeJSON(peliasConfig.api, base), postProc.sendJSON ]), status: createRouter([