Release 1.8.2

This is again for #157 without closing it

NEWS.md

@@ -1,3 +1,18 @@
+1.8.2 -- 2014-02-25
+-------------------
+
+Enhancements:
+
+ * Allow using ":host" as part of statsd.prefix (#153)
+ * Expand "addCacheChannel" stats
+ * Allow using GET with sql-api for queries shorter than configured len (#155)
+   [ new sqlapi.max_get_sql_length directive, defaults to 2048 ]
+ * Do not log an error for a legit request requiring no X-Cache-Channel
+
+Bug fixes:
+
+ * Fix munin plugin after log format changes (#154)
+
 1.8.1 -- 2014-02-19
 -------------------
 

config/environments/development.js.example

@@ -102,7 +102,11 @@ var config = {
         // the cartodb username and passed to
         // SQL-API requests in the Host HTTP header
         domain: 'localhost.lan',
-        version: 'v1'
+        version: 'v1',
+        // Maximum lenght of SQL query for GET
+        // requests. Longer queries will be sent
+        // using POST. Defaults to 2048
+        max_get_sql_length: 2048
     }
     ,varnish: {
         host: 'localhost',

config/environments/production.js.example

@@ -58,7 +58,7 @@ var config = {
     ,statsd: {
         host: 'localhost',
         port: 8125,
-        prefix: '', // could be hostname, better not containing dots
+        prefix: ':host.', // could be hostname, better not containing dots
         // support all allowed node-statsd options
     }
     ,renderer: {
@@ -96,7 +96,11 @@ var config = {
         // the cartodb username and passed to
         // SQL-API requests in the Host HTTP header
         domain: 'cartodb.com',
-        version: 'v2'
+        version: 'v2',
+        // Maximum lenght of SQL query for GET
+        // requests. Longer queries will be sent
+        // using POST. Defaults to 2048
+        max_get_sql_length: 2048
     }
     ,varnish: {
         host: 'localhost',

config/environments/staging.js.example

@@ -58,7 +58,7 @@ var config = {
     ,statsd: {
         host: 'localhost',
         port: 8125,
-        prefix: 'stage.'
+        prefix: 'stage.:host.'
         // support all allowed node-statsd options
     }
     ,renderer: {
@@ -96,7 +96,11 @@ var config = {
         // the cartodb username and passed to
         // SQL-API requests in the Host HTTP header
         domain: 'cartodb.com',
-        version: 'v2'
+        version: 'v2',
+        // Maximum lenght of SQL query for GET
+        // requests. Longer queries will be sent
+        // using POST. Defaults to 2048
+        max_get_sql_length: 2048
     }
     ,varnish: {
         host: 'localhost',

config/environments/test.js.example

@@ -38,10 +38,10 @@ var config = {
     ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler])'
     // Templated database username for authorized user
     // Supported labels: 'user_id' (read from redis)
-    ,postgres_auth_user: 'test_cartodb_user_<%= user_id %>'
+    ,postgres_auth_user: 'test_windshaft_cartodb_user_<%= user_id %>'
     // Templated database password for authorized user
     // Supported labels: 'user_id', 'user_password' (both read from redis)
-    ,postgres_auth_pass: 'test_cartodb_user_<%= user_id %>_pass'
+    ,postgres_auth_pass: 'test_windshaft_cartodb_user_<%= user_id %>_pass'
     ,postgres: {
         // Parameters to pass to datasource plugin of mapnik
         // See http://github.com/mapnik/mapnik/wiki/PostGIS
@@ -58,7 +58,7 @@ var config = {
     ,statsd: {
         host: 'localhost',
         port: 8125,
-        prefix: 'test.'
+        prefix: 'test.:host.'
         // support all allowed node-statsd options
     }
     ,renderer: {
@@ -98,7 +98,11 @@ var config = {
         domain: 'donot_look_this_up',
         // This port will be used by "make check" for testing purposes
         // It must be available
-        version: 'v1'
+        version: 'v1',
+        // Maximum lenght of SQL query for GET
+        // requests. Longer queries will be sent
+        // using POST. Defaults to 2048
+        max_get_sql_length: 2048
     }
     ,varnish: {
         host: '',
@@ -109,7 +113,7 @@ var config = {
     // If useProfiler is true every response will be served with an
     // X-Tiler-Profile header containing elapsed timing for various 
     // steps taken for producing the response.
-    ,useProfiler:false
+    ,useProfiler:true
 };
 
 module.exports = config;

lib/cartodb/cartodb_windshaft.js

@@ -7,11 +7,22 @@ var _ = require('underscore')
     , cartoData  = require('cartodb-redis')(global.environment.redis)
     , SignedMaps = require('./signed_maps.js')
     , TemplateMaps = require('./template_maps.js')
-    , Cache = require('./cache_validator');
+    , Cache = require('./cache_validator')
+    , os = require('os')
+;
 
 var CartodbWindshaft = function(serverOptions) {
    var debug = global.environment.debug;
 
+    // Perform keyword substitution in statsd
+    // See https://github.com/CartoDB/Windshaft-cartodb/issues/153
+    if ( global.environment.statsd ) {
+      if ( global.environment.statsd.prefix ) {
+        var host_token = os.hostname().split('.').reverse().join('.');
+        global.environment.statsd.prefix = global.environment.statsd.prefix.replace(/:host/, host_token);
+      }
+    }
+
     if(serverOptions.cache_enabled) {
         console.log("cache invalidation enabled, varnish on ", serverOptions.varnish_host, ' ', serverOptions.varnish_port);
         Cache.init(serverOptions.varnish_host, serverOptions.varnish_port, serverOptions.varnish_secret);
@@ -64,6 +75,11 @@ var CartodbWindshaft = function(serverOptions) {
             // creation
             return false;
           }
+          if ( ! req.params ) {
+            // service requests (/version, /) 
+            // have no need for an X-Cache-Channel
+            return false;
+          }
           if ( statusCode != 200 ) {
             // We do not want to cache
             // unsuccessful responses
@@ -72,7 +88,6 @@ var CartodbWindshaft = function(serverOptions) {
           serverOptions.addCacheChannel(that, req, this);
         },
         function sendResponse(err, added) {
-          if (added && req.profiler) req.profiler.done('addCacheChannel');
           ws_sendResponse.apply(that, thatArgs);
         }
       );

lib/cartodb/server_options.js

@@ -115,14 +115,23 @@ module.exports = function(){
       // See http://nodejs.org/api/http.html#http_agent_maxsockets
       //
       var maxSockets = global.environment.maxConnections || 128;
-      request.post({
-          url:sqlapi, body:qs, json:true,
-          headers:{host: sqlapihostname}
-          // http://nodejs.org/api/http.html#http_agent_maxsockets
-          ,pool:{maxSockets:maxSockets}
-          //,timeout:100
-        }, function(err, res, body)
-      {
+      var maxGetLen = api.max_get_sql_length || 2048;
+      var reqSpec = {
+            url:sqlapi,
+            json:true,
+            headers:{host: sqlapihostname}
+            // http://nodejs.org/api/http.html#http_agent_maxsockets
+            ,pool:{maxSockets:maxSockets}
+            //,timeout:100
+      }
+      if ( sql.length > maxGetLen ) {
+        reqSpec.method = 'POST';
+        reqSpec.body = qs;
+      } else {
+        reqSpec.method = 'GET';
+        reqSpec.qs = qs;
+      }
+      request(reqSpec, function(err, res, body) {
           if (err){
             console.log('ERROR connecting to SQL API on ' + sqlapi + ': ' + err);
             callback(err);
@@ -199,10 +208,8 @@ module.exports = function(){
 
     me.generateCacheChannel = function(app, req, callback){
 
-      // use key to call sql api with sql request if present, else
-      // just return dbname and table name base key
+      // Build channelCache key
       var dbName = req.params.dbname;
-
       var cacheKey = [ dbName ];
       if ( req.params.token ) cacheKey.push(req.params.token);
       else if ( req.params.sql ) cacheKey.push( me.generateMD5(req.params.sql) );
@@ -236,6 +243,7 @@ module.exports = function(){
                 mapStore.load(req.params.token, this);
               },
               function getSQL(err, mapConfig) {
+                if (req.profiler) req.profiler.done('mapStore_load');
                 if ( err ) throw err;
                 var sql = [];
                 _.each(mapConfig.obj().layers, function(lyr) {
@@ -276,6 +284,9 @@ module.exports = function(){
         },
         function buildCacheChannel(err, tableNames) {
           if ( err ) throw err;
+          if (req.profiler && ! req.params.table ) {
+            req.profiler.done('affectedTables');
+          }
 
           var dbName     = req.params.dbname;
           var cacheChannel = me.buildCacheChannel(dbName,tableNames);
@@ -304,6 +315,7 @@ module.exports = function(){
     me.addCacheChannel = function(app, req, cb) {
         // skip non-GET requests, or requests for which there's no response
         if ( req.method != 'GET' || ! req.res ) { cb(null, null); return; }
+        if (req.profiler) req.profiler.start('addCacheChannel');
         var res = req.res;
         var cache_policy = req.query.cache_policy;
         if ( req.params.token ) cache_policy = 'persist';
@@ -326,6 +338,8 @@ module.exports = function(){
         res.header('Last-Modified', lastUpdated.toUTCString());
 
         me.generateCacheChannel(app, req, function(err, channel){
+            if (req.profiler) req.profiler.done('generateCacheChannel');
+            if (req.profiler) req.profiler.end();
             if ( ! err ) {
               res.header('X-Cache-Channel', channel);
               cb(null, channel);

npm-shrinkwrap.json

@@ -1,6 +1,6 @@
 {
   "name": "windshaft-cartodb",
-  "version": "1.8.1",
+  "version": "1.8.2",
   "dependencies": {
     "node-varnish": {
       "version": "0.2.0",
@@ -429,12 +429,12 @@
         }
       }
     },
-    "strftime": {
-      "version": "0.6.2"
-    },
     "redis": {
       "version": "0.8.6"
     },
+    "strftime": {
+      "version": "0.6.2"
+    },
     "semver": {
       "version": "1.1.4"
     },

package.json

@@ -1,7 +1,7 @@
 {
     "private": true,
     "name": "windshaft-cartodb",
-    "version": "1.8.1",
+    "version": "1.8.2",
     "description": "A map tile server for CartoDB",
     "keywords": [
       "cartodb"

test/acceptance/multilayer.js

@@ -35,6 +35,10 @@ suite('multilayer', function() {
     var expected_last_updated_epoch = 1234567890123; // this is hard-coded into SQLAPIEmu
     var expected_last_updated = new Date(expected_last_updated_epoch).toISOString();
 
+    var test_user = _.template(global.environment.postgres_auth_user, {user_id:1});
+    var test_pubuser = global.environment.postgres.user;
+    var test_database = test_user + '_db';
+
     suiteSetup(function(done){
       sqlapi_server = new SQLAPIEmu(global.environment.sqlapi.port, done);
     });
@@ -108,7 +112,7 @@ suite('multilayer', function() {
               // Check X-Cache-Channel
               cc = res.headers['x-cache-channel'];
               assert.ok(cc); 
-              var dbname = 'test_cartodb_user_1_db'
+              var dbname = test_database;
               assert.equal(cc.substring(0, dbname.length), dbname);
               var jsonquery = cc.substring(dbname.length+1);
               var sentquery = JSON.parse(jsonquery);
@@ -238,7 +242,7 @@ suite('multilayer', function() {
               // Check X-Cache-Channel
               var cc = res.headers['x-cache-channel'];
               assert.ok(cc); 
-              var dbname = 'test_cartodb_user_1_db'
+              var dbname = test_database;
               assert.equal(cc.substring(0, dbname.length), dbname);
               var jsonquery = cc.substring(dbname.length+1);
               var sentquery = JSON.parse(jsonquery);
@@ -271,7 +275,7 @@ suite('multilayer', function() {
               // Check X-Cache-Channel
               var cc = res.headers['x-cache-channel'];
               assert.ok(cc); 
-              var dbname = 'test_cartodb_user_1_db'
+              var dbname = test_database;
               assert.equal(cc.substring(0, dbname.length), dbname);
               var jsonquery = cc.substring(dbname.length+1);
               var sentquery = JSON.parse(jsonquery);
@@ -563,7 +567,7 @@ suite('multilayer', function() {
               // Check X-Cache-Channel
               var cc = res.headers['x-cache-channel'];
               assert.ok(cc); 
-              var dbname = 'test_cartodb_user_1_db'
+              var dbname = test_database;
               assert.equal(cc.substring(0, dbname.length), dbname);
               next(err);
           });
@@ -728,7 +732,7 @@ suite('multilayer', function() {
           // Check X-Cache-Channel
           var cc = res.headers['x-cache-channel'];
           assert.ok(cc, "Missing X-Cache-Channel"); 
-          var dbname = 'test_cartodb_user_1_db'
+          var dbname = test_database;
           assert.equal(cc.substring(0, dbname.length), dbname);
           return null;
         },
@@ -758,7 +762,7 @@ suite('multilayer', function() {
           // Check X-Cache-Channel
           var cc = res.headers['x-cache-channel'];
           assert.ok(cc, "Missing X-Cache-Channel on restart");
-          var dbname = 'test_cartodb_user_1_db'
+          var dbname = test_database;
           assert.equal(cc.substring(0, dbname.length), dbname);
           return null;
         },
@@ -1059,10 +1063,12 @@ suite('multilayer', function() {
           var parsedBody = JSON.parse(res.body);
           var token_components = parsedBody.layergroupid.split(':');
           expected_token = token_components[0];
+          var last_request = sqlapi_server.getLastRequest();
+          assert.equal(last_request.method, 'POST');
           return null;
         },
         function cleanup(err) {
-          if ( err ) errors.push(err.message);
+          if ( err ) errors.push('' + err);
           if ( ! expected_token ) return null;
           var next = this;
           redis_client.keys("map_cfg|" + expected_token, function(err, matches) {

test/acceptance/server.js

@@ -22,7 +22,7 @@ suite('server', function() {
     var sqlapi_server;
 
     var mapnik_version = global.environment.mapnik_version || mapnik.versions.mapnik;
-    var test_database = 'test_cartodb_user_1_db';
+    var test_database = _.template(global.environment.postgres_auth_user, {user_id:1}) + '_db';
     var default_style;
     if ( semver.satisfies(mapnik_version, '<2.1.0') ) {
       // 2.0.0 default
@@ -53,12 +53,25 @@ suite('server', function() {
     
     // TODO: I guess this should be a 404 instead...
     test("get call to server returns 200", function(done){
-        assert.response(server, {
-            url: '/',
-            method: 'GET'
-        },{
-            status: 200
-        }, function() { done(); });
+      Step(
+        function doGet() {
+          var next = this;
+          assert.response(server, {
+              url: '/',
+              method: 'GET'
+          },{}, function(res, err) { next(err,res); });
+        },
+        function doCheck(err, res) {
+          if ( err ) throw err;
+          assert.ok(res.statusCode, 200);
+          var cc = res.headers['x-cache-channel'];
+          assert.ok(!cc);
+          return null;
+        },
+        function finish(err) {
+          done(err);
+        }
+      );
     });
 
     /////////////////////////////////////////////////////////////////////////////////
@@ -175,7 +188,8 @@ suite('server', function() {
         },
         function setupRedisBase(err, matches) {
           if ( err ) throw err;
-          assert.equal(matches.length, 0);
+          assert.equal(matches.length, 0,
+            'Unexpected redis keys at test start: ' + matches.join("\n"));
           redis_client.set(base_key, 
             JSON.stringify({ style: style }),
           this);
@@ -1112,7 +1126,7 @@ suite('server', function() {
             assert.equal(ct, 'image/png');
             var cc = res.headers['x-cache-channel'];
             assert(cc, 'Missing X-Cache-Channel');
-            var dbname = 'test_cartodb_user_1_db'
+            var dbname = test_database;
             assert.equal(cc.substring(0, dbname.length), dbname);
             var jsonquery = cc.substring(dbname.length+1);
             var sentquery = JSON.parse(jsonquery);
@@ -1148,6 +1162,7 @@ suite('server', function() {
             assert.ok(last_request);
             var host = last_request.headers['host'];
             assert.ok(host);
+            assert.equal(last_request.method, 'GET');
             assert.equal(host, 'localhost.donot_look_this_up');
             return null;
           },

test/unit/cartodb/req2params.test.js

@@ -8,6 +8,11 @@ suite('req2params', function() {
 
     // configure redis pool instance to use in tests
     var opts = require('../../../lib/cartodb/server_options')();
+
+    var test_user = _.template(global.environment.postgres_auth_user, {user_id:1});
+    var test_pubuser = global.environment.postgres.user;
+    var test_database = test_user + '_db';
+
     
     test('can be found in server_options', function(){
       assert.ok(_.isFunction(opts.req2params));
@@ -20,8 +25,8 @@ suite('req2params', function() {
           assert.ok(!req.query.hasOwnProperty('dbuser'), 'dbuser was removed from query');
           assert.ok(req.hasOwnProperty('params'), 'request has params');
           assert.ok(req.params.hasOwnProperty('interactivity'), 'request params have interactivity');
-          assert.equal(req.params.dbname, 'test_cartodb_user_1_db', 'could forge dbname: '+ req.params.dbname);
-          assert.ok(req.params.dbuser === 'testpublicuser', 'could inject dbuser ('+req.params.dbuser+')');
+          assert.equal(req.params.dbname, test_database, 'could forge dbname: '+ req.params.dbname);
+          assert.ok(req.params.dbuser === test_pubuser, 'could inject dbuser ('+req.params.dbuser+')');
           done();
       });
     });
@@ -34,10 +39,8 @@ suite('req2params', function() {
           assert.ok(!req.query.hasOwnProperty('dbuser'), 'dbuser was removed from query');
           assert.ok(req.hasOwnProperty('params'), 'request has params');
           assert.ok(req.params.hasOwnProperty('interactivity'), 'request params have interactivity');
-          // database_name for user "localhost" (see test/support/prepare_db.sh)
-          assert.equal(req.params.dbname, 'test_cartodb_user_1_db');
-          // unauthenticated request gets no dbuser
-          assert.ok(req.params.dbuser === 'testpublicuser', 'could inject dbuser ('+req.params.dbuser+')');
+          assert.equal(req.params.dbname, test_database);
+          assert.ok(req.params.dbuser === test_pubuser, 'could inject dbuser ('+req.params.dbuser+')');
           done();
       });
     });
@@ -50,14 +53,12 @@ suite('req2params', function() {
           assert.ok(!req.query.hasOwnProperty('dbuser'), 'dbuser was removed from query');
           assert.ok(req.hasOwnProperty('params'), 'request has params');
           assert.ok(req.params.hasOwnProperty('interactivity'), 'request params have interactivity');
-          // database_name for user "localhost" (see test/support/prepare_db.sh)
-          assert.equal(req.params.dbname, 'test_cartodb_user_1_db');
-          // id for user "localhost" (see test/support/prepare_db.sh)
-          assert.equal(req.params.dbuser, 'test_cartodb_user_1');
+          assert.equal(req.params.dbname, test_database);
+          assert.equal(req.params.dbuser, test_user);
  
           opts.req2params({headers: { host:'localhost' }, query: {map_key: '1235'} }, function(err, req) {
               // wrong key resets params to no user
-              assert.ok(req.params.dbuser === 'testpublicuser', 'could inject dbuser ('+req.params.dbuser+')');
+              assert.ok(req.params.dbuser === test_pubuser, 'could inject dbuser ('+req.params.dbuser+')');
               done();
           });
       });

tools/munin/windshaft

@@ -68,7 +68,7 @@ for pid in ${pids}; do
   log=$(grep "${pid}" "${tmpreport}" | grep -w 1w | awk '{print $9}')
   if test -e "${log}"; then
     kill -USR2 "${pid}"
-    cnt=$(tac ${log} | sed -n -e '/ItemKey/p;/^RenderCache/q' | wc -l)
+    cnt=$(tac ${log} | sed -n -e '/ItemKey/p;/ RenderCache /q' | wc -l)
     if test $cnt -gt $maxcache; then maxcache=$cnt; fi
   else
     # report the error...