Merge pull request #1122 from CartoDB/fix-missing-template

Improve named map provider cache
This commit is contained in:
Daniel G. Aubert 2019-09-16 13:37:52 +02:00 committed by GitHub
commit ce884732f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 482 additions and 131 deletions

View File

@ -5,6 +5,8 @@ Released 2019-XX-XX
Announcements:
- Stop caching map template errors in Named Map Provider Cache
- Gather metrics from Named Maps Providers Cache
- Improved efficiency of query samples while instatiating a map (#1120).
- 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.
- Update deps:

View File

@ -29,6 +29,7 @@ const VarnishHttpCacheBackend = require('../cache/backend/varnish_http');
const FastlyCacheBackend = require('../cache/backend/fastly');
const NamedMapProviderCache = require('../cache/named_map_provider_cache');
const NamedMapsCacheEntry = require('../cache/model/named_maps_entry');
const NamedMapProviderReporter = require('../stats/reporter/named-map-provider');
const SqlWrapMapConfigAdapter = require('../models/mapconfig/adapter/sql-wrap-mapconfig-adapter');
const MapConfigNamedLayersAdapter = require('../models/mapconfig/adapter/mapconfig-named-layers-adapter');
@ -161,10 +162,13 @@ module.exports = class ApiRouter {
layergroupAffectedTablesCache
);
['update', 'delete'].forEach(function(eventType) {
templateMaps.on(eventType, namedMapProviderCache.invalidate.bind(namedMapProviderCache));
const namedMapProviderReporter = new NamedMapProviderReporter({
namedMapProviderCache,
intervalInMilliseconds: rendererCacheOpts.statsInterval
});
namedMapProviderReporter.start();
const collaborators = {
analysisStatusBackend,
attributesBackend,

View File

@ -1,39 +1,43 @@
'use strict';
var _ = require('underscore');
var dot = require('dot');
var NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider');
var templateName = require('../backends/template_maps').templateName;
var queue = require('queue-async');
const LruCache = require('lru-cache');
var LruCache = require("lru-cache");
const NamedMapMapConfigProvider = require('../models/mapconfig/provider/named-map-provider');
const { templateName } = require('../backends/template_maps');
function NamedMapProviderCache(
templateMaps,
pgConnection,
metadataBackend,
userLimitsBackend,
mapConfigAdapter,
affectedTablesCache
) {
this.templateMaps = templateMaps;
this.pgConnection = pgConnection;
this.metadataBackend = metadataBackend;
this.userLimitsBackend = userLimitsBackend;
this.mapConfigAdapter = mapConfigAdapter;
this.affectedTablesCache = affectedTablesCache;
const TEN_MINUTES_IN_MILLISECONDS = 1000 * 60 * 10;
const ACTIONS = ['update', 'delete'];
this.providerCache = new LruCache({ max: 2000 });
}
module.exports = class NamedMapProviderCache {
constructor (
templateMaps,
pgConnection,
metadataBackend,
userLimitsBackend,
mapConfigAdapter,
affectedTablesCache
) {
this.templateMaps = templateMaps;
this.pgConnection = pgConnection;
this.metadataBackend = metadataBackend;
this.userLimitsBackend = userLimitsBackend;
this.mapConfigAdapter = mapConfigAdapter;
this.affectedTablesCache = affectedTablesCache;
module.exports = NamedMapProviderCache;
this.providerCache = new LruCache({ max: 2000, maxAge: TEN_MINUTES_IN_MILLISECONDS });
NamedMapProviderCache.prototype.get = function(user, templateId, config, authToken, params, callback) {
var namedMapKey = createNamedMapKey(user, templateId);
var namedMapProviders = this.providerCache.get(namedMapKey) || {};
ACTIONS.forEach(action => templateMaps.on(action, (user, templateId) => this.invalidate(user, templateId)));
}
get (user, templateId, config, authToken, params, callback) {
const namedMapKey = createNamedMapKey(user, templateId);
const namedMapProviders = this.providerCache.get(namedMapKey) || {};
const providerKey = createProviderKey(config, authToken, params);
if (namedMapProviders.hasOwnProperty(providerKey)) {
return callback(null, namedMapProviders[providerKey]);
}
var providerKey = createProviderKey(config, authToken, params);
if (!namedMapProviders.hasOwnProperty(providerKey)) {
namedMapProviders[providerKey] = new NamedMapMapConfigProvider(
this.templateMaps,
this.pgConnection,
@ -47,51 +51,32 @@ NamedMapProviderCache.prototype.get = function(user, templateId, config, authTok
authToken,
params
);
this.providerCache.set(namedMapKey, namedMapProviders);
// early exit, if provider did not exist we just return it
return callback(null, namedMapProviders[providerKey]);
}
var namedMapProvider = namedMapProviders[providerKey];
var self = this;
queue(2)
.defer(namedMapProvider.getTemplate.bind(namedMapProvider))
.defer(this.templateMaps.getTemplate.bind(this.templateMaps), user, templateId)
.awaitAll(function templatesQueueDone(err, results) {
if (err) {
return callback(err);
}
// We want to reset provider its template has changed
// Ideally this should be done in a passive mode where this cache gets notified of template changes
var uniqueFingerprints = _.uniq(results.map(self.templateMaps.fingerPrint)).length;
if (uniqueFingerprints > 1) {
namedMapProvider.reset();
}
return callback(null, namedMapProvider);
});
invalidate (user, templateId) {
this.providerCache.del(createNamedMapKey(user, templateId));
}
};
NamedMapProviderCache.prototype.invalidate = function(user, templateId) {
this.providerCache.del(createNamedMapKey(user, templateId));
};
function createNamedMapKey(user, templateId) {
return user + ':' + templateName(templateId);
function createNamedMapKey (user, templateId) {
return `${user}:${templateName(templateId)}`;
}
var providerKey = '{{=it.authToken}}:{{=it.configHash}}:{{=it.format}}:{{=it.layer}}:{{=it.scale_factor}}';
var providerKeyTpl = dot.template(providerKey);
const providerKeyTpl = ctx => `${ctx.authToken}:${ctx.configHash}:${ctx.format}:${ctx.layer}:${ctx.scale_factor}`;
function createProviderKey(config, authToken, params) {
var tplValues = _.defaults({}, params, {
function createProviderKey (config, authToken, params) {
const defaults = {
authToken: authToken || '',
configHash: NamedMapMapConfigProvider.configHash(config),
layer: '',
format: '',
scale_factor: 1
});
return providerKeyTpl(tplValues);
};
const ctx = Object.assign({}, defaults, params);
return providerKeyTpl(ctx);
}

View File

@ -48,7 +48,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
this.affectedTablesCache = affectedTablesCache;
// providing
this.err = null;
this.mapConfig = null;
this.rendererParams = null;
this.context = {};
@ -56,13 +55,12 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
}
getMapConfig (callback) {
if (!!this.err || this.mapConfig !== null) {
return callback(this.err, this.mapConfig, this.rendererParams, this.context);
if (this.mapConfig !== null) {
return callback(null, this.mapConfig, this.rendererParams, this.context);
}
this.getContext((err, context) => {
if (err) {
this.err = err;
return callback(err);
}
@ -75,8 +73,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
this.config;
} catch (e) {
const err = new Error('malformed config parameter, should be a valid JSON');
this.err = err;
return callback(err);
}
}
@ -85,7 +81,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
this.getTemplate((err, template) => {
if (err) {
this.err = err;
return callback(err);
}
@ -94,7 +89,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
try {
requestMapConfig = this.templateMaps.instance(template, templateParams);
} catch (err) {
this.err = err;
return callback(err);
}
@ -103,7 +97,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
this.mapConfigAdapter.getMapConfig(
user, requestMapConfig, rendererParams, context, (err, mapConfig, stats = {}) => {
if (err) {
this.err = err;
return callback(err);
}
@ -148,7 +141,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
this.userLimitsBackend.getRenderLimits(this.user, this.params.api_key, (err, renderLimits) => {
if (err) {
this.err = err;
return callback(err);
}
@ -163,13 +155,12 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
}
getTemplate (callback) {
if (!!this.err || this.template !== null) {
return callback(this.err, this.template);
if (this.template !== null) {
return callback(null, this.template);
}
this.templateMaps.getTemplate(this.user, this.templateName, (err, tpl) => {
if (err) {
this.err = err;
return callback(err);
}
@ -177,8 +168,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
const error = new Error(`Template '${this.templateName}' of user '${this.user}' not found`);
error.http_status = 404;
this.err = error;
return callback(error);
}
@ -190,15 +179,12 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
const error = new Error('Failed to authorize template');
error.http_status = 403;
this.err = error;
return callback(error);
}
if (!authorized) {
const error = new Error('Unauthorized template instantiation');
error.http_status = 403;
this.err = error;
return callback(error);
}
@ -222,7 +208,6 @@ module.exports = class NamedMapMapConfigProvider extends BaseMapConfigProvider {
this.affectedTables = null;
this.err = null;
this.mapConfig = null;
this.cacheBuster = Date.now();

View File

@ -0,0 +1,33 @@
'use strict';
const statKeyTemplate = ctx => `windshaft.named-map-provider-cache.${ctx.metric}`;
module.exports = class NamedMapProviderReporter {
constructor ({ namedMapProviderCache, intervalInMilliseconds } = {}) {
this.namedMapProviderCache = namedMapProviderCache;
this.intervalInMilliseconds = intervalInMilliseconds;
this.intervalId = null;
}
start () {
const { providerCache: cache } = this.namedMapProviderCache;
const { statsClient: stats } = global;
this.intervalId = setInterval(() => {
stats.gauge(statKeyTemplate({ metric: 'named-map.count' }), cache.length);
const providers = cache.dump();
const namedMapInstantiations = providers.reduce((acc, { v: providers }) => {
acc += Object.keys(providers).length;
return acc;
}, 0);
stats.gauge(statKeyTemplate({ metric: 'named-map.instantiation.count' }), namedMapInstantiations);
}, this.intervalInMilliseconds);
}
stop () {
clearInterval(this.intervalId);
this.intervalId = null;
}
};

View File

@ -0,0 +1,236 @@
'use strict';
require('../support/test_helper');
const request = require('request');
const assert = require('assert');
const Server = require('../../lib/cartodb/server');
const serverOptions = require('../../lib/cartodb/server_options');
const { mapnik } = require('windshaft');
const helper = require('../support/test_helper');
const namedTileUrlTemplate = (ctx) => {
return `http://${ctx.address}/api/v1/map/static/named/${ctx.templateId}/256/256.png?api_key=${ctx.apiKey}`;
};
describe('named map cache regressions', function () {
const server = new Server(serverOptions);
const apiKey = 1234;
const template = {
version: '0.0.1',
name: 'named-map-cache-regression-missing-template',
layergroup: {
version: '1.8.0',
layers: [
{
type: 'cartodb',
options: {
source: {
id: 'a1'
},
cartocss: '#layer{marker-placement: point;marker-width: 5;marker-fill: red;}',
cartocss_version: '2.3.0'
}
}
],
analyses: [
{
id: 'a1',
type: 'source',
params: {
query: 'select * from populated_places_simple_reduced'
}
}
]
}
};
const port = 0; // let the OS to choose a free port
const host = '127.0.0.1';
let listener;
let address;
const keysToDelete = {};
before(function (done) {
listener = server.listen(port, host);
listener.on('error', done);
listener.on('listening', () => {
const { address: host, port } = listener.address();
address = `${host}:${port}`;
done();
});
});
after(function (done) {
helper.deleteRedisKeys(keysToDelete, () => listener.close(done));
});
it('should not fail when a template gets recreated', function (done) {
this.timeout(10000);
const createTemplateRequest = {
url: `http://${address}/api/v1/map/named?api_key=${apiKey}`,
method: 'POST',
headers: {
host: 'localhost',
'Content-Type': 'application/json'
},
body: template,
json: true
};
request(createTemplateRequest, (err, res, body) => {
if (err) {
return done(err);
}
assert.strictEqual(res.statusCode, 200);
const templateId = body.template_id;
keysToDelete['map_tpl|localhost'] = 0;
const previewRequest = {
url: `http://${address}/api/v1/map/static/named/${templateId}/256/256.png?api_key=${apiKey}`,
encoding: 'binary',
method: 'GET',
headers: {
host: 'localhost'
}
};
request(previewRequest, (err, res) => {
if (err) {
return done(err);
}
assert.strictEqual(res.statusCode, 200);
const preview = mapnik.Image.fromBytes(Buffer.from(res.body, 'binary'));
assert.strictEqual(preview.width(), 256);
assert.strictEqual(preview.height(), 256);
const templateUpdate = Object.assign({}, template);
const newQuery = 'select * from populated_places_simple_reduced limit 100';
templateUpdate.layergroup.analyses[0].params.query = newQuery;
const updateTemplateRequest = {
url: `http://${address}/api/v1/map/named/${templateId}?api_key=${apiKey}`,
method: 'PUT',
headers: {
host: 'localhost',
'Content-Type': 'application/json'
},
body: templateUpdate,
json: true
};
request(updateTemplateRequest, (err, res) => {
if (err) {
return done(err);
}
assert.strictEqual(res.statusCode, 200);
request(previewRequest, (err, res) => {
if (err) {
return done(err);
}
const preview = mapnik.Image.fromBytes(Buffer.from(res.body, 'binary'));
assert.strictEqual(preview.width(), 256);
assert.strictEqual(preview.height(), 256);
request(previewRequest, (err, res) => {
if (err) {
return done(err);
}
const preview = mapnik.Image.fromBytes(Buffer.from(res.body, 'binary'));
assert.strictEqual(preview.width(), 256);
assert.strictEqual(preview.height(), 256);
const deleteTemplateRequest = {
url: `http://${address}/api/v1/map/named/${templateId}?api_key=${apiKey}`,
method: 'DELETE',
headers: {
host: 'localhost',
}
};
request(deleteTemplateRequest, (err) => {
if (err) {
return done(err);
}
delete keysToDelete['map_tpl|localhost'];
assert.strictEqual(res.statusCode, 200);
request(createTemplateRequest, (err, res, body) => {
if (err) {
return done(err);
}
assert.strictEqual(res.statusCode, 200);
const templateId = body.template_id;
keysToDelete['map_tpl|localhost'] = 0;
const previewRequest = {
url: namedTileUrlTemplate({ address, templateId, apiKey }),
encoding: 'binary',
method: 'GET',
headers: {
host: 'localhost'
}
};
request(previewRequest, (err, res) => {
if (err) {
return done(err);
}
assert.strictEqual(res.statusCode, 200);
const preview = mapnik.Image.fromBytes(Buffer.from(res.body, 'binary'));
assert.strictEqual(preview.width(), 256);
assert.strictEqual(preview.height(), 256);
request(deleteTemplateRequest, (err) => {
if (err) {
return done(err);
}
delete keysToDelete['map_tpl|localhost'];
assert.strictEqual(res.statusCode, 200);
keysToDelete['user:localhost:mapviews:global'] = 0;
keysToDelete['user:localhost:mapviews:global'] = 5;
helper.deleteRedisKeys(keysToDelete, done);
});
});
});
});
});
});
});
});
});
});
});

View File

@ -1,13 +1,12 @@
'use strict';
require('../support/test_helper');
var RedisPool = require('redis-mpool');
const helper = require('../support/test_helper');
var assert = require('../support/assert');
var mapnik = require('windshaft').mapnik;
var CartodbWindshaft = require('../../lib/cartodb/server');
var serverOptions = require('../../lib/cartodb/server_options');
var TemplateMaps = require('../../lib/cartodb/backends/template_maps.js');
describe('named maps provider cache', function() {
var server;
@ -16,14 +15,8 @@ describe('named maps provider cache', function() {
server = new CartodbWindshaft(serverOptions);
});
// configure redis pool instance to use in tests
var redisPool = new RedisPool(global.environment.redis);
var templateMaps = new TemplateMaps(redisPool, {
max_user_templates: global.environment.maxUserTemplates
});
var username = 'localhost';
const apikey = 1234;
var templateName = 'template_with_color';
var IMAGE_TOLERANCE = 20;
@ -31,7 +24,7 @@ describe('named maps provider cache', function() {
function createTemplate(color) {
return {
version: '0.0.1',
name: templateName,
name: `${templateName}_${color}`,
auth: {
method: 'open'
},
@ -56,17 +49,13 @@ describe('named maps provider cache', function() {
};
}
afterEach(function (done) {
templateMaps.delTemplate(username, templateName, done);
});
function getNamedTile(options, callback) {
function getNamedTile(templateId, options, callback) {
if (!callback) {
callback = options;
options = {};
}
var url = '/api/v1/map/named/' + templateName + '/all/' + [0,0,0].join('/') + '.png';
var url = '/api/v1/map/named/' + templateId + '/all/' + [0,0,0].join('/') + '.png';
var requestOptions = {
url: url,
@ -88,60 +77,116 @@ describe('named maps provider cache', function() {
assert.response(server, requestOptions, expectedResponse, function (res, err) {
var img;
if (statusCode === 200) {
img = mapnik.Image.fromBytes(new Buffer(res.body, 'binary'));
if (res.statusCode === 200) {
img = mapnik.Image.fromBytes(new Buffer.from(res.body, 'binary'));
}
return callback(err, res, img);
});
}
function addTemplate (template, callback) {
const createTemplateRequest = {
url: `/api/v1/map/named?api_key=${apikey}`,
method: 'POST',
headers: {
host: username,
'Content-Type': 'application/json'
},
body: JSON.stringify(template)
};
const expectedResponse = {
status: 200,
headers: {
'Content-Type': 'application/json; charset=utf-8'
}
};
assert.response(server, createTemplateRequest, expectedResponse, (res, err) => {
let template;
if (res.statusCode === 200) {
template = JSON.parse(res.body);
}
return callback(err, res, template);
});
}
function deleteTemplate (templateId, callback) {
const deleteTemplateRequest = {
url: `/api/v1/map/named/${templateId}?api_key=${apikey}`,
method: 'DELETE',
headers: {
host: 'localhost',
}
};
const expectedResponse = {
status: 204,
};
assert.response(server, deleteTemplateRequest, expectedResponse, (res, err) => {
return callback(err, res);
});
}
function previewFixture(color) {
return './test/fixtures/provider/populated_places_simple_reduced-' + color + '.png';
}
var colors = ['red', 'red', 'green', 'blue'];
var colors = ['black', 'red', 'green', 'blue'];
colors.forEach(function(color) {
it('should return an image estimating its bounds based on dataset', function (done) {
templateMaps.addTemplate(username, createTemplate(color), function (err) {
addTemplate(createTemplate(color), function (err, res, template) {
if (err) {
return done(err);
}
getNamedTile(function(err, res, img) {
getNamedTile(template.template_id, function(err, res, img) {
assert.ok(!err);
assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, done);
});
});
});
});
assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, (err) => {
assert.ifError(err);
it('should fail to use template from named map provider after template deletion', function (done) {
var color = 'black';
templateMaps.addTemplate(username, createTemplate(color), function (err) {
if (err) {
return done(err);
}
getNamedTile(function(err, res, img) {
assert.ok(!err);
assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, function(err) {
assert.ok(!err);
templateMaps.delTemplate(username, templateName, function (err) {
assert.ok(!err);
getNamedTile({ statusCode: 404 }, function(err, res) {
assert.ok(!err);
assert.deepEqual(
JSON.parse(res.body).errors,
["Template 'template_with_color' of user 'localhost' not found"]
);
// add template again so it's clean in afterEach
templateMaps.addTemplate(username, createTemplate(color), done);
});
const keysToDelete = {};
keysToDelete['map_tpl|localhost'] = 0;
helper.deleteRedisKeys(keysToDelete, done);
});
});
});
});
});
it('should fail to use template from named map provider after template deletion', function (done) {
const color = 'black';
const templateId = `${templateName}_${color}`;
addTemplate(createTemplate(color), function (err) {
assert.ifError(err);
getNamedTile(templateId, function(err, res, img) {
assert.ifError(err);
assert.imageIsSimilarToFile(img, previewFixture(color), IMAGE_TOLERANCE, function (err) {
assert.ifError(err);
deleteTemplate(templateId, function (err) {
assert.ifError(err);
getNamedTile(templateId, { statusCode: 404 }, function(err, res) {
assert.ifError(err);
assert.deepEqual(
JSON.parse(res.body).errors,
["Template 'template_with_color_black' of user 'localhost' not found"]
);
done();
});
});
});
});
});
});
});

View File

@ -0,0 +1,61 @@
'use strict';
const assert = require('assert');
const NamedMapProviderReporter = require('../../../../../lib/cartodb/stats/reporter/named-map-provider');
describe('named-map-provider-reporter', function () {
it('should report metrics every 100 ms', function (done) {
const oldStatsClient = global.statsClient;
global.statsClient = {
gauge: function (metric, value) {
this[metric] = value;
}
};
const dummyCacheEntries = [
{
k: 'foo:template_1',
v: { 'instantiation_1': 1 }
},
{
k: 'bar:template_2',
v: { 'instantiation_1': 1, 'instantiation_2': 2 }
},
{
k: 'buz:template_3',
v: { 'instantiation_1': 1, 'instantiation_2': 2, 'instantiation_3': 3 }
}
];
const reporter = new NamedMapProviderReporter({
namedMapProviderCache: {
providerCache: {
dump: () => dummyCacheEntries,
length: dummyCacheEntries.length
}
},
intervalInMilliseconds: 100
});
reporter.start();
setTimeout(() => {
reporter.stop();
assert.strictEqual(
global.statsClient['windshaft.named-map-provider-cache.named-map.count'],
3
);
assert.strictEqual(
global.statsClient['windshaft.named-map-provider-cache.named-map.instantiation.count'],
6
);
global.statsClient = oldStatsClient;
done();
}, 110);
});
});