From 693c8e8c60d3e92830b7a9be523bd77f156555d9 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 15 Feb 2018 13:30:25 +0000 Subject: [PATCH] move where lock keys are defined --- .../ProjectEntityMongoUpdateHandler.coffee | 10 ++++++++- .../Project/ProjectEntityUpdateHandler.coffee | 6 +++++- .../Features/Project/ProjectGetter.coffee | 4 +++- .../coffee/infrastructure/LockManager.coffee | 21 +------------------ ...rojectEntityMongoUpdateHandlerTests.coffee | 5 ++--- .../ProjectEntityUpdateHandlerTests.coffee | 5 ++--- .../coffee/Project/ProjectGetterTests.coffee | 3 ++- 7 files changed, 24 insertions(+), 30 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee index 3a3a92d266..d1e75de5de 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee @@ -14,8 +14,13 @@ ProjectLocator = require('./ProjectLocator') SafePath = require './SafePath' wrapWithLock = (methodWithoutLock) -> + # This lock is used whenever we read or write to an existing project's + # structure. Some operations to project structure cannot be done atomically + # in mongo, this lock is used to prevent reading the structure between two + # parts of a staged update. methodWithLock = (project_id, args..., callback) -> - LockManager.mongoTransactionLock.runWithLock project_id, + lockKey = ProjectEntityMongoUpdateHandler.getProjectMongoLockKey project_id + LockManager.runWithLock lockKey, (cb) -> methodWithoutLock project_id, args..., cb callback methodWithLock.withoutLock = methodWithoutLock @@ -161,6 +166,9 @@ module.exports = ProjectEntityMongoUpdateHandler = self = return callback(err) callback null, folder, parentFolder_id + getProjectMongoLockKey: (project_id) -> + "lock:web:mongoTransaction:{#{project_id}}" + _removeElementFromMongoArray: (model, model_id, path, callback = (err, project) ->)-> conditions = {_id:model_id} update = {"$unset":{}} diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index 3804a2e5f2..3a64cde840 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -19,8 +19,12 @@ SafePath = require './SafePath' TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender') wrapWithLock = (methodWithoutLock) -> + # This lock is used to make sure that the project structure updates are made + # sequentially. In particular the updates must be made in mongo and sent to + # the doc-updater in the same order. methodWithLock = (project_id, args..., callback) -> - LockManager.sequentialProjectStructureUpdateLock.runWithLock project_id, + lockKey = "lock:web:sequentialProjectStructureUpdateLock:{#{project_id}}" + LockManager.runWithLock lockKey, (cb) -> methodWithoutLock project_id, args..., cb callback methodWithLock.withoutLock = methodWithoutLock diff --git a/services/web/app/coffee/Features/Project/ProjectGetter.coffee b/services/web/app/coffee/Features/Project/ProjectGetter.coffee index 18ede4a3a0..edbd7ab844 100644 --- a/services/web/app/coffee/Features/Project/ProjectGetter.coffee +++ b/services/web/app/coffee/Features/Project/ProjectGetter.coffee @@ -28,7 +28,9 @@ module.exports = ProjectGetter = return callback("no project_id provided") if projection?.rootFolder - LockManager.mongoTransactionLock.runWithLock project_id, + ProjectEntityMongoUpdateHandler = require './ProjectEntityMongoUpdateHandler' + lockKey = ProjectEntityMongoUpdateHandler.getProjectMongoLockKey project_id + LockManager.runWithLock lockKey, (cb) -> ProjectGetter.getProjectWithoutLock project_id, projection, cb callback else diff --git a/services/web/app/coffee/infrastructure/LockManager.coffee b/services/web/app/coffee/infrastructure/LockManager.coffee index 5e1b42dcb0..5eb7586054 100644 --- a/services/web/app/coffee/infrastructure/LockManager.coffee +++ b/services/web/app/coffee/infrastructure/LockManager.coffee @@ -9,26 +9,7 @@ module.exports = LockManager = MAX_LOCK_WAIT_TIME: 10000 # 10s maximum time to spend trying to get the lock REDIS_LOCK_EXPIRY: 30 # seconds. Time until lock auto expires in redis. - # This lock is used whenever we read or write to an existing project's - # structure. Some operations to project structure cannot be done atomically - # in mongo, this lock is used to prevent reading the structure between two - # parts of a staged update. - # - # This lock should only be called by ProjectEntityHandler and ProjectGetter - mongoTransactionLock: - runWithLock: (project_id, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> - LockManager._runWithLock "lock:web:mongoTransaction:{#{project_id}}", runner, callback - - # This lock is used to make sure that the project structure updates are made - # sequentially. In particular the updates must be made in mongo and sent to - # the doc-updater in the same order. - # - # This lock is generally called at a high level. - sequentialProjectStructureUpdateLock: - runWithLock: (project_id, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> - LockManager._runWithLock "lock:web:sequentialProjectStructureUpdateLock:{#{project_id}}", runner, callback - - _runWithLock: (key, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> + runWithLock: (key, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> LockManager._getLock key, (error) -> return callback(error) if error? runner (error1, values...) -> diff --git a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee index 4ec73e5209..9bcfbb0420 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee @@ -36,9 +36,8 @@ describe 'ProjectEntityMongoUpdateHandler', -> "../Cooldown/CooldownManager": @CooldownManager = {} '../../models/Folder': Folder:@FolderModel "../../infrastructure/LockManager":@LockManager = - mongoTransactionLock: - runWithLock: - sinon.spy((key, runner, callback) -> runner(callback)) + runWithLock: + sinon.spy((key, runner, callback) -> runner(callback)) '../../models/Project': Project:@ProjectModel = {} './ProjectEntityHandler': @ProjectEntityHandler = {} './ProjectLocator': @ProjectLocator = {} diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index 35de50851d..c6d732f6e6 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -52,9 +52,8 @@ describe 'ProjectEntityUpdateHandler', -> '../../models/File': File:@FileModel '../FileStore/FileStoreHandler':@FileStoreHandler "../../infrastructure/LockManager":@LockManager = - sequentialProjectStructureUpdateLock: - runWithLock: - sinon.spy((key, runner, callback) -> runner(callback)) + runWithLock: + sinon.spy((key, runner, callback) -> runner(callback)) '../../models/Project': Project:@ProjectModel = {} "./ProjectGetter": @ProjectGetter = {} './ProjectLocator': @ProjectLocator = {} diff --git a/services/web/test/unit/coffee/Project/ProjectGetterTests.coffee b/services/web/test/unit/coffee/Project/ProjectGetterTests.coffee index 529d0586bd..3dc0ecb92e 100644 --- a/services/web/test/unit/coffee/Project/ProjectGetterTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectGetterTests.coffee @@ -20,8 +20,9 @@ describe "ProjectGetter", -> "../../models/Project": Project: @Project = {} "../Collaborators/CollaboratorsHandler": @CollaboratorsHandler = {} "../../infrastructure/LockManager": @LockManager = - mongoTransactionLock: runWithLock : sinon.spy((key, runner, callback) -> runner(callback)) + './ProjectEntityMongoUpdateHandler': + lockKey: (project_id) -> project_id "logger-sharelatex": err:-> log:->