From c5449ae282efccb32220b03161647617d7370c08 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 13 Apr 2017 17:00:42 +0100 Subject: [PATCH] Split out redis config for real-time and track-changes into separate cluster-compatible configs --- .../app/coffee/HistoryManager.coffee | 4 +- .../app/coffee/HistoryRedisManager.coffee | 20 ++++++ .../app/coffee/WebRedisManager.coffee | 24 ++----- .../config/settings.defaults.coffee | 11 +++ .../HistoryManager/HistoryManagerTests.coffee | 18 ++--- .../HistoryRedisManagerTests.coffee | 69 +++++++++++++++++++ .../WebRedisManagerTests.coffee | 50 ++------------ 7 files changed, 121 insertions(+), 75 deletions(-) create mode 100644 services/document-updater/app/coffee/HistoryRedisManager.coffee create mode 100644 services/document-updater/test/unit/coffee/HistoryRedisManager/HistoryRedisManagerTests.coffee diff --git a/services/document-updater/app/coffee/HistoryManager.coffee b/services/document-updater/app/coffee/HistoryManager.coffee index 637fd2cb5f..512fd5e68b 100644 --- a/services/document-updater/app/coffee/HistoryManager.coffee +++ b/services/document-updater/app/coffee/HistoryManager.coffee @@ -2,7 +2,7 @@ settings = require "settings-sharelatex" request = require "request" logger = require "logger-sharelatex" async = require "async" -WebRedisManager = require "./WebRedisManager" +HistoryRedisManager = require "./HistoryRedisManager" module.exports = HistoryManager = flushDocChanges: (project_id, doc_id, callback = (error) ->) -> @@ -25,7 +25,7 @@ module.exports = HistoryManager = pushUncompressedHistoryOps: (project_id, doc_id, ops = [], callback = (error) ->) -> if ops.length == 0 return callback() - WebRedisManager.pushUncompressedHistoryOps project_id, doc_id, ops, (error, length) -> + HistoryRedisManager.pushUncompressedHistoryOps project_id, doc_id, ops, (error, length) -> return callback(error) if error? # We want to flush every 50 ops, i.e. 50, 100, 150, etc # Find out which 'block' (i.e. 0-49, 50-99) we were in before and after pushing these diff --git a/services/document-updater/app/coffee/HistoryRedisManager.coffee b/services/document-updater/app/coffee/HistoryRedisManager.coffee new file mode 100644 index 0000000000..315d9daabf --- /dev/null +++ b/services/document-updater/app/coffee/HistoryRedisManager.coffee @@ -0,0 +1,20 @@ +Settings = require('settings-sharelatex') +rclient = require("redis-sharelatex").createClient(Settings.redis.history) +Keys = Settings.redis.history.key_schema +async = require "async" +logger = require('logger-sharelatex') + +module.exports = HistoryRedisManager = + pushUncompressedHistoryOps: (project_id, doc_id, ops = [], callback = (error, length) ->) -> + if ops.length == 0 + return callback(new Error("cannot push no ops")) # This should never be called with no ops, but protect against a redis error if we sent an empty array to rpush + opVersions = ops.map (op) -> op?.v + logger.log project_id: project_id, doc_id: doc_id, op_versions: opVersions, "pushing uncompressed history ops" + jsonOps = ops.map (op) -> JSON.stringify op + async.parallel [ + (cb) -> rclient.rpush Keys.uncompressedHistoryOps({doc_id}), jsonOps..., cb + (cb) -> rclient.sadd Keys.docsWithHistoryOps({project_id}), doc_id, cb + ], (error, results) -> + return callback(error) if error? + [length, _] = results + callback(error, length) \ No newline at end of file diff --git a/services/document-updater/app/coffee/WebRedisManager.coffee b/services/document-updater/app/coffee/WebRedisManager.coffee index f500c62daf..6b7326d73a 100644 --- a/services/document-updater/app/coffee/WebRedisManager.coffee +++ b/services/document-updater/app/coffee/WebRedisManager.coffee @@ -1,13 +1,13 @@ Settings = require('settings-sharelatex') rclient = require("redis-sharelatex").createClient(Settings.redis.web) -async = require "async" +Keys = Settings.redis.web.key_schema logger = require('logger-sharelatex') module.exports = WebRedisManager = getPendingUpdatesForDoc : (doc_id, callback)-> multi = rclient.multi() - multi.lrange "PendingUpdates:#{doc_id}", 0 , -1 - multi.del "PendingUpdates:#{doc_id}" + multi.lrange Keys.pendingUpdates({doc_id}), 0 , -1 + multi.del Keys.pendingUpdates({doc_id}) multi.exec (error, replys) -> return callback(error) if error? jsonUpdates = replys[0] @@ -21,21 +21,7 @@ module.exports = WebRedisManager = callback error, updates getUpdatesLength: (doc_id, callback)-> - rclient.llen "PendingUpdates:#{doc_id}", callback + rclient.llen Keys.pendingUpdates({doc_id}), callback - pushUncompressedHistoryOps: (project_id, doc_id, ops = [], callback = (error, length) ->) -> - if ops.length == 0 - return callback(new Error("cannot push no ops")) # This should never be called with no ops, but protect against a redis error if we sent an empty array to rpush - opVersions = ops.map (op) -> op?.v - logger.log project_id: project_id, doc_id: doc_id, op_versions: opVersions, "pushing uncompressed history ops" - jsonOps = ops.map (op) -> JSON.stringify op - async.parallel [ - (cb) -> rclient.rpush "UncompressedHistoryOps:#{doc_id}", jsonOps..., cb - (cb) -> rclient.sadd "DocsWithHistoryOps:#{project_id}", doc_id, cb - ], (error, results) -> - return callback(error) if error? - [length, _] = results - callback(error, length) - sendData: (data) -> - rclient.publish "applied-ops", JSON.stringify(data) \ No newline at end of file + rclient.publish "applied-ops", JSON.stringify(data) diff --git a/services/document-updater/config/settings.defaults.coffee b/services/document-updater/config/settings.defaults.coffee index d638329622..973cfca970 100755 --- a/services/document-updater/config/settings.defaults.coffee +++ b/services/document-updater/config/settings.defaults.coffee @@ -20,6 +20,8 @@ module.exports = port:"6379" host:"localhost" password:"" + key_schema: + pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}" documentupdater: port: "6379" host: "localhost" @@ -33,6 +35,15 @@ module.exports = projectKey: ({doc_id}) -> "ProjectId:#{doc_id}" docsInProject: ({project_id}) -> "DocsIn:#{project_id}" ranges: ({doc_id}) -> "Ranges:#{doc_id}" + history: + port:"6379" + host:"localhost" + password:"" + key_schema: + uncompressedHistoryOps: ({doc_id}) -> "UncompressedHistoryOps:#{doc_id}" + docsWithHistoryOps: ({project_id}) -> "DocsWithHistoryOps:#{project_id}" + + # cluster: [{ # port: "7000" # host: "localhost" diff --git a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee index c33a18d4e6..66ffd98e80 100644 --- a/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/HistoryManager/HistoryManagerTests.coffee @@ -9,7 +9,7 @@ describe "HistoryManager", -> "request": @request = {} "settings-sharelatex": @Settings = {} "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } - "./WebRedisManager": @WebRedisManager = {} + "./HistoryRedisManager": @HistoryRedisManager = {} @project_id = "mock-project-id" @doc_id = "mock-doc-id" @callback = sinon.stub() @@ -47,11 +47,11 @@ describe "HistoryManager", -> describe "pushing the op", -> beforeEach -> - @WebRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 1) + @HistoryRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 1) @HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback it "should push the ops into redis", -> - @WebRedisManager.pushUncompressedHistoryOps + @HistoryRedisManager.pushUncompressedHistoryOps .calledWith(@project_id, @doc_id, @ops) .should.equal true @@ -63,7 +63,7 @@ describe "HistoryManager", -> describe "when we hit a multiple of FLUSH_EVERY_N_OPS ops", -> beforeEach -> - @WebRedisManager.pushUncompressedHistoryOps = + @HistoryRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 2 * @HistoryManager.FLUSH_EVERY_N_OPS) @HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback @@ -75,7 +75,7 @@ describe "HistoryManager", -> describe "when we go over a multiple of FLUSH_EVERY_N_OPS ops", -> beforeEach -> @ops = ["op1", "op2", "op3"] - @WebRedisManager.pushUncompressedHistoryOps = + @HistoryRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 2 * @HistoryManager.FLUSH_EVERY_N_OPS + 1) @HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback @@ -86,7 +86,7 @@ describe "HistoryManager", -> describe "when HistoryManager errors", -> beforeEach -> - @WebRedisManager.pushUncompressedHistoryOps = + @HistoryRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 2 * @HistoryManager.FLUSH_EVERY_N_OPS) @HistoryManager.flushDocChanges = sinon.stub().callsArgWith(2, @error = new Error("oops")) @HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, @callback @@ -103,10 +103,10 @@ describe "HistoryManager", -> describe "with no ops", -> beforeEach -> - @WebRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 1) + @HistoryRedisManager.pushUncompressedHistoryOps = sinon.stub().callsArgWith(3, null, 1) @HistoryManager.pushUncompressedHistoryOps @project_id, @doc_id, [], @callback - it "should not call WebRedisManager.pushUncompressedHistoryOps", -> - @WebRedisManager.pushUncompressedHistoryOps.called.should.equal false + it "should not call HistoryRedisManager.pushUncompressedHistoryOps", -> + @HistoryRedisManager.pushUncompressedHistoryOps.called.should.equal false diff --git a/services/document-updater/test/unit/coffee/HistoryRedisManager/HistoryRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/HistoryRedisManager/HistoryRedisManagerTests.coffee new file mode 100644 index 0000000000..f51942c1e1 --- /dev/null +++ b/services/document-updater/test/unit/coffee/HistoryRedisManager/HistoryRedisManagerTests.coffee @@ -0,0 +1,69 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +modulePath = "../../../../app/js/HistoryRedisManager.js" +SandboxedModule = require('sandboxed-module') +Errors = require "../../../../app/js/Errors" + +describe "HistoryRedisManager", -> + beforeEach -> + @rclient = + auth: () -> + exec: sinon.stub() + @rclient.multi = () => @rclient + @HistoryRedisManager = SandboxedModule.require modulePath, requires: + "redis-sharelatex": createClient: () => @rclient + "settings-sharelatex": + redis: + history: @settings = + key_schema: + uncompressedHistoryOps: ({doc_id}) -> "UncompressedHistoryOps:#{doc_id}" + docsWithHistoryOps: ({project_id}) -> "DocsWithHistoryOps:#{project_id}" + "logger-sharelatex": { log: () -> } + @doc_id = "doc-id-123" + @project_id = "project-id-123" + @callback = sinon.stub() + + describe "pushUncompressedHistoryOps", -> + beforeEach -> + @ops = [{ op: [{ i: "foo", p: 4 }] },{ op: [{ i: "bar", p: 56 }] }] + @rclient.rpush = sinon.stub().yields(null, @length = 42) + @rclient.sadd = sinon.stub().yields() + + describe "with ops", -> + beforeEach (done) -> + @HistoryRedisManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, (args...) => + @callback(args...) + done() + + it "should push the doc op into the doc ops list as JSON", -> + @rclient.rpush + .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) + .should.equal true + + it "should add the doc_id to the set of which records the project docs", -> + @rclient.sadd + .calledWith("DocsWithHistoryOps:#{@project_id}", @doc_id) + .should.equal true + + it "should call the callback with the length", -> + @callback.calledWith(null, @length).should.equal true + + describe "with no ops", -> + beforeEach (done) -> + @HistoryRedisManager.pushUncompressedHistoryOps @project_id, @doc_id, [], (args...) => + @callback(args...) + done() + + it "should not push the doc op into the doc ops list as JSON", -> + @rclient.rpush + .called + .should.equal false + + it "should not add the doc_id to the set of which records the project docs", -> + @rclient.sadd + .called + .should.equal false + + it "should call the callback with an error", -> + @callback.calledWith(new Error("cannot push no ops")).should.equal true diff --git a/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee b/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee index a0f88b33f1..e1fb89eaed 100644 --- a/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/WebRedisManager/WebRedisManagerTests.coffee @@ -13,7 +13,11 @@ describe "WebRedisManager", -> @rclient.multi = () => @rclient @WebRedisManager = SandboxedModule.require modulePath, requires: "redis-sharelatex": createClient: () => @rclient - "settings-sharelatex": redis: web: @settings = {"mock": "settings"} + "settings-sharelatex": + redis: + web: @settings = + key_schema: + pendingUpdates: ({doc_id}) -> "PendingUpdates:#{doc_id}" "logger-sharelatex": { log: () -> } @doc_id = "doc-id-123" @project_id = "project-id-123" @@ -70,47 +74,3 @@ describe "WebRedisManager", -> it "should return the length", -> @callback.calledWith(null, @length).should.equal true - - describe "pushUncompressedHistoryOps", -> - beforeEach -> - @ops = [{ op: [{ i: "foo", p: 4 }] },{ op: [{ i: "bar", p: 56 }] }] - @rclient.rpush = sinon.stub().yields(null, @length = 42) - @rclient.sadd = sinon.stub().yields() - - describe "with ops", -> - beforeEach (done) -> - @WebRedisManager.pushUncompressedHistoryOps @project_id, @doc_id, @ops, (args...) => - @callback(args...) - done() - - it "should push the doc op into the doc ops list as JSON", -> - @rclient.rpush - .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@ops[0]), JSON.stringify(@ops[1])) - .should.equal true - - it "should add the doc_id to the set of which records the project docs", -> - @rclient.sadd - .calledWith("DocsWithHistoryOps:#{@project_id}", @doc_id) - .should.equal true - - it "should call the callback with the length", -> - @callback.calledWith(null, @length).should.equal true - - describe "with no ops", -> - beforeEach (done) -> - @WebRedisManager.pushUncompressedHistoryOps @project_id, @doc_id, [], (args...) => - @callback(args...) - done() - - it "should not push the doc op into the doc ops list as JSON", -> - @rclient.rpush - .called - .should.equal false - - it "should not add the doc_id to the set of which records the project docs", -> - @rclient.sadd - .called - .should.equal false - - it "should call the callback with an error", -> - @callback.calledWith(new Error("cannot push no ops")).should.equal true