diff --git a/services/track-changes/app/coffee/HistoryManager.coffee b/services/track-changes/app/coffee/HistoryManager.coffee index 597d6e2460..e11e911698 100644 --- a/services/track-changes/app/coffee/HistoryManager.coffee +++ b/services/track-changes/app/coffee/HistoryManager.coffee @@ -1,6 +1,7 @@ MongoManager = require "./MongoManager" RedisManager = require "./RedisManager" UpdateCompressor = require "./UpdateCompressor" +LockManager = require "./LockManager" logger = require "logger-sharelatex" module.exports = HistoryManager = @@ -37,5 +38,10 @@ module.exports = HistoryManager = return callback(error) if error? callback() - processUncompressUpdatesWithLock: (doc_id, callback = (error) ->) -> + processUncompressedUpdatesWithLock: (doc_id, callback = (error) ->) -> + LockManager.runWithLock( + "HistoryLock:#{doc_id}", + HistoryManager.processUncompressedUpdates, + callback + ) diff --git a/services/track-changes/app/coffee/LockManager.coffee b/services/track-changes/app/coffee/LockManager.coffee new file mode 100644 index 0000000000..5f71a336fb --- /dev/null +++ b/services/track-changes/app/coffee/LockManager.coffee @@ -0,0 +1,54 @@ +Settings = require "settings-sharelatex" +redis = require('redis') +redisConf = Settings.redis?.web or {host: "localhost", port: 6379} +rclient = redis.createClient(redisConf.port, redisConf.host) +rclient.auth(redisConf.password) + +module.exports = LockManager = + LOCK_TEST_INTERVAL: 50 # 50ms between each test of the lock + MAX_LOCK_WAIT_TIME: 10000 # 10s maximum time to spend trying to get the lock + LOCK_TTL: 10 # seconds + + tryLock : (key, callback = (err, gotLock) ->) -> + rclient.set key, "locked", "EX", @LOCK_TTL, "NX", (err, gotLock)-> + return callback(err) if err? + if gotLock == "OK" + callback err, true + else + callback err, false + + getLock: (key, callback = (error) ->) -> + startTime = Date.now() + do attempt = () -> + if Date.now() - startTime > LockManager.MAX_LOCK_WAIT_TIME + return callback(new Error("Timeout")) + + LockManager.tryLock key, (error, gotLock) -> + return callback(error) if error? + if gotLock + callback(null) + else + setTimeout attempt, LockManager.LOCK_TEST_INTERVAL + + checkLock: (key, callback = (err, isFree) ->) -> + rclient.exists key, (err, exists) -> + return callback(err) if err? + exists = parseInt exists + if exists == 1 + callback err, false + else + callback err, true + + releaseLock: (key, callback) -> + rclient.del key, callback + + runWithLock: (key, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> + LockManager.getLock key, (error) -> + return callback(error) if error? + runner (error1) -> + LockManager.releaseLock key, (error2) -> + error = error1 or error2 + return callback(error) if error? + callback() + + diff --git a/services/track-changes/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee b/services/track-changes/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee index b2a234c928..0770b0518e 100644 --- a/services/track-changes/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee @@ -11,6 +11,7 @@ describe "HistoryManager", -> "./UpdateCompressor": @UpdateCompressor = {} "./MongoManager" : @MongoManager = {} "./RedisManager" : @RedisManager = {} + "./LockManager" : @LockManager = {} "logger-sharelatex": { log: sinon.stub() } @doc_id = "doc-id-123" @callback = sinon.stub() @@ -136,4 +137,19 @@ describe "HistoryManager", -> it "should call the callback", -> @callback.called.should.equal true + describe "processCompressedUpdatesWithLock", -> + beforeEach -> + @HistoryManager.processUncompressedUpdates = sinon.stub() + @LockManager.runWithLock = sinon.stub().callsArg(2) + @HistoryManager.processUncompressedUpdatesWithLock @doc_id, @callback + it "should run processUncompressedUpdates with the lock", -> + @LockManager.runWithLock + .calledWith( + "HistoryLock:#{@doc_id}", + @HistoryManager.processUncompressedUpdates + ) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true diff --git a/services/track-changes/test/unit/coffee/LockManager/LockManagerTests.coffee b/services/track-changes/test/unit/coffee/LockManager/LockManagerTests.coffee new file mode 100644 index 0000000000..e5caea82c1 --- /dev/null +++ b/services/track-changes/test/unit/coffee/LockManager/LockManagerTests.coffee @@ -0,0 +1,171 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +expect = chai.expect +modulePath = "../../../../app/js/LockManager.js" +SandboxedModule = require('sandboxed-module') + +describe "LockManager", -> + beforeEach -> + @LockManager = SandboxedModule.require modulePath, requires: + "redis": + createClient: () => @rclient = + auth: sinon.stub() + "settings-sharelatex": @Settings = {} + @key = "lock-key" + @callback = sinon.stub() + + describe "checkLock", -> + describe "when the lock is taken", -> + beforeEach -> + @rclient.exists = sinon.stub().callsArgWith(1, null, "1") + @LockManager.checkLock @key, @callback + + it "should check the lock in redis", -> + @rclient.exists + .calledWith(@key) + .should.equal true + + it "should return the callback with false", -> + @callback.calledWith(null, false).should.equal true + + describe "when the lock is free", -> + beforeEach -> + @rclient.exists = sinon.stub().callsArgWith(1, null, "0") + @LockManager.checkLock @key, @callback + + it "should return the callback with true", -> + @callback.calledWith(null, true).should.equal true + + + describe "tryLock", -> + describe "when the lock is taken", -> + beforeEach -> + @rclient.set = sinon.stub().callsArgWith(5, null, null) + @LockManager.tryLock @key, @callback + + it "should check the lock in redis", -> + @rclient.set + .calledWith(@key, "locked", "EX", @LockManager.LOCK_TTL, "NX") + .should.equal true + + it "should return the callback with false", -> + @callback.calledWith(null, false).should.equal true + + describe "when the lock is free", -> + beforeEach -> + @rclient.set = sinon.stub().callsArgWith(5, null, "OK") + @LockManager.tryLock @key, @callback + + it "should return the callback with true", -> + @callback.calledWith(null, true).should.equal true + + describe "deleteLock", -> + beforeEach -> + beforeEach -> + @rclient.del = sinon.stub().callsArg(1) + @LockManager.deleteLock @key, @callback + + it "should delete the lock in redis", -> + @rclient.del + .calledWith(key) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + describe "getLock", -> + describe "when the lock is not taken", -> + beforeEach (done) -> + @LockManager.tryLock = sinon.stub().callsArgWith(1, null, true) + @LockManager.getLock @key, (args...) => + @callback(args...) + done() + + it "should try to get the lock", -> + @LockManager.tryLock + .calledWith(@key) + .should.equal true + + it "should only need to try once", -> + @LockManager.tryLock.callCount.should.equal 1 + + it "should return the callback", -> + @callback.calledWith(null).should.equal true + + describe "when the lock is initially set", -> + beforeEach (done) -> + startTime = Date.now() + @LockManager.LOCK_TEST_INTERVAL = 5 + @LockManager.tryLock = (doc_id, callback = (error, isFree) ->) -> + if Date.now() - startTime < 20 + callback null, false + else + callback null, true + sinon.spy @LockManager, "tryLock" + + @LockManager.getLock @key, (args...) => + @callback(args...) + done() + + it "should call tryLock multiple times until free", -> + (@LockManager.tryLock.callCount > 1).should.equal true + + it "should return the callback", -> + @callback.calledWith(null).should.equal true + + describe "when the lock times out", -> + beforeEach (done) -> + time = Date.now() + @LockManager.MAX_LOCK_WAIT_TIME = 5 + @LockManager.tryLock = sinon.stub().callsArgWith(1, null, false) + @LockManager.getLock @key, (args...) => + @callback(args...) + done() + + it "should return the callback with an error", -> + @callback.calledWith(new Error("timeout")).should.equal true + + describe "runWithLock", -> + describe "with successful run", -> + beforeEach -> + @runner = (releaseLock = (error) ->) -> + releaseLock() + sinon.spy @, "runner" + @LockManager.getLock = sinon.stub().callsArg(1) + @LockManager.releaseLock = sinon.stub().callsArg(1) + @LockManager.runWithLock @key, @runner, @callback + + it "should get the lock", -> + @LockManager.getLock + .calledWith(@key) + .should.equal true + + it "should run the passed function", -> + @runner.called.should.equal true + + it "should release the lock", -> + @LockManager.releaseLock + .calledWith(@key) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + describe "when the runner function returns an error", -> + beforeEach -> + @error = new Error("oops") + @runner = (releaseLock = (error) ->) => + releaseLock(@error) + sinon.spy @, "runner" + @LockManager.getLock = sinon.stub().callsArg(1) + @LockManager.releaseLock = sinon.stub().callsArg(1) + @LockManager.runWithLock @key, @runner, @callback + + it "should release the lock", -> + @LockManager.releaseLock + .calledWith(@key) + .should.equal true + + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true