From 46190008f59e8ec7203ea1e19f94039a7a0baba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Wed, 24 Jul 2019 17:44:04 +0200 Subject: [PATCH] Remove table cache --- NEWS.md | 5 +- app/controllers/cache_status_controller.js | 29 ----- app/controllers/query_controller.js | 9 +- app/server.js | 8 -- app/services/cached-query-tables.js | 44 -------- app/utils/cache_key_generator.js | 5 - app/utils/no_cache.js | 49 --------- app/utils/table_cache_factory.js | 32 ------ config/environments/development.js.example | 7 -- config/environments/production.js.example | 7 -- config/environments/staging.js.example | 7 -- config/environments/test.js.example | 7 -- test/acceptance/query-tables-api-cache.js | 101 ------------------ .../utils/table_cache_factory.test.js | 41 ------- 14 files changed, 7 insertions(+), 344 deletions(-) delete mode 100644 app/controllers/cache_status_controller.js delete mode 100644 app/services/cached-query-tables.js delete mode 100644 app/utils/cache_key_generator.js delete mode 100644 app/utils/no_cache.js delete mode 100644 app/utils/table_cache_factory.js delete mode 100644 test/acceptance/query-tables-api-cache.js delete mode 100644 test/integration/utils/table_cache_factory.test.js diff --git a/NEWS.md b/NEWS.md index 5dc06be5..08701565 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,8 +1,11 @@ # Changelog -## 3.1.0 +## 4.0.0 Released 2019-mm-dd +Breaking: +* Remove in-memory table cache and `/cachestatus` endpoint. + Announcements: * Update `cartodb-query-tables` to version [`0.5.0`](https://github.com/CartoDB/node-cartodb-query-tables/releases/tag/0.5.0) * Cache control header fine tuning. Set a shorter value for "max-age" directive if there is no way to know when to trigger the invalidation. diff --git a/app/controllers/cache_status_controller.js b/app/controllers/cache_status_controller.js deleted file mode 100644 index 2b7a4cf5..00000000 --- a/app/controllers/cache_status_controller.js +++ /dev/null @@ -1,29 +0,0 @@ -'use strict'; - -var _ = require('underscore'); - -function CacheStatusController(tableCache) { - this.tableCache = tableCache; -} - -CacheStatusController.prototype.route = function (app) { - app.get(global.settings.base_url + '/cachestatus', this.handleCacheStatus.bind(this)); -}; - -CacheStatusController.prototype.handleCacheStatus = function (req, res) { - var tableCacheValues = this.tableCache.values(); - var totalExplainKeys = tableCacheValues.length; - var totalExplainHits = _.reduce(tableCacheValues, function(memo, res) { - return memo + res.hits; - }, 0); - - res.send({ - explain: { - pid: process.pid, - hits: totalExplainHits, - keys : totalExplainKeys - } - }); -}; - -module.exports = CacheStatusController; diff --git a/app/controllers/query_controller.js b/app/controllers/query_controller.js index 4e05324a..a8e2cb46 100644 --- a/app/controllers/query_controller.js +++ b/app/controllers/query_controller.js @@ -3,7 +3,7 @@ var _ = require('underscore'); var step = require('step'); var PSQL = require('cartodb-psql'); -var CachedQueryTables = require('../services/cached-query-tables'); +const QueryTables = require('cartodb-query-tables'); const pgEntitiesAccessValidator = require('../services/pg-entities-access-validator'); var queryMayWrite = require('../utils/query_may_write'); @@ -31,11 +31,10 @@ const cacheControl = Object.assign({ fallbackTtl: FIVE_MINUTES_IN_SECONDS }, global.settings.cache); -function QueryController(metadataBackend, userDatabaseService, tableCache, statsd_client, userLimitsService) { +function QueryController(metadataBackend, userDatabaseService, statsd_client, userLimitsService) { this.metadataBackend = metadataBackend; this.statsd_client = statsd_client; this.userDatabaseService = userDatabaseService; - this.queryTables = new CachedQueryTables(tableCache); this.userLimitsService = userLimitsService; } @@ -135,9 +134,7 @@ QueryController.prototype.handleQuery = function (req, res, next) { var pg = new PSQL(authDbParams); - var skipCache = authorizationLevel === 'master'; - - self.queryTables.getAffectedTablesFromQuery(pg, sql, skipCache, function(err, result) { + QueryTables.getAffectedTablesFromQuery(pg, sql, function (err, result) { if (err) { var errorMessage = (err && err.message) || 'unknown error'; console.error("Error on query explain '%s': %s", sql, errorMessage); diff --git a/app/server.js b/app/server.js index 7e096826..03e0a8b1 100644 --- a/app/server.js +++ b/app/server.js @@ -21,7 +21,6 @@ var Profiler = require('./stats/profiler-proxy'); var _ = require('underscore'); var fs = require('fs'); var mkdirp = require('mkdirp'); -var TableCacheFactory = require('./utils/table_cache_factory'); var RedisPool = require('redis-mpool'); var cartodbRedis = require('cartodb-redis'); @@ -41,7 +40,6 @@ var GenericController = require('./controllers/generic_controller'); var QueryController = require('./controllers/query_controller'); var CopyController = require('./controllers/copy_controller'); var JobController = require('./controllers/job_controller'); -var CacheStatusController = require('./controllers/cache_status_controller'); var HealthCheckController = require('./controllers/health_check_controller'); var VersionController = require('./controllers/version_controller'); @@ -90,8 +88,6 @@ function App(statsClient) { mkdirp.sync(global.settings.tmpDir); } - var tableCache = new TableCacheFactory().build(global.settings); - if ( global.log4js ) { var loggerOpts = { buffer: true, @@ -167,7 +163,6 @@ function App(statsClient) { var queryController = new QueryController( metadataBackend, userDatabaseService, - tableCache, statsClient, userLimitsService ); @@ -190,9 +185,6 @@ function App(statsClient) { ); jobController.route(app); - var cacheStatusController = new CacheStatusController(tableCache); - cacheStatusController.route(app); - var healthCheckController = new HealthCheckController(); healthCheckController.route(app); diff --git a/app/services/cached-query-tables.js b/app/services/cached-query-tables.js deleted file mode 100644 index cdda8051..00000000 --- a/app/services/cached-query-tables.js +++ /dev/null @@ -1,44 +0,0 @@ -'use strict'; - -var QueryTables = require('cartodb-query-tables'); - -var generateMD5 = require('../utils/md5'); - -function CachedQueryTables(tableCache) { - this.tableCache = tableCache; -} - -module.exports = CachedQueryTables; - -CachedQueryTables.prototype.getAffectedTablesFromQuery = function(pg, sql, skipCache, callback) { - var self = this; - - var cacheKey = sqlCacheKey(pg.username(), sql); - - var cachedResult; - if (!skipCache) { - cachedResult = this.tableCache.peek(cacheKey); - } - - if (cachedResult) { - cachedResult.hits++; - return callback(null, cachedResult.result); - } else { - QueryTables.getAffectedTablesFromQuery(pg, sql, function(err, result) { - if (err) { - return callback(err); - } - - self.tableCache.set(cacheKey, { - result: result, - hits: 0 - }); - - return callback(null, result); - }); - } -}; - -function sqlCacheKey(user, sql) { - return user + ':' + generateMD5(sql); -} diff --git a/app/utils/cache_key_generator.js b/app/utils/cache_key_generator.js deleted file mode 100644 index 5f1f4bb5..00000000 --- a/app/utils/cache_key_generator.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict'; - -module.exports = function generateCacheKey(database, affectedTables) { - return database + ":" + affectedTables.join(','); -}; diff --git a/app/utils/no_cache.js b/app/utils/no_cache.js deleted file mode 100644 index 7c268032..00000000 --- a/app/utils/no_cache.js +++ /dev/null @@ -1,49 +0,0 @@ -'use strict'; - -/** - * This module provides an object with the interface of an LRU cache - * but that actually does not store anything. - * - * See https://github.com/isaacs/node-lru-cache/tree/v2.5.0 - */ - -function NoCache() { -} - -module.exports = NoCache; - -NoCache.prototype.set = function (/* key, value */) { - return true; -}; - -NoCache.prototype.get = function (/* key */) { - return undefined; -}; - -NoCache.prototype.peek = function (/* key */) { - return undefined; -}; - -NoCache.prototype.del = function (/* key */) { - return undefined; -}; - -NoCache.prototype.reset = function () { - return undefined; -}; - -NoCache.prototype.has = function (/* key */) { - return false; -}; - -NoCache.prototype.forEach = function (/* fn, thisp */) { - return undefined; -}; - -NoCache.prototype.keys = function () { - return []; -}; - -NoCache.prototype.values = function () { - return []; -}; diff --git a/app/utils/table_cache_factory.js b/app/utils/table_cache_factory.js deleted file mode 100644 index 7c83c8ba..00000000 --- a/app/utils/table_cache_factory.js +++ /dev/null @@ -1,32 +0,0 @@ -'use strict'; - -var LRU = require('lru-cache'); -var NoCache = require('./no_cache'); - -/** - * This module abstracts the creation of a tableCache, - * depending on the configuration passed along - */ - -function TableCacheFactory() { -} - -module.exports = TableCacheFactory; - -TableCacheFactory.prototype.build = function (settings) { - var enabled = settings.tableCacheEnabled || false; - var tableCache = null; - - if(enabled) { - tableCache = LRU({ - // store no more than these many items in the cache - max: settings.tableCacheMax || 8192, - // consider entries expired after these many milliseconds (10 minutes by default) - maxAge: settings.tableCacheMaxAge || 1000*60*10 - }); - } else { - tableCache = new NoCache(); - } - - return tableCache; -}; diff --git a/config/environments/development.js.example b/config/environments/development.js.example index 6ffd9ff5..845f453c 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -85,13 +85,6 @@ module.exports.redisIdleTimeoutMillis = 100; module.exports.redisReapIntervalMillis = 10; module.exports.redisLog = false; -// tableCache settings -module.exports.tableCacheEnabled = false; // false by default -// Max number of entries in the query tables cache -module.exports.tableCacheMax = 8192; -// Max age of query table cache items, in milliseconds -module.exports.tableCacheMaxAge = 1000*60*10; - // Temporary directory, make sure it is writable by server user module.exports.tmpDir = '/tmp'; // change ogr2ogr command or path diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 077e2795..ac663399 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -86,13 +86,6 @@ module.exports.redisIdleTimeoutMillis = 10000; module.exports.redisReapIntervalMillis = 1000; module.exports.redisLog = false; -// tableCache settings -module.exports.tableCacheEnabled = false; // false by default -// Max number of entries in the query tables cache -module.exports.tableCacheMax = 8192; -// Max age of query table cache items, in milliseconds -module.exports.tableCacheMaxAge = 1000*60*10; - // Temporary directory, make sure it is writable by server user module.exports.tmpDir = '/tmp'; // change ogr2ogr command or path diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index ca3b6771..06e91ec5 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -86,13 +86,6 @@ module.exports.redisIdleTimeoutMillis = 10000; module.exports.redisReapIntervalMillis = 1000; module.exports.redisLog = false; -// tableCache settings -module.exports.tableCacheEnabled = false; // false by default -// Max number of entries in the query tables cache -module.exports.tableCacheMax = 8192; -// Max age of query table cache items, in milliseconds -module.exports.tableCacheMaxAge = 1000*60*10; - // Temporary directory, make sure it is writable by server user module.exports.tmpDir = '/tmp'; // change ogr2ogr command or path diff --git a/config/environments/test.js.example b/config/environments/test.js.example index 2ef95be4..5a123e90 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -83,13 +83,6 @@ module.exports.redisIdleTimeoutMillis = 1; module.exports.redisReapIntervalMillis = 1; module.exports.redisLog = false; -// tableCache settings -module.exports.tableCacheEnabled = true; // false by default. We want to make sure some functionallity is there until we fully deprecate it. -// Max number of entries in the query tables cache -module.exports.tableCacheMax = 8192; -// Max age of query table cache items, in milliseconds -module.exports.tableCacheMaxAge = 1000*60*10; - // Temporary directory, make sure it is writable by server user module.exports.tmpDir = '/tmp'; // change ogr2ogr command or path diff --git a/test/acceptance/query-tables-api-cache.js b/test/acceptance/query-tables-api-cache.js deleted file mode 100644 index e18375f3..00000000 --- a/test/acceptance/query-tables-api-cache.js +++ /dev/null @@ -1,101 +0,0 @@ -'use strict'; - -require('../helper'); - -var qs = require('querystring'); - -var server = require('../../app/server')(); -var assert = require('../support/assert'); - -describe('query-tables-api', function() { - - beforeEach(function(done) { - var tableCacheEnabled = global.settings.tableCacheEnabled || false; - if(!tableCacheEnabled) { - this.skip("tableCache is disabled"); - } - done(); - }); - - function getCacheStatus(callback) { - assert.response( - server, - { - method: 'GET', - url: '/api/v1/cachestatus' - }, - { - status: 200 - }, - function(err, res) { - callback(null, JSON.parse(res.body)); - } - ); - } - - var request = { - url: '/api/v1/sql?' + qs.stringify({ - q: 'SELECT * FROM untitle_table_4' - }), - headers: { - host: 'vizzuality.cartodb.com' - }, - method: 'GET' - }; - - var RESPONSE_OK = { - status: 200 - }; - - it('should create a key in affected tables cache', function(done) { - assert.response(server, request, RESPONSE_OK, function(err) { - assert.ok(!err, err); - - getCacheStatus(function(err, cacheStatus) { - assert.ok(!err, err); - assert.equal(cacheStatus.explain.keys, 1); - assert.equal(cacheStatus.explain.hits, 0); - - done(); - }); - }); - }); - - it('should use cache to retrieve affected tables', function(done) { - assert.response(server, request, RESPONSE_OK, function(err) { - assert.ok(!err, err); - - getCacheStatus(function(err, cacheStatus) { - assert.ok(!err, err); - assert.equal(cacheStatus.explain.keys, 1); - assert.equal(cacheStatus.explain.hits, 1); - - done(); - }); - }); - }); - - it('should skip cache to retrieve affected tables', function(done) { - var masterRequest = { - url: '/api/v1/sql?' + qs.stringify({ - q: 'SELECT * FROM untitle_table_4', - api_key: '1234' - }), - headers: { - host: 'vizzuality.cartodb.com' - }, - method: 'GET' - }; - assert.response(server, masterRequest, RESPONSE_OK, function(err) { - assert.ok(!err, err); - - getCacheStatus(function(err, cacheStatus) { - assert.ok(!err, err); - assert.equal(cacheStatus.explain.keys, 1); - assert.equal(cacheStatus.explain.hits, 0); - - done(); - }); - }); - }); -}); diff --git a/test/integration/utils/table_cache_factory.test.js b/test/integration/utils/table_cache_factory.test.js deleted file mode 100644 index 546152c1..00000000 --- a/test/integration/utils/table_cache_factory.test.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; - -require('../../helper'); -var assert = require('assert'); -var LRU = require('lru-cache'); -var NoCache = require('../../../app/utils/no_cache'); - -var TableCacheFactory = require('../../../app/utils/table_cache_factory'); -var factory = new TableCacheFactory(); - -describe('TableCacheFactory', function() { - - it('returns a NoCache by default', function() { - var tableCache = factory.build({}); - assert(tableCache instanceof NoCache); - }); - - it('returns a NoCache if it is disabled in settings', function() { - var tableCache = factory.build({tableCacheEnabled: false}); - assert(tableCache instanceof NoCache); - }); - - it('returns an LRU if enabled in settings, with its default settings', function() { - var tableCache = factory.build({tableCacheEnabled: true}); - assert(tableCache instanceof LRU); - assert.equal(tableCache._max, 8192); - assert.equal(tableCache._maxAge, 1000*60*10); - }); - - it('returns an LRU if enabled in settings, with the passed settings', function() { - var tableCache = factory.build({ - tableCacheEnabled: true, - tableCacheMax: 42, - tableCacheMaxAge: 1000 - }); - assert(tableCache instanceof LRU); - assert.equal(tableCache._max, 42); - assert.equal(tableCache._maxAge, 1000); - }); - -});