From 351e331954b8a1dd32be26f49e6ddbeba28c9dd2 Mon Sep 17 00:00:00 2001
From: Jonathan Niles <jonathanwniles@imaworldhealth.org>
Date: Thu, 4 Aug 2016 12:00:44 +0100
Subject: [PATCH] feat(db): automatically retry txns on deadlocks

This commit implements automatic retrying of transactions when a
transaction deadlock occurs.  This allows the server to handle up to 20
simultaneous patient registrations!  The integration tests have been
updated accordingly to stress the server.

The number of retries is currently 3, but is configurable via the
MAX_TRANSACTION_DEADLOCK_RESTARTS constant in the transaction library.

Closes #620.
---
 server/lib/db/transaction.js    | 27 ++++++++++++++++++++++++++-
 server/lib/helpers/translate.js |  2 ++
 server/lib/renderers/html.js    |  1 +
 server/lib/renderers/json.js    |  2 ++
 server/test/api/patients.js     |  5 ++---
 5 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/server/lib/db/transaction.js b/server/lib/db/transaction.js
index 4b451b36a9..307dce0736 100644
--- a/server/lib/db/transaction.js
+++ b/server/lib/db/transaction.js
@@ -3,6 +3,9 @@
 const q = require('q');
 const winston = require('winston');
 
+/** @const the number of times a transaction is restarted in case of deadlock*/
+const MAX_TRANSACTION_DEADLOCK_RESTARTS = 3;
+
 // Uses an already existing connection to query the database, returning a promise
 function queryConnection(connection, sql, params) {
   const deferred = q.defer();
@@ -52,6 +55,7 @@ class Transaction {
   constructor(db) {
     this.queries = [];
     this.db = db;
+    this.restarts = 0;
   }
 
   /**
@@ -124,9 +128,30 @@ class Transaction {
             // individual query did not work - rollback transaction
             connection.rollback(() => {
               connection.destroy();
-              winston.debug(`[Transaction] Rollback due to : ${error}`);
+              winston.warn(
+                `[Transaction] Encountered error ${error.code}.  Rolling transaction back and recoverying database connections.`
+              );
             });
 
+            // restart transactions a set number of times if the error is due to table deadlocks
+            if (error.code === 'ER_LOCK_DEADLOCK' && this.restarts++ < MAX_TRANSACTION_DEADLOCK_RESTARTS) {
+              winston.warn(
+                `[Transaction] Transacton deadlock discovered. Attempting ${this.restarts} / ${MAX_TRANSACTION_DEADLOCK_RESTARTS} restarts.`
+              );
+
+              // restart transaction
+              return this.execute()
+                .then(results => deferred.resolve(results))
+                .catch(err => deferred.reject(err));
+            }
+
+            // if we get here, all attempted restarts failed.  Report an error in case tables are permanently locked.
+            if (error.code === 'ER_LOCK_DEADLOCK') {
+              winston.error(
+                `[Transaction] Unrecoverable deadlock error. Completed ${this.restarts} / ${MAX_TRANSACTION_DEADLOCK_RESTARTS} restarts.`
+              );
+            }
+
             return deferred.reject(error);
           });
       });
diff --git a/server/lib/helpers/translate.js b/server/lib/helpers/translate.js
index 5ae4c422ee..6861510fa5 100644
--- a/server/lib/helpers/translate.js
+++ b/server/lib/helpers/translate.js
@@ -1,3 +1,5 @@
+'use strict';
+
 const en = require('../../../client/i18n/en.json');
 const fr = require('../../../client/i18n/fr.json');
 
diff --git a/server/lib/renderers/html.js b/server/lib/renderers/html.js
index e812f587a8..efa68aa8e1 100644
--- a/server/lib/renderers/html.js
+++ b/server/lib/renderers/html.js
@@ -6,6 +6,7 @@
  * @requires server/lib/template
  */
 'use strict';
+
 const exhbs = require('express-handlebars');
 
 const hbs = require('../template');
diff --git a/server/lib/renderers/json.js b/server/lib/renderers/json.js
index c2ca1ea178..49008730e7 100644
--- a/server/lib/renderers/json.js
+++ b/server/lib/renderers/json.js
@@ -9,6 +9,8 @@
  *
  * @module  lib/renderers/json
  */
+'use strict';
+
 var q = require('q');
 
 exports.render = renderJSON;
diff --git a/server/test/api/patients.js b/server/test/api/patients.js
index 76d9adb498..abf59194af 100644
--- a/server/test/api/patients.js
+++ b/server/test/api/patients.js
@@ -240,7 +240,7 @@ describe('(/patients) Patients', function () {
     // var NUMBER_OF_PATIENTS = 200;
     // var timeoutInterval = 30;
 
-    var NUMBER_OF_PATIENTS = 2;
+    var NUMBER_OF_PATIENTS = 7;
     var timeoutInterval = 0;
 
     var timeout = 0;
@@ -294,7 +294,7 @@ describe('(/patients) Patients', function () {
 
   // TODO get information on the registered patient - ensure details route is correct
   function delayPatientRequest(timeout, hospitalNo) {
-    var deferred = q.defer();
+    const deferred = q.defer();
 
     setTimeout(function () {
 
@@ -304,7 +304,6 @@ describe('(/patients) Patients', function () {
       agent.post('/patients')
         .send(simultaneousRequest)
         .then(function (res) {
-          console.log('res:', res.body);
           deferred.resolve(res);
         })
         .catch(function (error) {