Merge pull request #53 from sharelatex/bg-timeout-on-slow-prev-ops

add a timeout for getPreviousDocOps

services/document-updater/app/coffee/RedisManager.coffee

@@ -104,7 +104,9 @@ module.exports = RedisManager =
 			# check if request took too long and bail out.  only do this for
 			# get, because it is the first call in each update, so if this
 			# passes we'll assume others have a reasonable chance to succeed.
-			return callback(new Error("redis getDoc exceeded timeout")) if timeSpan > MAX_REDIS_REQUEST_LENGTH
+			if timeSpan > MAX_REDIS_REQUEST_LENGTH
+				error = new Error("redis getDoc exceeded timeout")
+				return callback(error)
 			# check sha1 hash value if present
 			if docLines? and storedHash?
 				computedHash = RedisManager._computeHash(docLines)
@@ -141,6 +143,7 @@ module.exports = RedisManager =
 			callback null, version
 
 	getPreviousDocOps: (doc_id, start, end, callback = (error, jsonOps) ->) ->
+		timer = new metrics.Timer("redis.get-prev-docops")
 		rclient.llen keys.docOps(doc_id: doc_id), (error, length) ->
 			return callback(error) if error?
 			rclient.get keys.docVersion(doc_id: doc_id), (error, version) ->
@@ -168,6 +171,10 @@ module.exports = RedisManager =
 						ops = jsonOps.map (jsonOp) -> JSON.parse jsonOp
 					catch e
 						return callback(e)
+					timeSpan = timer.done()
+					if timeSpan > MAX_REDIS_REQUEST_LENGTH
+						error = new Error("redis getPreviousDocOps exceeded timeout")
+						return callback(error)
 					callback null, ops
 
 	DOC_OPS_TTL: 60 * minutes

services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee

@@ -265,6 +265,35 @@ describe "RedisManager", ->
 			it "should log out the problem", ->
 				@logger.warn.called.should.equal true
 
+		describe "with a slow request to redis", ->
+			beforeEach ->
+				@first_version_in_redis = 30
+				@version = 70
+				@length = @version - @first_version_in_redis
+				@start  = 50
+				@end    = 60
+				@ops = [
+					{ "mock": "op-1" },
+					{ "mock": "op-2" }
+				]
+				@jsonOps = @ops.map (op) -> JSON.stringify op
+				@rclient.llen = sinon.stub().callsArgWith(1, null, @length)
+				@rclient.get = sinon.stub().callsArgWith(1, null, @version.toString())
+				@clock = sinon.useFakeTimers();
+				@rclient.lrange = (key, start, end, cb) =>
+					@clock.tick(6000);
+					cb(null, @jsonOps)
+				@RedisManager.getPreviousDocOps(@doc_id, @start, @end, @callback)
+
+			afterEach ->
+				@clock.restore()
+
+			it 'should return an error', ->
+				@callback
+					.calledWith(new Error("redis getPreviousDocOps exceeded timeout"))
+					.should.equal true
+
+
 	describe "updateDocument", ->
 		beforeEach ->
 			@rclient.set = sinon.stub()