From b15d2ef796e89b4ccac20bd59cfb96b6af478b72 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 15 May 2017 16:18:40 +0100 Subject: [PATCH] Revert "fail safely if doc cannot be loaded" --- .../app/coffee/DocumentManager.coffee | 7 --- .../app/coffee/RedisManager.coffee | 6 --- .../app/coffee/UpdateManager.coffee | 10 ++--- .../DocumentManagerTests.coffee | 45 +------------------ .../UpdateManager/UpdateManagerTests.coffee | 5 +-- 5 files changed, 6 insertions(+), 67 deletions(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 489b79bd56..054baca47e 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -28,13 +28,6 @@ module.exports = DocumentManager = else callback null, lines, version, ranges, true - ensureDocIsLoaded: (project_id, doc_id, callback = (error) ->) -> - RedisManager.checkDocInMemory project_id, doc_id, (error) -> - if error? - DocumentManager.getDoc project_id, doc_id, callback - else - callback() - getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps, ranges) ->) -> timer = new Metrics.Timer("docManager.getDocAndRecentOps") callback = (args...) -> diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 857e19c88c..4aad7ec109 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -30,12 +30,6 @@ historyKeys = Settings.redis.history.key_schema module.exports = RedisManager = rclient: rclient - checkDocInMemory: (project_id, doc_id, callback) -> - rclient.exists keys.docLines(doc_id:doc_id), (error, result) -> - return callback(error) if error? - return callback new Error("doc not in memory") if result isnt 1 - callback() - putDocInMemory : (project_id, doc_id, docLines, version, ranges, _callback)-> timer = new metrics.Timer("redis.put-doc") callback = (error) -> diff --git a/services/document-updater/app/coffee/UpdateManager.coffee b/services/document-updater/app/coffee/UpdateManager.coffee index 257bc71601..269b16ee67 100644 --- a/services/document-updater/app/coffee/UpdateManager.coffee +++ b/services/document-updater/app/coffee/UpdateManager.coffee @@ -14,14 +14,10 @@ RangesManager = require "./RangesManager" module.exports = UpdateManager = processOutstandingUpdates: (project_id, doc_id, callback = (error) ->) -> timer = new Metrics.Timer("updateManager.processOutstandingUpdates") - DocumentManager.ensureDocIsLoaded project_id, doc_id, (error) -> - # an error at this point is safe, because we haven't consumed any ops yet + UpdateManager.fetchAndApplyUpdates project_id, doc_id, (error) -> + timer.done() return callback(error) if error? - UpdateManager.fetchAndApplyUpdates project_id, doc_id, (error) -> - timer.done() - # now we have taken ops off the queue so errors here will cause data loss - return callback(error) if error? - callback() + callback() processOutstandingUpdatesWithLock: (project_id, doc_id, callback = (error) ->) -> LockManager.tryLock doc_id, (error, gotLock, lockValue) => diff --git a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee index d5607220f0..b2eaa321d6 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/DocumentManagerTests.coffee @@ -188,48 +188,7 @@ describe "DocumentManager", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - - - describe "ensureDocIsLoaded", -> - describe "when the doc exists in Redis", -> - beforeEach -> - @RedisManager.checkDocInMemory = sinon.stub().callsArgWith(2) - @DocumentManager.ensureDocIsLoaded @project_id, @doc_id, @callback - - it "should check the doc in Redis", -> - @RedisManager.checkDocInMemory - .calledWith(@project_id, @doc_id) - .should.equal true - - it "should call the callback", -> - @callback.called.should.equal true - - describe "when the doc does not exist in Redis", -> - beforeEach -> - @RedisManager.getDoc = sinon.stub().callsArgWith(2, null, null, null, null, null) - @PersistenceManager.getDoc = sinon.stub().callsArgWith(2, null, @lines, @version, @ranges) - @RedisManager.putDocInMemory = sinon.stub().yields() - @RedisManager.checkDocInMemory = sinon.stub().callsArgWith(2, new Error("doc is not loaded")) - @DocumentManager.ensureDocIsLoaded @project_id, @doc_id, @callback - - it "should try to get the doc from Redis", -> - @RedisManager.getDoc - .calledWith(@project_id, @doc_id) - .should.equal true - - it "should get the doc from the PersistenceManager", -> - @PersistenceManager.getDoc - .calledWith(@project_id, @doc_id) - .should.equal true - - it "should set the doc in Redis", -> - @RedisManager.putDocInMemory - .calledWith(@project_id, @doc_id, @lines, @version, @ranges) - .should.equal true - - it "should call the callback", -> - @callback.calledWith(null).should.equal true - + describe "setDoc", -> describe "with plain tex lines", -> beforeEach -> @@ -414,4 +373,4 @@ describe "DocumentManager", -> it "should call the callback with a not found error", -> error = new Errors.NotFoundError("document not found: #{@doc_id}") - @callback.calledWith(error).should.equal true + @callback.calledWith(error).should.equal true \ No newline at end of file diff --git a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee index f458f5d0b4..2de6e93e44 100644 --- a/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/UpdateManager/UpdateManagerTests.coffee @@ -25,7 +25,6 @@ describe "UpdateManager", -> describe "processOutstandingUpdates", -> beforeEach -> - @DocumentManager.ensureDocIsLoaded = sinon.stub().callsArg(2) @UpdateManager.fetchAndApplyUpdates = sinon.stub().callsArg(2) @UpdateManager.processOutstandingUpdates @project_id, @doc_id, @callback @@ -43,7 +42,6 @@ describe "UpdateManager", -> beforeEach -> @LockManager.tryLock = sinon.stub().callsArgWith(1, null, true, @lockValue = "mock-lock-value") @LockManager.releaseLock = sinon.stub().callsArg(2) - @RedisManager.checkDocInMemory = sinon.stub().callsArg(2) @UpdateManager.continueProcessingUpdatesWithLock = sinon.stub().callsArg(2) @UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2) @@ -92,8 +90,7 @@ describe "UpdateManager", -> it "should not process the updates", -> @UpdateManager.processOutstandingUpdates.called.should.equal false - - + describe "continueProcessingUpdatesWithLock", -> describe "when there are outstanding updates", -> beforeEach ->