diff --git a/services/document-updater/app/coffee/RangesManager.coffee b/services/document-updater/app/coffee/RangesManager.coffee index f39dea9537..c5aaf97473 100644 --- a/services/document-updater/app/coffee/RangesManager.coffee +++ b/services/document-updater/app/coffee/RangesManager.coffee @@ -3,7 +3,7 @@ logger = require "logger-sharelatex" module.exports = RangesManager = MAX_COMMENTS: 500 - MAX_CHANGES: 1200 + MAX_CHANGES: 2000 applyUpdate: (project_id, doc_id, entries = {}, updates = [], callback = (error, new_entries) ->) -> {changes, comments} = entries diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 1ce0062fe6..cf8249dbd7 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -22,6 +22,9 @@ logHashErrors = Settings.documentupdater?.logHashErrors logHashReadErrors = logHashErrors?.read logHashWriteErrors = logHashErrors?.write +MEGABYTES = 1024 * 1024 +MAX_RANGES_SIZE = 3 * MEGABYTES + module.exports = RedisManager = rclient: rclient @@ -37,24 +40,27 @@ module.exports = RedisManager = return callback(error) docHash = RedisManager._computeHash(docLines) logger.log project_id:project_id, doc_id:doc_id, version: version, hash:docHash, "putting doc in redis" - ranges = RedisManager._serializeRanges(ranges) - multi = rclient.multi() - multi.eval setScript, 1, keys.docLines(doc_id:doc_id), docLines - multi.set keys.projectKey({doc_id:doc_id}), project_id - multi.set keys.docVersion(doc_id:doc_id), version - multi.set keys.docHash(doc_id:doc_id), docHash - if ranges? - multi.set keys.ranges(doc_id:doc_id), ranges - else - multi.del keys.ranges(doc_id:doc_id) - multi.exec (error, result) -> - return callback(error) if error? - # check the hash computed on the redis server - writeHash = result?[0] - if logHashWriteErrors and writeHash? and writeHash isnt docHash - logger.error project_id: project_id, doc_id: doc_id, writeHash: writeHash, origHash: docHash, docLines:docLines, "hash mismatch on putDocInMemory" - # update docsInProject set - rclient.sadd keys.docsInProject(project_id:project_id), doc_id, callback + RedisManager._serializeRanges ranges, (error, ranges) -> + if error? + logger.error {err: error, doc_id, project_id}, error.message + return callback(error) + multi = rclient.multi() + multi.eval setScript, 1, keys.docLines(doc_id:doc_id), docLines + multi.set keys.projectKey({doc_id:doc_id}), project_id + multi.set keys.docVersion(doc_id:doc_id), version + multi.set keys.docHash(doc_id:doc_id), docHash + if ranges? + multi.set keys.ranges(doc_id:doc_id), ranges + else + multi.del keys.ranges(doc_id:doc_id) + multi.exec (error, result) -> + return callback(error) if error? + # check the hash computed on the redis server + writeHash = result?[0] + if logHashWriteErrors and writeHash? and writeHash isnt docHash + logger.error project_id: project_id, doc_id: doc_id, writeHash: writeHash, origHash: docHash, docLines:docLines, "hash mismatch on putDocInMemory" + # update docsInProject set + rclient.sadd keys.docsInProject(project_id:project_id), doc_id, callback removeDocFromMemory : (project_id, doc_id, _callback)-> logger.log project_id:project_id, doc_id:doc_id, "removing doc from redis" @@ -163,36 +169,41 @@ module.exports = RedisManager = logger.log doc_id: doc_id, version: newVersion, hash: newHash, "updating doc in redis" - multi = rclient.multi() - multi.eval setScript, 1, keys.docLines(doc_id:doc_id), newDocLines - multi.set keys.docVersion(doc_id:doc_id), newVersion - multi.set keys.docHash(doc_id:doc_id), newHash - if jsonOps.length > 0 - multi.rpush keys.docOps(doc_id: doc_id), jsonOps... - multi.expire keys.docOps(doc_id: doc_id), RedisManager.DOC_OPS_TTL - multi.ltrim keys.docOps(doc_id: doc_id), -RedisManager.DOC_OPS_MAX_LENGTH, -1 - ranges = RedisManager._serializeRanges(ranges) - if ranges? - multi.set keys.ranges(doc_id:doc_id), ranges - else - multi.del keys.ranges(doc_id:doc_id) - multi.exec (error, result) -> - return callback(error) if error? - # check the hash computed on the redis server - writeHash = result?[0] - if logHashWriteErrors and writeHash? and writeHash isnt newHash - logger.error doc_id: doc_id, writeHash: writeHash, origHash: newHash, docLines:newDocLines, "hash mismatch on updateDocument" - return callback() + RedisManager._serializeRanges ranges, (error, ranges) -> + if error? + logger.error {err: error, doc_id}, error.message + return callback(error) + multi = rclient.multi() + multi.eval setScript, 1, keys.docLines(doc_id:doc_id), newDocLines + multi.set keys.docVersion(doc_id:doc_id), newVersion + multi.set keys.docHash(doc_id:doc_id), newHash + if jsonOps.length > 0 + multi.rpush keys.docOps(doc_id: doc_id), jsonOps... + multi.expire keys.docOps(doc_id: doc_id), RedisManager.DOC_OPS_TTL + multi.ltrim keys.docOps(doc_id: doc_id), -RedisManager.DOC_OPS_MAX_LENGTH, -1 + if ranges? + multi.set keys.ranges(doc_id:doc_id), ranges + else + multi.del keys.ranges(doc_id:doc_id) + multi.exec (error, result) -> + return callback(error) if error? + # check the hash computed on the redis server + writeHash = result?[0] + if logHashWriteErrors and writeHash? and writeHash isnt newHash + logger.error doc_id: doc_id, writeHash: writeHash, origHash: newHash, docLines:newDocLines, "hash mismatch on updateDocument" + return callback() getDocIdsInProject: (project_id, callback = (error, doc_ids) ->) -> rclient.smembers keys.docsInProject(project_id: project_id), callback - _serializeRanges: (ranges) -> + _serializeRanges: (ranges, callback = (error, serializedRanges) ->) -> jsonRanges = JSON.stringify(ranges) + if jsonRanges? and jsonRanges.length > MAX_RANGES_SIZE + return callback new Error("ranges are too large") if jsonRanges == '{}' # Most doc will have empty ranges so don't fill redis with lots of '{}' keys jsonRanges = null - return jsonRanges + return callback null, jsonRanges _deserializeRanges: (ranges) -> if !ranges? or ranges == "" diff --git a/services/document-updater/test/acceptance/coffee/RangesTests.coffee b/services/document-updater/test/acceptance/coffee/RangesTests.coffee index 044fd3191f..e3ec097e2e 100644 --- a/services/document-updater/test/acceptance/coffee/RangesTests.coffee +++ b/services/document-updater/test/acceptance/coffee/RangesTests.coffee @@ -264,4 +264,41 @@ describe "Ranges", -> DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) => throw error if error? expect(data.ranges.comments).to.be.undefined - done() \ No newline at end of file + done() + + describe "tripping range size limit", -> + before (done) -> + @project_id = DocUpdaterClient.randomId() + @user_id = DocUpdaterClient.randomId() + @id_seed = DocUpdaterClient.randomId() + @doc = { + id: DocUpdaterClient.randomId() + lines: ["aaa"] + } + @i = new Array(3 * 1024 * 1024).join("a") + @updates = [{ + doc: @doc.id + op: [{ i: @i, p: 1 }] + v: 0 + meta: { user_id: @user_id, tc: @id_seed } + }] + MockWebApi.insertDoc @project_id, @doc.id, { + lines: @doc.lines + version: 0 + } + jobs = [] + for update in @updates + do (update) => + jobs.push (callback) => DocUpdaterClient.sendUpdate @project_id, @doc.id, update, callback + DocUpdaterClient.preloadDoc @project_id, @doc.id, (error) => + throw error if error? + async.series jobs, (error) -> + throw error if error? + setTimeout done, 200 + + it "should not update the ranges", (done) -> + DocUpdaterClient.getDoc @project_id, @doc.id, (error, res, data) => + throw error if error? + ranges = data.ranges + expect(ranges.changes).to.be.undefined + done() \ No newline at end of file diff --git a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee index abc7307c15..258603be9b 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/RedisManagerTests.coffee @@ -337,6 +337,18 @@ describe "RedisManager", -> it "should call the callback with an error", -> @callback.calledWith(new Error("null bytes found in doc lines")).should.equal true + + describe "with ranges that are too big", -> + beforeEach -> + @RedisManager.getDocVersion.withArgs(@doc_id).yields(null, @version - @ops.length) + @RedisManager._serializeRanges = sinon.stub().yields(new Error("ranges are too large")) + @RedisManager.updateDocument @doc_id, @lines, @version, @ops, @ranges, @callback + + it 'should log an error', -> + @logger.error.called.should.equal true + + it "should call the callback with the error", -> + @callback.calledWith(new Error("ranges are too large")).should.equal true describe "putDocInMemory", -> beforeEach -> @@ -425,6 +437,17 @@ describe "RedisManager", -> it "should call the callback with an error", -> @callback.calledWith(new Error("null bytes found in doc lines")).should.equal true + + describe "with ranges that are too big", -> + beforeEach -> + @RedisManager._serializeRanges = sinon.stub().yields(new Error("ranges are too large")) + @RedisManager.putDocInMemory @project_id, @doc_id, @lines, @version, @ranges, @callback + + it 'should log an error', -> + @logger.error.called.should.equal true + + it "should call the callback with the error", -> + @callback.calledWith(new Error("ranges are too large")).should.equal true describe "removeDocFromMemory", -> beforeEach (done) ->