Do not expose remove from pool after query error as a config option
This commit is contained in:
parent
7c696317d0
commit
d42e5a6b75
3
NEWS.md
3
NEWS.md
@ -3,8 +3,7 @@
|
||||
|
||||
New features:
|
||||
|
||||
* New configuration option `db_pool_destroy_client_on_error`: client is removed from pool after error happens
|
||||
This help to avoid issues with transactions (#241)
|
||||
* Client is removed from pool after error happens. This help to avoid issues with transactions (#241).
|
||||
|
||||
Announcements:
|
||||
|
||||
|
@ -66,8 +66,6 @@ var cdbReq = new CdbRequest();
|
||||
// Set default configuration
|
||||
global.settings.db_pubuser = global.settings.db_pubuser || "publicuser";
|
||||
global.settings.bufferedRows = global.settings.bufferedRows || 1000;
|
||||
global.settings.db_pool_destroy_client_on_error = global.settings.hasOwnProperty('db_pool_destroy_client_on_error') ?
|
||||
global.settings.db_pool_destroy_client_on_error : true;
|
||||
|
||||
var tableCache = LRU({
|
||||
// store no more than these many items in the cache
|
||||
@ -360,7 +358,7 @@ function handleQuery(req, res) {
|
||||
|
||||
checkAborted('queryExplain');
|
||||
|
||||
pg = new PSQL(dbopts, {}, { destroyOnError: global.settings.db_pool_destroy_client_on_error });
|
||||
pg = new PSQL(dbopts, {}, { destroyOnError: true });
|
||||
// get all the tables from Cache or SQL
|
||||
tableCacheItem = tableCache.get(sql_md5);
|
||||
if (tableCacheItem) {
|
||||
|
@ -78,7 +78,7 @@ OgrFormat.prototype.toOGR = function(options, out_format, out_filename, callback
|
||||
|
||||
function fetchColumns() {
|
||||
var colsql = 'SELECT * FROM (' + sql + ') as _cartodbsqlapi LIMIT 0';
|
||||
pg = new PSQL(dbopts, {}, { destroyOnError: global.settings.db_pool_destroy_client_on_error });
|
||||
pg = new PSQL(dbopts, {}, { destroyOnError: true });
|
||||
pg.query(colsql, this);
|
||||
},
|
||||
// jshint maxcomplexity:9
|
||||
|
@ -119,7 +119,7 @@ PostgresFormat.prototype.sendResponse = function(opts, callback) {
|
||||
|
||||
this.start_time = Date.now();
|
||||
|
||||
this.client = new PSQL(opts.dbopts, {}, { destroyOnError: global.settings.db_pool_destroy_client_on_error });
|
||||
this.client = new PSQL(opts.dbopts, {}, { destroyOnError: true });
|
||||
this.client.eventedQuery(sql, function(err, query, queryCanceller) {
|
||||
that.queryCanceller = queryCanceller;
|
||||
if (err) {
|
||||
|
@ -36,8 +36,6 @@ module.exports.db_pool_size = 500;
|
||||
module.exports.db_pool_idleTimeout = 30000;
|
||||
// Milliseconds between idle client checking
|
||||
module.exports.db_pool_reapInterval = 1000;
|
||||
// Whether a client must be removed from the pool after an error happened in the query or not
|
||||
module.exports.db_pool_destroy_client_on_error = true;
|
||||
// max number of bytes for a row, when exceeded the query will throw an error
|
||||
//module.exports.db_max_row_size = 10 * 1024 * 1024;
|
||||
// allows to use an object to connect with node-postgres instead of a connection string
|
||||
|
@ -37,8 +37,6 @@ module.exports.db_pool_size = 500;
|
||||
module.exports.db_pool_idleTimeout = 30000;
|
||||
// Milliseconds between idle client checking
|
||||
module.exports.db_pool_reapInterval = 1000;
|
||||
// Whether a client must be removed from the pool after an error happened in the query or not
|
||||
module.exports.db_pool_destroy_client_on_error = true;
|
||||
// max number of bytes for a row, when exceeded the query will throw an error
|
||||
//module.exports.db_max_row_size = 10 * 1024 * 1024;
|
||||
// allows to use an object to connect with node-postgres instead of a connection string
|
||||
|
@ -37,8 +37,6 @@ module.exports.db_pool_size = 500;
|
||||
module.exports.db_pool_idleTimeout = 30000;
|
||||
// Milliseconds between idle client checking
|
||||
module.exports.db_pool_reapInterval = 1000;
|
||||
// Whether a client must be removed from the pool after an error happened in the query or not
|
||||
module.exports.db_pool_destroy_client_on_error = true;
|
||||
// max number of bytes for a row, when exceeded the query will throw an error
|
||||
//module.exports.db_max_row_size = 10 * 1024 * 1024;
|
||||
// allows to use an object to connect with node-postgres instead of a connection string
|
||||
|
@ -34,8 +34,6 @@ module.exports.db_pool_size = 500;
|
||||
module.exports.db_pool_idleTimeout = 30000;
|
||||
// Milliseconds between idle client checking
|
||||
module.exports.db_pool_reapInterval = 1000;
|
||||
// Whether a client must be removed from the pool after an error happened in the query or not
|
||||
module.exports.db_pool_destroy_client_on_error = true;
|
||||
// max number of bytes for a row, when exceeded the query will throw an error
|
||||
//module.exports.db_max_row_size = 10 * 1024 * 1024;
|
||||
// allows to use an object to connect with node-postgres instead of a connection string
|
||||
|
@ -9,13 +9,12 @@ describe('transaction', function() {
|
||||
var SERVER_PORT = 5554;
|
||||
|
||||
var server;
|
||||
beforeEach(function(done) {
|
||||
before(function(done) {
|
||||
server = require(global.settings.app_root + '/app/controllers/app')();
|
||||
server.listen(SERVER_PORT, '127.0.0.1', done);
|
||||
});
|
||||
|
||||
afterEach(function(done) {
|
||||
global.settings.db_pool_destroy_client_on_error = true;
|
||||
after(function(done) {
|
||||
server.close(done);
|
||||
});
|
||||
|
||||
@ -23,65 +22,30 @@ describe('transaction', function() {
|
||||
headers: { host: 'vizzuality.localhost' }
|
||||
});
|
||||
|
||||
function createRequest(query) {
|
||||
function requestUrl(query) {
|
||||
return 'http://127.0.0.1:' + SERVER_PORT + '/api/v1/sql?' + qs.stringify({ q: query });
|
||||
}
|
||||
|
||||
|
||||
var destroyOnErrorScenarios = [
|
||||
{
|
||||
destroyOnError: true,
|
||||
assertFn: function (done) {
|
||||
return function (err, response, body) {
|
||||
assert.ok(!err);
|
||||
assert.equal(response.statusCode, 200);
|
||||
|
||||
var parsedBody = JSON.parse(body);
|
||||
assert.ok(parsedBody);
|
||||
assert.deepEqual(parsedBody.rows, [{ foo: 1 }]);
|
||||
|
||||
done();
|
||||
};
|
||||
}
|
||||
},
|
||||
{
|
||||
destroyOnError: false,
|
||||
assertFn: function(done) {
|
||||
return function(err, response, body) {
|
||||
assert.ok(!err);
|
||||
assert.equal(response.statusCode, 400);
|
||||
|
||||
var parsedBody = JSON.parse(body);
|
||||
assert.ok(parsedBody);
|
||||
assert.deepEqual(
|
||||
parsedBody.error,
|
||||
['current transaction is aborted, commands ignored until end of transaction block']
|
||||
);
|
||||
|
||||
// do extra request to clean up scenario
|
||||
global.settings.db_pool_destroy_client_on_error = true;
|
||||
sqlRequest(createRequest('select 1'), done);
|
||||
};
|
||||
}
|
||||
}
|
||||
];
|
||||
|
||||
var errorQuery = 'BEGIN; PREPARE _pstm AS select error; EXECUTE _pstm; COMMIT;';
|
||||
|
||||
destroyOnErrorScenarios.forEach(function(scenario) {
|
||||
var shouldOrShouldNot = scenario.destroyOnError ? ' NOT ' : ' ';
|
||||
it('should' + shouldOrShouldNot + 'fail to second request after error in transaction', function(done) {
|
||||
global.settings.db_pool_destroy_client_on_error = scenario.destroyOnError;
|
||||
it('should NOT fail to second request after error in transaction', function(done) {
|
||||
sqlRequest(requestUrl(errorQuery), function(err, response, body) {
|
||||
assert.ok(!err);
|
||||
assert.equal(response.statusCode, 400);
|
||||
|
||||
sqlRequest(createRequest(errorQuery), function(err, response, body) {
|
||||
var parsedBody = JSON.parse(body);
|
||||
assert.ok(parsedBody);
|
||||
assert.deepEqual(parsedBody, { error: ['column "error" does not exist'] });
|
||||
|
||||
sqlRequest(requestUrl('select 1 as foo'), function (err, response, body) {
|
||||
assert.ok(!err);
|
||||
assert.equal(response.statusCode, 400);
|
||||
assert.equal(response.statusCode, 200);
|
||||
|
||||
var parsedBody = JSON.parse(body);
|
||||
assert.ok(parsedBody);
|
||||
assert.deepEqual(parsedBody, { error: ['column "error" does not exist'] });
|
||||
assert.deepEqual(parsedBody.rows, [{ foo: 1 }]);
|
||||
|
||||
sqlRequest(createRequest('select 1 as foo'), scenario.assertFn(done));
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user