Merge pull request #1384 from sharelatex/bg-use-queued-lock

use queued locks

GitOrigin-RevId: b5a3bfb55af201392a50ffb3fe30e740e56da30d

services/web/app/coffee/infrastructure/LockManager.coffee

@@ -5,12 +5,15 @@ rclient = RedisWrapper.client("lock")
 logger = require "logger-sharelatex"
 os = require "os"
 crypto = require "crypto"
+async = require "async"
 
 HOST = os.hostname()
 PID = process.pid
 RND = crypto.randomBytes(4).toString('hex')
 COUNT = 0
 
+LOCK_QUEUES = new Map() # queue lock requests for each name/id so they get the lock on a first-come first-served basis
+
 module.exports = LockManager =
 	LOCK_TEST_INTERVAL: 50 # 50ms between each test of the lock
 	MAX_TEST_INTERVAL: 1000 # back off to 1s between each test of the lock
@@ -68,7 +71,34 @@ module.exports = LockManager =
 				logger.log key: key, redis_response: gotLock, "lock is locked"
 				callback err, false
 
+	# it's sufficient to serialize within a process because that is where the parallel operations occur
 	_getLock: (key, namespace, callback = (error, lockValue) ->) ->
+		# this is what we need to do for each lock we want to request
+		task = (next) ->
+			LockManager._getLockByPolling key, namespace, (error, lockValue) ->
+				# tell the queue to start trying to get the next lock (if any)
+				next()
+				# we have got a lock result, so we can continue with our own execution
+				callback(error, lockValue)
+		# create a queue for this key if needed
+		queueName = "#{key}:#{namespace}"
+		queue = LOCK_QUEUES.get queueName
+		if !queue?
+			handler = (fn, cb) ->
+				fn(cb) # execute any function as our task
+			# set up a new queue for this key
+			queue = async.queue handler, 1
+			queue.push task
+			# remove the queue object when queue is empty
+			queue.drain = () ->
+				LOCK_QUEUES.delete queueName
+			# store the queue in our global map
+			LOCK_QUEUES.set queueName, queue
+		else
+			# queue the request to get the lock
+			queue.push task
+
+	_getLockByPolling: (key, namespace, callback = (error, lockValue) ->) ->
 		startTime = Date.now()
 		testInterval = LockManager.LOCK_TEST_INTERVAL
 		attempts = 0
@@ -85,8 +115,6 @@ module.exports = LockManager =
 					callback(null, lockValue)
 				else
 					setTimeout attempt, testInterval
-					# back off when the lock is taken to avoid overloading
-					testInterval = Math.min(testInterval * 2, LockManager.MAX_TEST_INTERVAL)
 
 	_releaseLock: (key, lockValue, callback)->
 		rclient.eval LockManager.unlockScript, 1, key, lockValue, (err, result) ->

services/web/test/unit/coffee/infrastructure/LockManager/getLockTests.coffee

@@ -73,3 +73,38 @@ describe 'LockManager - getting the lock', ->
 
 		it "should return the callback with an error", ->
 			@callback.calledWith(new Error("timeout")).should.equal true
+
+	describe "when there are multiple requests for the same lock", ->
+		beforeEach (done) ->
+			locked = false
+			@results = []
+			@LockManager.LOCK_TEST_INTERVAL = 1
+			@LockManager._tryLock = (key, namespace, callback = (error, gotLock, lockValue) ->) ->
+				if locked
+					callback null, false
+				else
+					locked = true # simulate getting the lock
+					callback null, true
+			# Start ten lock requests in order at 1ms 2ms 3ms...
+			# with them randomly holding the lock for 0-100ms.
+			# Use predefined values for the random delay to make the test
+			# deterministic.
+			randomDelays = [52, 45, 41, 84, 60, 81, 31, 46, 9, 43 ]
+			startTime = 0
+			for randomDelay, i in randomDelays
+				do (randomDelay, i) =>
+					startTime += 1
+					setTimeout () =>
+						# changing the next line to the old method of LockManager._getLockByPolling
+						# should give results in a random order and cause the test to fail.
+						@LockManager._getLock @key, @namespace, (args...) =>
+							setTimeout () ->
+								locked = false  # release the lock after a random amount of time
+							, randomDelay
+							@results.push i
+							if @results.length is 10
+								done()
+					, startTime
+
+		it "should process the requests in order", ->
+			@results.should.deep.equal [0,1,2,3,4,5,6,7,8,9]