From 7d2d2f951a034ddf9ef1e2f2c987023293d18a89 Mon Sep 17 00:00:00 2001
From: Lloyd Hilaiel <lloyd@hilaiel.com>
Date: Fri, 30 Dec 2011 18:32:46 -0700
Subject: [PATCH] implement a 'maximum request time' for bcrypt work. server
now fails more gracefully under backbreaking load, returning 503s to clients
of the authenticate_user api. update loadgen to be less dramatic about, but
still display, 503 errors. first part of issue #787 - a partial fix for #785
in dere too
---
bin/load_gen | 21 ++++++++++++++++-----
lib/bcrypt.js | 6 ++++--
lib/configuration.js | 9 ++++++---
lib/load_gen/activities/add_email.js | 1 +
lib/load_gen/user_db.js | 4 ++++
lib/wsapi/authenticate_user.js | 5 +++++
package.json | 2 +-
7 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/bin/load_gen b/bin/load_gen
index 5eb1ea00b..c8ad67f96 100755
--- a/bin/load_gen
+++ b/bin/load_gen
@@ -231,18 +231,27 @@ function poll() {
if (activitiesToRun.indexOf(act) !== -1) {
outstanding[act]++;
activity[act].startFunc(configuration, function(err) {
- if (err) winston.error(err);
outstanding[act]--;
- if (undefined === completed[act]) completed[act] = [ 0, 0 ];
- completed[act][err ? 1 : 0]++;
+ if (undefined === completed[act]) completed[act] = [ 0, 0, 0 ];
+ if (err) {
+ if (err.indexOf('server is too busy') != -1) {
+ completed[act][2]++;
+ } else {
+ completed[act][1]++;
+ winston.error(err);
+ }
+ } else {
+ completed[act][0]++;
+ }
});
} else {
- if (undefined === completed[act]) completed[act] = [ 0, 0 ];
+ if (undefined === completed[act]) completed[act] = [ 0, 0, 0 ];
completed[act][0]++;
}
}
var numErrors = 0;
+ var num503s = 0;
var numStarted = 0;
function updateAverages(elapsed) {
@@ -252,6 +261,7 @@ function poll() {
Object.keys(completed).forEach(function(k) {
numActCompleted += completed[k][0];
numErrors += completed[k][1];
+ num503s += completed[k][2];
});
completed = { };
var avgUsersThisPeriod = (numActCompleted / activitiesPerUserPerSecond) * (elapsed / 1000);
@@ -279,7 +289,8 @@ function poll() {
"\t", averages[1].toFixed(2),
"\t", averages[2].toFixed(2),
"\t", actSumString,
- "\t", numErrors ? "(" + numErrors + " ERRORS!)" : "");
+ "\t", numErrors ? "(" + numErrors + " ERRORS!)" : "",
+ "\t", num503s ? " (" + num503s + " 503s)" : "");
}
// ** how much time has elapsed since the last poll?
diff --git a/lib/bcrypt.js b/lib/bcrypt.js
index a9c679b61..0c613b147 100644
--- a/lib/bcrypt.js
+++ b/lib/bcrypt.js
@@ -1,11 +1,13 @@
const
computecluster = require('compute-cluster'),
logger = require('../lib/logging.js').logger,
-bcrypt = require('bcrypt');
+bcrypt = require('bcrypt'),
+config = require('./configuration.js');
var cc = new computecluster({
module: path.join(__dirname, "bcrypt-compute.js"),
- max_backlog: 100000
+ max_backlog: 100000,
+ max_request_time: config.get('max_compute_duration')
});
cc.on('error', function(e) {
diff --git a/lib/configuration.js b/lib/configuration.js
index 4fb0d277b..92921fcdc 100644
--- a/lib/configuration.js
+++ b/lib/configuration.js
@@ -105,8 +105,10 @@ g_configs.production = {
authentication_duration_ms: (2 * 7 * 24 * 60 * 60 * 1000),
certificate_validity_ms: (24 * 60 * 60 * 1000),
min_time_between_emails_ms: (60 * 1000),
- // may be specified to manipulate the maximum number of compute
- max_compute_processes: undefined
+ // may be specified to manipulate the maximum number of compute processes
+ max_compute_processes: undefined,
+ // return a 503 if a compute process would take over 10s to complete
+ max_compute_duration: 10
};
@@ -124,7 +126,8 @@ g_configs.local = {
authentication_duration_ms: g_configs.production.authentication_duration_ms,
certificate_validity_ms: g_configs.production.certificate_validity_ms,
min_time_between_emails_ms: g_configs.production.min_time_between_emails_ms,
- max_compute_processes: undefined
+ max_compute_processes: undefined,
+ max_compute_duration: 10
};
// test environments are variations on local
diff --git a/lib/load_gen/activities/add_email.js b/lib/load_gen/activities/add_email.js
index d059fdba7..3e5e34e50 100644
--- a/lib/load_gen/activities/add_email.js
+++ b/lib/load_gen/activities/add_email.js
@@ -67,6 +67,7 @@ exports.startFunc = function(cfg, cb) {
cb = (function() {
var _cb = cb;
return function(x) {
+ if (x) userdb.removeLastEmailFromUser(user);
userdb.releaseUser(user);
_cb(x);
};
diff --git a/lib/load_gen/user_db.js b/lib/load_gen/user_db.js
index c6ceff78b..3e15e19ef 100644
--- a/lib/load_gen/user_db.js
+++ b/lib/load_gen/user_db.js
@@ -138,6 +138,10 @@ exports.addEmailToUser = function(user) {
return email;
};
+exports.removeLastEmailFromUser = function(user) {
+ user.emails.pop();
+};
+
exports.addKeyToUserCtx = function(ctx, email) {
// this is simulated. it will need to be real to apply load to
// the verifier, but that in turn will drastically increase the
diff --git a/lib/wsapi/authenticate_user.js b/lib/wsapi/authenticate_user.js
index 4749c8f5a..ae49c96af 100644
--- a/lib/wsapi/authenticate_user.js
+++ b/lib/wsapi/authenticate_user.js
@@ -38,6 +38,11 @@ exports.process = function(req, res) {
statsd.timing('bcrypt.compare_time', reqTime);
if (err) {
+ if (err.indexOf('exceeded') != -1) {
+ logger.warn("max load hit, failing on auth request with 503: " + err);
+ res.status(503);
+ return fail("server is too busy");
+ }
logger.error("error comparing passwords with bcrypt: " + err);
return fail("internal password check error");
} else if (!success) {
diff --git a/package.json b/package.json
index ef87aafb0..b5550b9fa 100644
--- a/package.json
+++ b/package.json
@@ -5,7 +5,7 @@
, "dependencies": {
"JSONSelect": "0.2.1"
, "bcrypt": "0.4.1"
- , "compute-cluster": "0.0.5"
+ , "compute-cluster": "0.0.6"
, "connect": "1.7.2"
, "connect-cookie-session" : "0.0.2"
, "connect-logger-statsd": "0.0.1"
--
GitLab