From 6f19f46d961d92737a7df011a751f8fbe463c6fe Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Feb 2014 16:37:45 +0000 Subject: [PATCH 01/16] Create method for pushing uncompressed ops into redis --- .../app/coffee/DocOpsManager.coffee | 1 + .../app/coffee/RedisKeyBuilder.coffee | 2 ++ .../app/coffee/RedisManager.coffee | 4 +++ .../pushUncompressedHistoryOpTests.coffee | 34 +++++++++++++++++++ 4 files changed, 41 insertions(+) create mode 100644 services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee diff --git a/services/document-updater/app/coffee/DocOpsManager.coffee b/services/document-updater/app/coffee/DocOpsManager.coffee index 0e90f5b462..032f9f6566 100644 --- a/services/document-updater/app/coffee/DocOpsManager.coffee +++ b/services/document-updater/app/coffee/DocOpsManager.coffee @@ -44,6 +44,7 @@ module.exports = DocOpsManager = callback null, ops pushDocOp: (project_id, doc_id, op, callback = (error) ->) -> + console.log "PUSHING OP", op RedisManager.pushDocOp doc_id, op, callback _ensureOpsAreLoaded: (project_id, doc_id, backToVersion, callback = (error) ->) -> diff --git a/services/document-updater/app/coffee/RedisKeyBuilder.coffee b/services/document-updater/app/coffee/RedisKeyBuilder.coffee index a444341ea1..2bd1ed08c8 100644 --- a/services/document-updater/app/coffee/RedisKeyBuilder.coffee +++ b/services/document-updater/app/coffee/RedisKeyBuilder.coffee @@ -8,12 +8,14 @@ DOCLINES = "doclines" DOCOPS = "DocOps" DOCVERSION = "DocVersion" DOCIDSWITHPENDINGUPDATES = "DocsWithPendingUpdates" +UNCOMPRESSED_HISTORY_OPS = "UncompressedHistoryOps" module.exports = allDocs : ALLDOCSKEY docLines : (op)-> DOCLINES+":"+op.doc_id docOps : (op)-> DOCOPS+":"+op.doc_id + uncompressedHistoryOp: (op) -> UNCOMPRESSED_HISTORY_OPS + ":" + op.doc_id docVersion : (op)-> DOCVERSION+":"+op.doc_id projectKey : (op)-> PROJECTKEY+":"+op.doc_id blockingKey : (op)-> BLOCKINGKEY+":"+op.doc_id diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index b2c4c9c9d0..5f6c880cee 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -155,6 +155,10 @@ module.exports = jsonOps = ops.map (op) -> JSON.stringify op rclient.lpush keys.docOps(doc_id: doc_id), jsonOps.reverse(), callback + pushUncompressedHistoryOp: (doc_id, op, callback = (error) ->) -> + jsonOp = JSON.stringify op + rclient.rpush keys.uncompressedHistoryOp(doc_id: doc_id), jsonOp, callback + getDocOpsLength: (doc_id, callback = (error, length) ->) -> rclient.llen keys.docOps(doc_id: doc_id), callback diff --git a/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee new file mode 100644 index 0000000000..3b743db6e4 --- /dev/null +++ b/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee @@ -0,0 +1,34 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +modulePath = "../../../../app/js/RedisManager.js" +SandboxedModule = require('sandboxed-module') + +describe "RedisManager.pushUncompressedHistoryOp", -> + beforeEach -> + @RedisManager = SandboxedModule.require modulePath, requires: + "redis": createClient: () => + @rclient = + auth: () -> + multi: () => @rclient + "logger-sharelatex": @logger = {log: sinon.stub()} + @doc_id = "doc-id-123" + @callback = sinon.stub() + @rclient.rpush = sinon.stub() + + describe "successfully", -> + beforeEach -> + @op = { op: [{ i: "foo", p: 4 }] } + @rclient.rpush = sinon.stub().callsArg(2) + @RedisManager.pushUncompressedHistoryOp @doc_id, @op, @callback + + it "should push the doc op into the doc ops list", -> + @rclient.rpush + .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@op)) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + + From b13f70eadb643a31c75a94c7e740d08ed3d0945f Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 24 Feb 2014 16:52:12 +0000 Subject: [PATCH 02/16] Push ops into uncompressedHistoryOps list --- services/document-updater/Gruntfile.coffee | 2 +- .../app/coffee/DocOpsManager.coffee | 5 +++-- .../app/coffee/ShareJsDB.coffee | 2 +- .../DocOpsManager/DocOpsManagerTests.coffee | 20 +++++++++++++++++++ 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/services/document-updater/Gruntfile.coffee b/services/document-updater/Gruntfile.coffee index 30dd63e708..717ebd464c 100644 --- a/services/document-updater/Gruntfile.coffee +++ b/services/document-updater/Gruntfile.coffee @@ -103,7 +103,7 @@ module.exports = (grunt) -> grunt.registerTask 'install', "Compile everything when installing as an npm module", ['compile'] - grunt.registerTask 'test:unit', 'Run the unit tests (use --grep= for individual tests)', ['compile:unit_tests', 'mochaTest:unit'] + grunt.registerTask 'test:unit', 'Run the unit tests (use --grep= for individual tests)', ['compile:server', 'compile:unit_tests', 'mochaTest:unit'] grunt.registerTask 'test:acceptance', 'Run the acceptance tests (use --grep= for individual tests)', ['compile:acceptance_tests', 'mochaTest:acceptance'] grunt.registerTask 'run', "Compile and run the document-updater-sharelatex server", ['compile', 'bunyan', 'execute'] diff --git a/services/document-updater/app/coffee/DocOpsManager.coffee b/services/document-updater/app/coffee/DocOpsManager.coffee index 032f9f6566..43fd7a4aac 100644 --- a/services/document-updater/app/coffee/DocOpsManager.coffee +++ b/services/document-updater/app/coffee/DocOpsManager.coffee @@ -44,8 +44,9 @@ module.exports = DocOpsManager = callback null, ops pushDocOp: (project_id, doc_id, op, callback = (error) ->) -> - console.log "PUSHING OP", op - RedisManager.pushDocOp doc_id, op, callback + RedisManager.pushDocOp doc_id, op, (error) -> + return callback(error) if error? + RedisManager.pushUncompressedHistoryOp doc_id, op, callback _ensureOpsAreLoaded: (project_id, doc_id, backToVersion, callback = (error) ->) -> RedisManager.getDocVersion doc_id, (error, redisVersion) -> diff --git a/services/document-updater/app/coffee/ShareJsDB.coffee b/services/document-updater/app/coffee/ShareJsDB.coffee index 3704121b6d..6eaf21846c 100644 --- a/services/document-updater/app/coffee/ShareJsDB.coffee +++ b/services/document-updater/app/coffee/ShareJsDB.coffee @@ -23,7 +23,7 @@ module.exports = ShareJsDB = writeOp: (doc_key, opData, callback) -> [project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key) - DocOpsManager.pushDocOp project_id, doc_id, {op:opData.op, meta:opData.meta}, (error, version) -> + DocOpsManager.pushDocOp project_id, doc_id, opData, (error, version) -> return callback error if error? if version == opData.v + 1 diff --git a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee index 83e0ff48cf..c26171d98f 100644 --- a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee @@ -306,4 +306,24 @@ describe "DocOpsManager", -> it "should return the ops", -> @callback.calledWith(null, @doc.docOps).should.equal true + describe "pushDocOp", -> + beforeEach -> + @op = "mock-op" + @RedisManager.pushDocOp = sinon.stub().callsArg(2) + @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArg(2) + @DocOpsManager.pushDocOp @project_id, @doc_id, @op, @callback + + it "should push the op in to the docOps list", -> + @RedisManager.pushDocOp + .calledWith(@doc_id, @op) + .should.equal true + + it "should push the op into the pushUncompressedHistoryOp", -> + @RedisManager.pushUncompressedHistoryOp + .calledWith(@doc_id, @op) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + From dfd3ec993b64fe49c1dcaa5d33b186ae419fd94f Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 26 Feb 2014 14:49:52 +0000 Subject: [PATCH 03/16] Ensure version is still returned from redis --- services/document-updater/app/coffee/DocOpsManager.coffee | 6 ++++-- services/document-updater/app/coffee/ShareJsDB.coffee | 7 ++++--- .../unit/coffee/DocOpsManager/DocOpsManagerTests.coffee | 6 +++--- .../test/unit/coffee/ShareJsDB/WriteOpsTests.coffee | 7 ++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/services/document-updater/app/coffee/DocOpsManager.coffee b/services/document-updater/app/coffee/DocOpsManager.coffee index 43fd7a4aac..971db83358 100644 --- a/services/document-updater/app/coffee/DocOpsManager.coffee +++ b/services/document-updater/app/coffee/DocOpsManager.coffee @@ -44,9 +44,11 @@ module.exports = DocOpsManager = callback null, ops pushDocOp: (project_id, doc_id, op, callback = (error) ->) -> - RedisManager.pushDocOp doc_id, op, (error) -> + RedisManager.pushDocOp doc_id, op, (error, version) -> return callback(error) if error? - RedisManager.pushUncompressedHistoryOp doc_id, op, callback + RedisManager.pushUncompressedHistoryOp doc_id, op, (error) -> + return callback(error) if error? + callback null, version _ensureOpsAreLoaded: (project_id, doc_id, backToVersion, callback = (error) ->) -> RedisManager.getDocVersion doc_id, (error, redisVersion) -> diff --git a/services/document-updater/app/coffee/ShareJsDB.coffee b/services/document-updater/app/coffee/ShareJsDB.coffee index 6eaf21846c..da6640685b 100644 --- a/services/document-updater/app/coffee/ShareJsDB.coffee +++ b/services/document-updater/app/coffee/ShareJsDB.coffee @@ -4,6 +4,7 @@ DocumentManager = require "./DocumentManager" RedisManager = require "./RedisManager" DocOpsManager = require "./DocOpsManager" Errors = require "./Errors" +logger = require "logger-sharelatex" module.exports = ShareJsDB = getOps: (doc_key, start, end, callback) -> @@ -29,9 +30,9 @@ module.exports = ShareJsDB = if version == opData.v + 1 callback() else - # The document has been corrupted by the change. For now, throw an exception. - # Later, rebuild the snapshot. - callback "Version mismatch in db.append. '#{doc_id}' is corrupted." + error = new Error("Version mismatch. '#{doc_id}' is corrupted.") + logger.error err: error, doc_id: doc_id, project_id: project_id, opVersion: opData.v, expectedVersion: version, "doc is corrupt" + callback error getSnapshot: (doc_key, callback) -> [project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key) diff --git a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee index c26171d98f..df2a546480 100644 --- a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee @@ -309,7 +309,7 @@ describe "DocOpsManager", -> describe "pushDocOp", -> beforeEach -> @op = "mock-op" - @RedisManager.pushDocOp = sinon.stub().callsArg(2) + @RedisManager.pushDocOp = sinon.stub().callsArgWith(2, null, @version = 42) @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArg(2) @DocOpsManager.pushDocOp @project_id, @doc_id, @op, @callback @@ -323,7 +323,7 @@ describe "DocOpsManager", -> .calledWith(@doc_id, @op) .should.equal true - it "should call the callback", -> - @callback.called.should.equal true + it "should call the callback with the version", -> + @callback.calledWith(null, @version).should.equal true diff --git a/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee b/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee index b28f23d2f4..6088de77f4 100644 --- a/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee +++ b/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee @@ -26,11 +26,8 @@ describe "ShareJsDB.writeOps", -> @ShareJsDB.writeOp @doc_key, @opData, @callback it "should write the op to redis", -> - op = - op: @opData.op - meta: @opData.meta @DocOpsManager.pushDocOp - .calledWith(@project_id, @doc_id, op) + .calledWith(@project_id, @doc_id, @opData) .should.equal true it "should call the callback without an error", -> @@ -46,7 +43,7 @@ describe "ShareJsDB.writeOps", -> @ShareJsDB.writeOp @doc_key, @opData, @callback it "should call the callback with an error", -> - @callback.calledWith(sinon.match.string).should.equal true + @callback.calledWith(new Error()).should.equal true From f3192da87f1f7f90ee2250f401ea7a5f7d4c1110 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 26 Feb 2014 15:56:52 +0000 Subject: [PATCH 04/16] Tell track changes api to flush doc when flushing doc to mongo --- services/document-updater/Gruntfile.coffee | 6 +-- services/document-updater/app.coffee | 9 ----- .../app/coffee/DocumentManager.coffee | 5 +++ .../app/coffee/TrackChangesManager.coffee | 20 ++++++++++ .../config/settings.development.coffee | 2 + .../coffee/ApplyingUpdatesToADocTests.coffee | 18 +++++++++ .../coffee/FlushingAProjectTests.coffee | 14 +++++++ .../coffee/FlushingDocsTests.coffee | 12 +++++- .../coffee/helpers/MockTrackChangesApi.coffee | 20 ++++++++++ .../coffee/helpers/MockWebApi.coffee | 3 +- .../DocumentManager/flushDocTests.coffee | 8 ++++ .../TrackChangesManagerTests.coffee | 38 +++++++++++++++++++ 12 files changed, 141 insertions(+), 14 deletions(-) create mode 100644 services/document-updater/app/coffee/TrackChangesManager.coffee create mode 100644 services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee create mode 100644 services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee diff --git a/services/document-updater/Gruntfile.coffee b/services/document-updater/Gruntfile.coffee index 717ebd464c..8905cabf81 100644 --- a/services/document-updater/Gruntfile.coffee +++ b/services/document-updater/Gruntfile.coffee @@ -45,16 +45,16 @@ module.exports = (grunt) -> clean: app: ["app/js"] - acceptance_tests: ["test/unit/js"] + acceptance_tests: ["test/acceptance/js"] mochaTest: unit: - src: ['test/unit/js/**/*.js'] + src: ["test/unit/js/#{grunt.option('feature') or '**'}/*.js"] options: reporter: grunt.option('reporter') or 'spec' grep: grunt.option("grep") acceptance: - src: ['test/acceptance/js/**/*.js'] + src: ["test/acceptance/js/#{grunt.option('feature') or '*'}.js"] options: reporter: grunt.option('reporter') or 'spec' grep: grunt.option("grep") diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index 1974169f4e..7168017790 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -21,15 +21,6 @@ app.configure -> app.use express.bodyParser() app.use app.router -app.configure 'development', ()-> - console.log "Development Enviroment" - app.use express.errorHandler({ dumpExceptions: true, showStack: true }) - -app.configure 'production', ()-> - console.log "Production Enviroment" - app.use express.logger() - app.use express.errorHandler() - rclient.subscribe("pending-updates") rclient.on "message", (channel, doc_key)-> [project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index aa64ac3d7f..2f3cfb8d79 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -2,6 +2,7 @@ RedisManager = require "./RedisManager" PersistenceManager = require "./PersistenceManager" DocOpsManager = require "./DocOpsManager" DiffCodec = require "./DiffCodec" +TrackChangesManager = require "./TrackChangesManager" logger = require "logger-sharelatex" Metrics = require "./Metrics" @@ -81,6 +82,10 @@ module.exports = DocumentManager = timer.done() _callback(args...) + TrackChangesManager.flushDocChanges doc_id, (error) -> + if error? + logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc to track changes api" + RedisManager.getDoc doc_id, (error, lines, version) -> return callback(error) if error? if !lines? or !version? diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee new file mode 100644 index 0000000000..c0b40331dc --- /dev/null +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -0,0 +1,20 @@ +settings = require "settings-sharelatex" +request = require "request" +logger = require "logger-sharelatex" + +module.exports = + flushDocChanges: (doc_id, callback = (error) ->) -> + if !settings.apis?.trackchanges? + logger.warn doc_id: doc_id, "track changes API is not configured, so not flushing" + return callback() + + url = "#{settings.apis.trackchanges.url}/doc/#{doc_id}/flush" + logger.log doc_id: doc_id, url: url, "flushing doc in track changes api" + request.post url, (error, res, body)-> + if error? + return callback(error) + else if res.statusCode >= 200 and res.statusCode < 300 + return callback(null) + else + error = new Error("track changes api returned a failure status code: #{res.statusCode}") + return callback(error) diff --git a/services/document-updater/config/settings.development.coffee b/services/document-updater/config/settings.development.coffee index d730bb0f2d..b4f12ed81c 100755 --- a/services/document-updater/config/settings.development.coffee +++ b/services/document-updater/config/settings.development.coffee @@ -12,6 +12,8 @@ module.exports = url: "http://localhost:3000" user: "sharelatex" pass: "password" + trackchanges: + url: "http://localhost:3014" redis: web: diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 5108a4c2cc..ac1a24223a 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -5,6 +5,7 @@ async = require "async" mongojs = require "../../../app/js/mongojs" db = mongojs.db ObjectId = mongojs.ObjectId +rclient = require("redis").createClient() MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -45,6 +46,11 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @result done() + it "should push the applied updates to the track changes api", (done) -> + rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => + JSON.parse(updates[0]).op.should.deep.equal @update.op + done() + describe "when the document is loaded", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -69,6 +75,11 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @result done() + it "should push the applied updates to the track changes api", (done) -> + rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => + JSON.parse(updates[0]).op.should.deep.equal @update.op + done() + describe "when the document has been deleted", -> describe "when the ops come in a single linear order", -> before -> @@ -112,6 +123,13 @@ describe "Applying updates to a doc", -> doc.lines.should.deep.equal @result done() + it "should push the applied updates to the track changes api", (done) -> + rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => + updates = (JSON.parse(u) for u in updates) + for appliedUpdate, i in @updates + appliedUpdate.op.should.deep.equal updates[i].op + done() + describe "when older ops come in after the delete", -> before -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] diff --git a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee index 02b44e3fd6..9adbc2458c 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee @@ -4,6 +4,7 @@ chai.should() async = require "async" MockWebApi = require "./helpers/MockWebApi" +MockTrackChangesApi = require "./helpers/MockTrackChangesApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Flushing a project", -> @@ -40,6 +41,8 @@ describe "Flushing a project", -> describe "with documents which have been updated", -> before (done) -> sinon.spy MockWebApi, "setDocumentLines" + sinon.spy MockTrackChangesApi, "flushDoc" + async.series @docs.map((doc) => (callback) => DocUpdaterClient.preloadDoc @project_id, doc.id, (error) => @@ -56,6 +59,7 @@ describe "Flushing a project", -> after -> MockWebApi.setDocumentLines.restore() + MockTrackChangesApi.flushDoc.restore() it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -74,3 +78,13 @@ describe "Flushing a project", -> callback() ), done + it "should flush the docs in the track changes api", (done) -> + # This is done in the background, so wait a little while to ensure it has happened + setTimeout () => + async.series @docs.map((doc) => + (callback) => + MockTrackChangesApi.flushDoc.calledWith(doc.id).should.equal true + ), done + done() + , 100 + diff --git a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee index aaaef99936..6d67dd68a0 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee @@ -4,6 +4,7 @@ chai.should() async = require "async" MockWebApi = require "./helpers/MockWebApi" +MockTrackChangesApi = require "./helpers/MockTrackChangesApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" mongojs = require "../../../app/js/mongojs" db = mongojs.db @@ -31,6 +32,7 @@ describe "Flushing a doc to Mongo", -> lines: @lines } sinon.spy MockWebApi, "setDocumentLines" + sinon.spy MockTrackChangesApi, "flushDoc" DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) => throw error if error? @@ -40,6 +42,7 @@ describe "Flushing a doc to Mongo", -> after -> MockWebApi.setDocumentLines.restore() + MockTrackChangesApi.flushDoc.restore() it "should flush the updated document to the web api", -> MockWebApi.setDocumentLines @@ -52,6 +55,13 @@ describe "Flushing a doc to Mongo", -> doc.docOps[0].op.should.deep.equal @update.op done() + it "should flush the doc in the track changes api", (done) -> + # This is done in the background, so wait a little while to ensure it has happened + setTimeout () => + MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true + done() + , 100 + describe "when the doc has a large number of ops to be flushed", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] @@ -93,5 +103,5 @@ describe "Flushing a doc to Mongo", -> it "should not flush the doc to the web api", -> MockWebApi.setDocumentLines.called.should.equal false - + diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee new file mode 100644 index 0000000000..2fdff0d3ca --- /dev/null +++ b/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee @@ -0,0 +1,20 @@ +express = require("express") +app = express() + +module.exports = MockTrackChangesApi = + flushDoc: (doc_id, callback = (error) ->) -> + callback() + + run: () -> + app.post "/doc/:doc_id/flush", (req, res, next) => + @flushDoc req.params.doc_id, (error) -> + if error? + res.send 500 + else + res.send 204 + + app.listen 3014, (error) -> + throw error if error? + +MockTrackChangesApi.run() + diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee index 7d50eb8377..693e98f8ad 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockWebApi.coffee @@ -34,7 +34,8 @@ module.exports = MockWebApi = else res.send 204 - app.listen(3000) + app.listen 3000, (error) -> + throw error if error? MockWebApi.run() diff --git a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee index 079341a536..5a4adc4a36 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee @@ -10,6 +10,7 @@ describe "DocumentUpdater - flushDocIfLoaded", -> "./RedisManager": @RedisManager = {} "./PersistenceManager": @PersistenceManager = {} "./DocOpsManager": @DocOpsManager = {} + "./TrackChangesManager": @TrackChangesManager = {} "logger-sharelatex": @logger = {log: sinon.stub()} "./Metrics": @Metrics = Timer: class Timer @@ -25,6 +26,7 @@ describe "DocumentUpdater - flushDocIfLoaded", -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version) @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback it "should get the doc from redis", -> @@ -48,11 +50,17 @@ describe "DocumentUpdater - flushDocIfLoaded", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true + it "should flush the doc in the track changes api", -> + @TrackChangesManager.flushDocChanges + .calledWith(@doc_id) + .should.equal true + describe "when the document is not in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null) @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback it "should get the doc from redis", -> diff --git a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee new file mode 100644 index 0000000000..672cdcfaa4 --- /dev/null +++ b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee @@ -0,0 +1,38 @@ +SandboxedModule = require('sandboxed-module') +sinon = require('sinon') +require('chai').should() +modulePath = require('path').join __dirname, '../../../../app/js/TrackChangesManager' + +describe "TrackChangesManager", -> + beforeEach -> + @TrackChangesManager = SandboxedModule.require modulePath, requires: + "request": @request = {} + "settings-sharelatex": @Settings = {} + @doc_id = "mock-doc-id" + @callback = sinon.stub() + + describe "flushDocChanges", -> + beforeEach -> + @Settings.apis = + trackchanges: url: "http://trackchanges.example.com" + + describe "successfully", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204) + @TrackChangesManager.flushDocChanges @doc_id, @callback + + it "should send a request to the track changes api", -> + @request.post + .calledWith("#{@Settings.apis.trackchanges.url}/doc/#{@doc_id}/flush") + .should.equal true + + it "should return the callback", -> + @callback.calledWith(null).should.equal true + + describe "when the track changes api returns an error", -> + beforeEach -> + @request.post = sinon.stub().callsArgWith(1, null, statusCode: 500) + @TrackChangesManager.flushDocChanges @doc_id, @callback + + it "should return the callback with an error", -> + @callback.calledWith(new Error("track changes api return non-success code: 500")).should.equal true From 77c5a27e12360d6ab82e8c9594343b0749163056 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 26 Feb 2014 16:54:35 +0000 Subject: [PATCH 05/16] Set up acceptance tests in TravisCI --- services/document-updater/.gitignore | 2 ++ services/document-updater/.travis.yml | 4 ++++ services/document-updater/Gruntfile.coffee | 6 ++++++ services/document-updater/package.json | 3 ++- 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/services/document-updater/.gitignore b/services/document-updater/.gitignore index 5755e37b12..a477cfd66c 100644 --- a/services/document-updater/.gitignore +++ b/services/document-updater/.gitignore @@ -43,4 +43,6 @@ app/js/* test/unit/js/* test/acceptance/js/* +forever/ + **.swp diff --git a/services/document-updater/.travis.yml b/services/document-updater/.travis.yml index 29f5884d60..6adc08643a 100644 --- a/services/document-updater/.travis.yml +++ b/services/document-updater/.travis.yml @@ -10,8 +10,12 @@ install: - npm install - grunt install +before_script: + - grunt forever:app:start + script: - grunt test:unit + - grunt test:acceptance services: - redis-server diff --git a/services/document-updater/Gruntfile.coffee b/services/document-updater/Gruntfile.coffee index 8905cabf81..8c96ea0650 100644 --- a/services/document-updater/Gruntfile.coffee +++ b/services/document-updater/Gruntfile.coffee @@ -5,8 +5,14 @@ module.exports = (grunt) -> grunt.loadNpmTasks 'grunt-available-tasks' grunt.loadNpmTasks 'grunt-execute' grunt.loadNpmTasks 'grunt-bunyan' + grunt.loadNpmTasks 'grunt-forever' grunt.initConfig + forever: + app: + options: + index: "app.js" + execute: app: src: "app.js" diff --git a/services/document-updater/package.json b/services/document-updater/package.json index 0bc012d4a6..fbcad4abc1 100644 --- a/services/document-updater/package.json +++ b/services/document-updater/package.json @@ -25,6 +25,7 @@ "grunt-available-tasks": "~0.4.1", "grunt-contrib-coffee": "~0.10.0", "bunyan": "~0.22.1", - "grunt-bunyan": "~0.5.0" + "grunt-bunyan": "~0.5.0", + "grunt-forever": "~0.4.2" } } From 3d70f9126e5f44ae94a3c81bd19323a907ae1721 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 28 Feb 2014 18:29:05 +0000 Subject: [PATCH 06/16] Flush track changes api every 50 updates --- .../app/coffee/DocOpsManager.coffee | 3 +- .../app/coffee/DocumentManager.coffee | 5 --- .../app/coffee/TrackChangesManager.coffee | 15 +++++++- .../coffee/ApplyingUpdatesToADocTests.coffee | 28 ++++++++++++++- .../coffee/FlushingAProjectTests.coffee | 13 ------- .../coffee/FlushingDocsTests.coffee | 10 ------ .../DocOpsManager/DocOpsManagerTests.coffee | 5 +-- .../DocumentManager/flushDocTests.coffee | 8 ----- .../pushUncompressedHistoryOpTests.coffee | 6 ++-- .../TrackChangesManagerTests.coffee | 35 +++++++++++++++++++ 10 files changed, 84 insertions(+), 44 deletions(-) diff --git a/services/document-updater/app/coffee/DocOpsManager.coffee b/services/document-updater/app/coffee/DocOpsManager.coffee index 971db83358..180f1e564b 100644 --- a/services/document-updater/app/coffee/DocOpsManager.coffee +++ b/services/document-updater/app/coffee/DocOpsManager.coffee @@ -5,6 +5,7 @@ ObjectId = mongojs.ObjectId logger = require "logger-sharelatex" async = require "async" Metrics = require("./Metrics") +TrackChangesManager = require "./TrackChangesManager" module.exports = DocOpsManager = flushDocOpsToMongo: (project_id, doc_id, _callback = (error) ->) -> @@ -46,7 +47,7 @@ module.exports = DocOpsManager = pushDocOp: (project_id, doc_id, op, callback = (error) ->) -> RedisManager.pushDocOp doc_id, op, (error, version) -> return callback(error) if error? - RedisManager.pushUncompressedHistoryOp doc_id, op, (error) -> + TrackChangesManager.pushUncompressedHistoryOp doc_id, op, (error) -> return callback(error) if error? callback null, version diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index 2f3cfb8d79..aa64ac3d7f 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -2,7 +2,6 @@ RedisManager = require "./RedisManager" PersistenceManager = require "./PersistenceManager" DocOpsManager = require "./DocOpsManager" DiffCodec = require "./DiffCodec" -TrackChangesManager = require "./TrackChangesManager" logger = require "logger-sharelatex" Metrics = require "./Metrics" @@ -82,10 +81,6 @@ module.exports = DocumentManager = timer.done() _callback(args...) - TrackChangesManager.flushDocChanges doc_id, (error) -> - if error? - logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc to track changes api" - RedisManager.getDoc doc_id, (error, lines, version) -> return callback(error) if error? if !lines? or !version? diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee index c0b40331dc..489694fbf4 100644 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -1,8 +1,9 @@ settings = require "settings-sharelatex" request = require "request" logger = require "logger-sharelatex" +RedisManager = require "./RedisManager" -module.exports = +module.exports = TrackChangesManager = flushDocChanges: (doc_id, callback = (error) ->) -> if !settings.apis?.trackchanges? logger.warn doc_id: doc_id, "track changes API is not configured, so not flushing" @@ -18,3 +19,15 @@ module.exports = else error = new Error("track changes api returned a failure status code: #{res.statusCode}") return callback(error) + + FLUSH_EVERY_N_OPS: 50 + pushUncompressedHistoryOp: (doc_id, op, callback = (error) ->) -> + RedisManager.pushUncompressedHistoryOp doc_id, op, (error, length) -> + if length > 0 and length % TrackChangesManager.FLUSH_EVERY_N_OPS == 0 + # Do this in the background since it uses HTTP and so may be too + # slow to wait for when processing a doc update. + logger.log length: length, doc_id: doc_id, "flushing track changes api" + TrackChangesManager.flushDocChanges doc_id, (error) -> + if error? + logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc to track changes api" + callback() diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index ac1a24223a..5b867b5807 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -7,6 +7,7 @@ db = mongojs.db ObjectId = mongojs.ObjectId rclient = require("redis").createClient() +MockTrackChangesApi = require "./helpers/MockTrackChangesApi" MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" @@ -230,4 +231,29 @@ describe "Applying updates to a doc", -> DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) => doc.lines.should.deep.equal @lines done() - + + describe "with enough updates to flush to the track changes api", -> + before (done) -> + [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] + MockWebApi.insertDoc @project_id, @doc_id, { + lines: @lines + } + @updates = [] + for v in [0..99] # Should flush after 50 ops + @updates.push + doc_id: @doc_id, + op: [i: v.toString(), p: 0] + v: v + + sinon.spy MockTrackChangesApi, "flushDoc" + + DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => + throw error if error? + setTimeout done, 200 + + after -> + MockTrackChangesApi.flushDoc.restore() + + it "should flush the doc twice", -> + console.log MockTrackChangesApi.flushDoc.args + MockTrackChangesApi.flushDoc.calledTwice.should.equal true diff --git a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee index 9adbc2458c..b78decd5a9 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingAProjectTests.coffee @@ -4,7 +4,6 @@ chai.should() async = require "async" MockWebApi = require "./helpers/MockWebApi" -MockTrackChangesApi = require "./helpers/MockTrackChangesApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Flushing a project", -> @@ -41,7 +40,6 @@ describe "Flushing a project", -> describe "with documents which have been updated", -> before (done) -> sinon.spy MockWebApi, "setDocumentLines" - sinon.spy MockTrackChangesApi, "flushDoc" async.series @docs.map((doc) => (callback) => @@ -59,7 +57,6 @@ describe "Flushing a project", -> after -> MockWebApi.setDocumentLines.restore() - MockTrackChangesApi.flushDoc.restore() it "should return a 204 status code", -> @statusCode.should.equal 204 @@ -78,13 +75,3 @@ describe "Flushing a project", -> callback() ), done - it "should flush the docs in the track changes api", (done) -> - # This is done in the background, so wait a little while to ensure it has happened - setTimeout () => - async.series @docs.map((doc) => - (callback) => - MockTrackChangesApi.flushDoc.calledWith(doc.id).should.equal true - ), done - done() - , 100 - diff --git a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee index 6d67dd68a0..da0036bd02 100644 --- a/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee +++ b/services/document-updater/test/acceptance/coffee/FlushingDocsTests.coffee @@ -4,7 +4,6 @@ chai.should() async = require "async" MockWebApi = require "./helpers/MockWebApi" -MockTrackChangesApi = require "./helpers/MockTrackChangesApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" mongojs = require "../../../app/js/mongojs" db = mongojs.db @@ -32,7 +31,6 @@ describe "Flushing a doc to Mongo", -> lines: @lines } sinon.spy MockWebApi, "setDocumentLines" - sinon.spy MockTrackChangesApi, "flushDoc" DocUpdaterClient.sendUpdates @project_id, @doc_id, [@update], (error) => throw error if error? @@ -42,7 +40,6 @@ describe "Flushing a doc to Mongo", -> after -> MockWebApi.setDocumentLines.restore() - MockTrackChangesApi.flushDoc.restore() it "should flush the updated document to the web api", -> MockWebApi.setDocumentLines @@ -55,13 +52,6 @@ describe "Flushing a doc to Mongo", -> doc.docOps[0].op.should.deep.equal @update.op done() - it "should flush the doc in the track changes api", (done) -> - # This is done in the background, so wait a little while to ensure it has happened - setTimeout () => - MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true - done() - , 100 - describe "when the doc has a large number of ops to be flushed", -> before (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] diff --git a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee index df2a546480..fe13d1cd55 100644 --- a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee @@ -19,6 +19,7 @@ describe "DocOpsManager", -> "./Metrics": @Metrics = Timer: class Timer done: sinon.stub() + "./TrackChangesManager": @TrackChangesManager = {} describe "flushDocOpsToMongo", -> describe "when versions are consistent", -> @@ -310,7 +311,7 @@ describe "DocOpsManager", -> beforeEach -> @op = "mock-op" @RedisManager.pushDocOp = sinon.stub().callsArgWith(2, null, @version = 42) - @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArg(2) + @TrackChangesManager.pushUncompressedHistoryOp = sinon.stub().callsArg(2) @DocOpsManager.pushDocOp @project_id, @doc_id, @op, @callback it "should push the op in to the docOps list", -> @@ -319,7 +320,7 @@ describe "DocOpsManager", -> .should.equal true it "should push the op into the pushUncompressedHistoryOp", -> - @RedisManager.pushUncompressedHistoryOp + @TrackChangesManager.pushUncompressedHistoryOp .calledWith(@doc_id, @op) .should.equal true diff --git a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee index 5a4adc4a36..079341a536 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/flushDocTests.coffee @@ -10,7 +10,6 @@ describe "DocumentUpdater - flushDocIfLoaded", -> "./RedisManager": @RedisManager = {} "./PersistenceManager": @PersistenceManager = {} "./DocOpsManager": @DocOpsManager = {} - "./TrackChangesManager": @TrackChangesManager = {} "logger-sharelatex": @logger = {log: sinon.stub()} "./Metrics": @Metrics = Timer: class Timer @@ -26,7 +25,6 @@ describe "DocumentUpdater - flushDocIfLoaded", -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, @lines, @version) @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) - @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback it "should get the doc from redis", -> @@ -50,17 +48,11 @@ describe "DocumentUpdater - flushDocIfLoaded", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - it "should flush the doc in the track changes api", -> - @TrackChangesManager.flushDocChanges - .calledWith(@doc_id) - .should.equal true - describe "when the document is not in Redis", -> beforeEach -> @RedisManager.getDoc = sinon.stub().callsArgWith(1, null, null, null) @PersistenceManager.setDoc = sinon.stub().callsArgWith(3) @DocOpsManager.flushDocOpsToMongo = sinon.stub().callsArgWith(2) - @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) @DocumentManager.flushDocIfLoaded @project_id, @doc_id, @callback it "should get the doc from redis", -> diff --git a/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee index 3b743db6e4..415f8e4572 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee @@ -19,7 +19,7 @@ describe "RedisManager.pushUncompressedHistoryOp", -> describe "successfully", -> beforeEach -> @op = { op: [{ i: "foo", p: 4 }] } - @rclient.rpush = sinon.stub().callsArg(2) + @rclient.rpush = sinon.stub().callsArgWith(2, null, @length = 42) @RedisManager.pushUncompressedHistoryOp @doc_id, @op, @callback it "should push the doc op into the doc ops list", -> @@ -27,8 +27,8 @@ describe "RedisManager.pushUncompressedHistoryOp", -> .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@op)) .should.equal true - it "should call the callback", -> - @callback.called.should.equal true + it "should call the callback with the length", -> + @callback.calledWith(null, @length).should.equal true diff --git a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee index 672cdcfaa4..6ff83d7414 100644 --- a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee @@ -8,6 +8,8 @@ describe "TrackChangesManager", -> @TrackChangesManager = SandboxedModule.require modulePath, requires: "request": @request = {} "settings-sharelatex": @Settings = {} + "logger-sharelatex": @logger = { log: sinon.stub() } + "./RedisManager": @RedisManager = {} @doc_id = "mock-doc-id" @callback = sinon.stub() @@ -36,3 +38,36 @@ describe "TrackChangesManager", -> it "should return the callback with an error", -> @callback.calledWith(new Error("track changes api return non-success code: 500")).should.equal true + + describe "pushUncompressedHistoryOp", -> + beforeEach -> + @op = "mock-op" + @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) + + describe "pushing the op", -> + beforeEach -> + @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 1) + @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + + it "should push the op into redis", -> + @RedisManager.pushUncompressedHistoryOp + .calledWith(@doc_id, @op) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + it "should not try to flush the op", -> + @TrackChangesManager.flushDocChanges.called.should.equal false + + describe "when there are a multiple of FLUSH_EVERY_N_OPS ops", -> + beforeEach -> + @RedisManager.pushUncompressedHistoryOp = + sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) + @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + + it "should tell the track changes api to flush", -> + @TrackChangesManager.flushDocChanges + .calledWith(@doc_id) + .should.equal true + From 86195ce7c3df1fe345140a722326159a6961739a Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 28 Feb 2014 19:09:29 +0000 Subject: [PATCH 07/16] Add in load throttling based on a redis key --- .../app/coffee/RedisKeyBuilder.coffee | 1 + .../app/coffee/RedisManager.coffee | 6 ++ .../app/coffee/TrackChangesManager.coffee | 29 +++++++--- .../coffee/ApplyingUpdatesToADocTests.coffee | 36 ++++++++---- .../getHistoryLoadManagerThreshold.coffee | 43 ++++++++++++++ .../TrackChangesManagerTests.coffee | 57 +++++++++++++------ 6 files changed, 136 insertions(+), 36 deletions(-) create mode 100644 services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee diff --git a/services/document-updater/app/coffee/RedisKeyBuilder.coffee b/services/document-updater/app/coffee/RedisKeyBuilder.coffee index 2bd1ed08c8..de2bc85443 100644 --- a/services/document-updater/app/coffee/RedisKeyBuilder.coffee +++ b/services/document-updater/app/coffee/RedisKeyBuilder.coffee @@ -25,6 +25,7 @@ module.exports = docsWithPendingUpdates : DOCIDSWITHPENDINGUPDATES combineProjectIdAndDocId: (project_id, doc_id) -> "#{project_id}:#{doc_id}" splitProjectIdAndDocId: (project_and_doc_id) -> project_and_doc_id.split(":") + historyLoadManagerThreshold: "HistoryLoadManagerThreshold" now : (key)-> d = new Date() d.getDate()+":"+(d.getMonth()+1)+":"+d.getFullYear()+":"+key diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 5f6c880cee..3d8efdbc70 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -164,6 +164,12 @@ module.exports = getDocIdsInProject: (project_id, callback = (error, doc_ids) ->) -> rclient.smembers keys.docsInProject(project_id: project_id), callback + + getHistoryLoadManagerThreshold: (callback = (error, threshold) ->) -> + rclient.get keys.historyLoadManagerThreshold, (error, value) -> + return callback(error) if error? + return callback null, 0 if !value? + callback null, parseInt(value, 10) getDocumentsProjectId = (doc_id, callback)-> diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee index 489694fbf4..0aca12792b 100644 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -2,6 +2,7 @@ settings = require "settings-sharelatex" request = require "request" logger = require "logger-sharelatex" RedisManager = require "./RedisManager" +crypto = require("crypto") module.exports = TrackChangesManager = flushDocChanges: (doc_id, callback = (error) ->) -> @@ -22,12 +23,22 @@ module.exports = TrackChangesManager = FLUSH_EVERY_N_OPS: 50 pushUncompressedHistoryOp: (doc_id, op, callback = (error) ->) -> - RedisManager.pushUncompressedHistoryOp doc_id, op, (error, length) -> - if length > 0 and length % TrackChangesManager.FLUSH_EVERY_N_OPS == 0 - # Do this in the background since it uses HTTP and so may be too - # slow to wait for when processing a doc update. - logger.log length: length, doc_id: doc_id, "flushing track changes api" - TrackChangesManager.flushDocChanges doc_id, (error) -> - if error? - logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc to track changes api" - callback() + RedisManager.getHistoryLoadManagerThreshold (error, threshold) -> + return callback(error) if error? + if TrackChangesManager.getLoadManagerBucket(doc_id) < threshold + RedisManager.pushUncompressedHistoryOp doc_id, op, (error, length) -> + return callback(error) if error? + if length > 0 and length % TrackChangesManager.FLUSH_EVERY_N_OPS == 0 + # Do this in the background since it uses HTTP and so may be too + # slow to wait for when processing a doc update. + logger.log length: length, doc_id: doc_id, "flushing track changes api" + TrackChangesManager.flushDocChanges doc_id, (error) -> + if error? + logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc to track changes api" + callback() + else + callback() + + getLoadManagerBucket: (doc_id) -> + hash = crypto.createHash("md5").update(doc_id).digest("hex") + return parseInt(hash.slice(0,4), 16) % 100 diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 5b867b5807..7d5ac144e7 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -12,7 +12,7 @@ MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Applying updates to a doc", -> - before -> + before (done) -> @lines = ["one", "two", "three"] @update = doc: @doc_id @@ -22,6 +22,8 @@ describe "Applying updates to a doc", -> }] v: 0 @result = ["one", "one and a half", "two", "three"] + rclient.set "HistoryLoadManagerThreshold", 100, (error) => + done() describe "when the document is not loaded", -> before (done) -> @@ -233,7 +235,7 @@ describe "Applying updates to a doc", -> done() describe "with enough updates to flush to the track changes api", -> - before (done) -> + beforeEach -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines @@ -247,13 +249,27 @@ describe "Applying updates to a doc", -> sinon.spy MockTrackChangesApi, "flushDoc" - DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => - throw error if error? - setTimeout done, 200 - - after -> + afterEach -> MockTrackChangesApi.flushDoc.restore() - it "should flush the doc twice", -> - console.log MockTrackChangesApi.flushDoc.args - MockTrackChangesApi.flushDoc.calledTwice.should.equal true + describe "when under the load manager threshold", -> + beforeEach (done) -> + rclient.set "HistoryLoadManagerThreshold", 100, (error) => + throw error if error? + DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => + throw error if error? + setTimeout done, 200 + + it "should flush the doc twice", -> + MockTrackChangesApi.flushDoc.calledTwice.should.equal true + + describe "when over the load manager threshold", -> + beforeEach (done) -> + rclient.set "HistoryLoadManagerThreshold", 0, (error) => + throw error if error? + DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => + throw error if error? + setTimeout done, 200 + + it "should not flush the doc", -> + MockTrackChangesApi.flushDoc.called.should.equal false diff --git a/services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee b/services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee new file mode 100644 index 0000000000..d69cec370c --- /dev/null +++ b/services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee @@ -0,0 +1,43 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +modulePath = "../../../../app/js/RedisManager.js" +SandboxedModule = require('sandboxed-module') + +describe "RedisManager.getHistoryLoadManagerThreshold", -> + beforeEach -> + @RedisManager = SandboxedModule.require modulePath, requires: + "redis": createClient: () => + @rclient = + auth: () -> + "logger-sharelatex": @logger = {log: sinon.stub()} + @callback = sinon.stub() + + describe "with no value", -> + beforeEach -> + @rclient.get = sinon.stub().callsArgWith(1, null, null) + @RedisManager.getHistoryLoadManagerThreshold @callback + + it "should get the value", -> + @rclient.get + .calledWith("HistoryLoadManagerThreshold") + .should.equal true + + it "should call the callback with 0", -> + @callback.calledWith(null, 0).should.equal true + + describe "with a value", -> + beforeEach -> + @rclient.get = sinon.stub().callsArgWith(1, null, "42") + @RedisManager.getHistoryLoadManagerThreshold @callback + + it "should get the value", -> + @rclient.get + .calledWith("HistoryLoadManagerThreshold") + .should.equal true + + it "should call the callback with the numeric value", -> + @callback.calledWith(null, 42).should.equal true + + + diff --git a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee index 6ff83d7414..0d696730b9 100644 --- a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee @@ -44,30 +44,53 @@ describe "TrackChangesManager", -> @op = "mock-op" @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) - describe "pushing the op", -> + describe "when the doc is under the load manager threshold", -> beforeEach -> + @RedisManager.getHistoryLoadManagerThreshold = sinon.stub().callsArgWith(0, null, 40) + @TrackChangesManager.getLoadManagerBucket = sinon.stub().returns(30) + + describe "pushing the op", -> + beforeEach -> + @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 1) + @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + + it "should push the op into redis", -> + @RedisManager.pushUncompressedHistoryOp + .calledWith(@doc_id, @op) + .should.equal true + + it "should call the callback", -> + @callback.called.should.equal true + + it "should not try to flush the op", -> + @TrackChangesManager.flushDocChanges.called.should.equal false + + describe "when there are a multiple of FLUSH_EVERY_N_OPS ops", -> + beforeEach -> + @RedisManager.pushUncompressedHistoryOp = + sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) + @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + + it "should tell the track changes api to flush", -> + @TrackChangesManager.flushDocChanges + .calledWith(@doc_id) + .should.equal true + + + describe "when the doc is over the load manager threshold", -> + beforeEach -> + @RedisManager.getHistoryLoadManagerThreshold = sinon.stub().callsArgWith(0, null, 40) + @TrackChangesManager.getLoadManagerBucket = sinon.stub().returns(50) @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 1) @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback - it "should push the op into redis", -> - @RedisManager.pushUncompressedHistoryOp - .calledWith(@doc_id, @op) - .should.equal true - - it "should call the callback", -> - @callback.called.should.equal true + it "should not push the op", -> + @RedisManager.pushUncompressedHistoryOp.called.should.equal false it "should not try to flush the op", -> @TrackChangesManager.flushDocChanges.called.should.equal false - describe "when there are a multiple of FLUSH_EVERY_N_OPS ops", -> - beforeEach -> - @RedisManager.pushUncompressedHistoryOp = - sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) - @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + it "should call the callback", -> + @callback.called.should.equal true - it "should tell the track changes api to flush", -> - @TrackChangesManager.flushDocChanges - .calledWith(@doc_id) - .should.equal true From 5d45e191f39c98b77b59379c521ada19e45a1352 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 4 Mar 2014 12:39:02 +0000 Subject: [PATCH 08/16] Don't crash when logging out error --- .../app/coffee/TrackChangesManager.coffee | 2 +- .../TrackChangesManagerTests.coffee | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee index 0aca12792b..9c8b514753 100644 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -34,7 +34,7 @@ module.exports = TrackChangesManager = logger.log length: length, doc_id: doc_id, "flushing track changes api" TrackChangesManager.flushDocChanges doc_id, (error) -> if error? - logger.error err: error, project_id: project_id, doc_id: doc_id, "error flushing doc to track changes api" + logger.error err: error, doc_id: doc_id, "error flushing doc to track changes api" callback() else callback() diff --git a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee index 0d696730b9..dd72937c39 100644 --- a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee @@ -8,7 +8,7 @@ describe "TrackChangesManager", -> @TrackChangesManager = SandboxedModule.require modulePath, requires: "request": @request = {} "settings-sharelatex": @Settings = {} - "logger-sharelatex": @logger = { log: sinon.stub() } + "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } "./RedisManager": @RedisManager = {} @doc_id = "mock-doc-id" @callback = sinon.stub() @@ -76,6 +76,21 @@ describe "TrackChangesManager", -> .calledWith(@doc_id) .should.equal true + describe "when TrackChangesManager errors", -> + beforeEach -> + @RedisManager.pushUncompressedHistoryOp = + sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArgWith(1, @error = new Error("oops")) + @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + + it "should log out the error", -> + @logger.error + .calledWith( + err: @error + doc_id: @doc_id + "error flushing doc to track changes api" + ) + .should.equal true describe "when the doc is over the load manager threshold", -> beforeEach -> From 4f878e000be984f0444acde40f6cf160a785060e Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 11 Mar 2014 12:47:26 +0000 Subject: [PATCH 09/16] Allow source and user_id to be included when setting a document --- .../app/coffee/DocumentManager.coffee | 8 ++-- .../app/coffee/HttpController.coffee | 6 ++- .../coffee/SettingADocumentTests.coffee | 4 +- .../coffee/helpers/DocUpdaterClient.coffee | 4 +- .../coffee/DocumentManager/setDocTests.coffee | 45 ++++++++----------- .../coffee/HttpController/setDocTests.coffee | 12 +++-- 6 files changed, 41 insertions(+), 38 deletions(-) diff --git a/services/document-updater/app/coffee/DocumentManager.coffee b/services/document-updater/app/coffee/DocumentManager.coffee index aa64ac3d7f..38f8a9dac4 100644 --- a/services/document-updater/app/coffee/DocumentManager.coffee +++ b/services/document-updater/app/coffee/DocumentManager.coffee @@ -42,7 +42,7 @@ module.exports = DocumentManager = return callback(error) if error? callback null, lines, version, ops - setDoc: (project_id, doc_id, newLines, _callback = (error) ->) -> + setDoc: (project_id, doc_id, newLines, source, user_id, _callback = (error) ->) -> timer = new Metrics.Timer("docManager.setDoc") callback = (args...) -> timer.done() @@ -68,6 +68,8 @@ module.exports = DocumentManager = v: version meta: type: "external" + source: source + user_id: user_id UpdateManager.applyUpdates project_id, doc_id, [update], (error) -> return callback(error) if error? DocumentManager.flushDocIfLoaded project_id, doc_id, (error) -> @@ -114,9 +116,9 @@ module.exports = DocumentManager = UpdateManager = require "./UpdateManager" UpdateManager.lockUpdatesAndDo DocumentManager.getDocAndRecentOps, project_id, doc_id, fromVersion, callback - setDocWithLock: (project_id, doc_id, lines, callback = (error) ->) -> + setDocWithLock: (project_id, doc_id, lines, source, user_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" - UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, callback + UpdateManager.lockUpdatesAndDo DocumentManager.setDoc, project_id, doc_id, lines, source, user_id, callback flushDocIfLoadedWithLock: (project_id, doc_id, callback = (error) ->) -> UpdateManager = require "./UpdateManager" diff --git a/services/document-updater/app/coffee/HttpController.coffee b/services/document-updater/app/coffee/HttpController.coffee index 391d02ee37..ef9fb38e19 100644 --- a/services/document-updater/app/coffee/HttpController.coffee +++ b/services/document-updater/app/coffee/HttpController.coffee @@ -32,9 +32,11 @@ module.exports = HttpController = doc_id = req.params.doc_id project_id = req.params.project_id lines = req.body.lines - logger.log project_id: project_id, doc_id: doc_id, lines: lines, "setting doc via http" + source = req.body.source + user_id = req.body.user_id + logger.log project_id: project_id, doc_id: doc_id, lines: lines, source: source, user_id: user_id, "setting doc via http" timer = new Metrics.Timer("http.setDoc") - DocumentManager.setDocWithLock project_id, doc_id, lines, (error) -> + DocumentManager.setDocWithLock project_id, doc_id, lines, source, user_id, (error) -> timer.done() return next(error) if error? logger.log project_id: project_id, doc_id: doc_id, "set doc via http" diff --git a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee index cc0f30834a..a02cb0250a 100644 --- a/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee +++ b/services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee @@ -18,6 +18,8 @@ describe "Setting a document", -> v: 0 @result = ["one", "one and a half", "two", "three"] @newLines = ["these", "are", "the", "new", "lines"] + @source = "dropbox" + @user_id = "user-id-123" MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines } @@ -30,7 +32,7 @@ describe "Setting a document", -> DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) => throw error if error? setTimeout () => - DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, (error, res, body) => + DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) => @statusCode = res.statusCode done() , 200 diff --git a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee index 4ddef90d26..ec70023876 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/DocUpdaterClient.coffee @@ -43,11 +43,13 @@ module.exports = DocUpdaterClient = request.post "http://localhost:3003/project/#{project_id}/doc/#{doc_id}/flush", (error, res, body) -> callback error, res, body - setDocLines: (project_id, doc_id, lines, callback = (error) ->) -> + setDocLines: (project_id, doc_id, lines, source, user_id, callback = (error) ->) -> request.post { url: "http://localhost:3003/project/#{project_id}/doc/#{doc_id}" json: lines: lines + source: source + user_id: user_id }, (error, res, body) -> callback error, res, body diff --git a/services/document-updater/test/unit/coffee/DocumentManager/setDocTests.coffee b/services/document-updater/test/unit/coffee/DocumentManager/setDocTests.coffee index d4b5e931b8..b827b584f8 100644 --- a/services/document-updater/test/unit/coffee/DocumentManager/setDocTests.coffee +++ b/services/document-updater/test/unit/coffee/DocumentManager/setDocTests.coffee @@ -22,6 +22,8 @@ describe "DocumentManager - setDoc", -> @version = 42 @ops = ["mock-ops"] @callback = sinon.stub() + @source = "dropbox" + @user_id = "mock-user-id" describe "with plain tex lines", -> beforeEach -> @@ -34,7 +36,7 @@ describe "DocumentManager - setDoc", -> @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) @UpdateManager.applyUpdates = sinon.stub().callsArgWith(3, null) @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) - @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @callback + @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback it "should get the current doc lines", -> @DocumentManager.getDoc @@ -48,7 +50,20 @@ describe "DocumentManager - setDoc", -> it "should apply the diff as a ShareJS op", -> @UpdateManager.applyUpdates - .calledWith(@project_id, @doc_id, [doc: @doc_id, v: @version, op: @ops, meta: { type: "external" }]) + .calledWith( + @project_id, + @doc_id, + [ + doc: @doc_id, + v: @version, + op: @ops, + meta: { + type: "external" + source: @source + user_id: @user_id + } + ] + ) .should.equal true it "should flush the doc to Mongo", -> @@ -62,30 +77,6 @@ describe "DocumentManager - setDoc", -> it "should time the execution", -> @Metrics.Timer::done.called.should.equal true - describe "with json lines", -> - beforeEach -> - @beforeLines = [text: "before", text: "lines"] - @afterLines = ["after", "lines"] - - describe "successfully", -> - beforeEach -> - @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version) - @DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops) - @UpdateManager.applyUpdates = sinon.stub().callsArgWith(3, null) - @DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) - @DocumentManager.setDoc @project_id, @doc_id, @afterLines, @callback - - it "should get the current doc lines", -> - @DocumentManager.getDoc - .calledWith(@project_id, @doc_id) - .should.equal true - - it "should return not try to get a diff", -> - @DiffCodec.diffAsShareJsOp.called.should.equal false - - it "should call the callback", -> - @callback.calledWith(null).should.equal true - describe "without new lines", -> beforeEach -> @DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version) @@ -96,7 +87,7 @@ describe "DocumentManager - setDoc", -> it "should not try to get the doc lines", -> @DocumentManager.getDoc.called.should.equal false - + diff --git a/services/document-updater/test/unit/coffee/HttpController/setDocTests.coffee b/services/document-updater/test/unit/coffee/HttpController/setDocTests.coffee index 2c3924c030..dd2a7c1d59 100644 --- a/services/document-updater/test/unit/coffee/HttpController/setDocTests.coffee +++ b/services/document-updater/test/unit/coffee/HttpController/setDocTests.coffee @@ -19,6 +19,8 @@ describe "HttpController - setDoc", -> @project_id = "project-id-123" @doc_id = "doc-id-123" @lines = ["one", "two", "three"] + @source = "dropbox" + @user_id = "user-id-123" @res = send: sinon.stub() @req = @@ -27,16 +29,18 @@ describe "HttpController - setDoc", -> doc_id: @doc_id body: lines: @lines + source: @source + user_id: @user_id @next = sinon.stub() describe "successfully", -> beforeEach -> - @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(3) + @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5) @HttpController.setDoc(@req, @res, @next) it "should set the doc", -> @DocumentManager.setDocWithLock - .calledWith(@project_id, @doc_id) + .calledWith(@project_id, @doc_id, @lines, @source, @user_id) .should.equal true it "should return a successful No Content response", -> @@ -46,7 +50,7 @@ describe "HttpController - setDoc", -> it "should log the request", -> @logger.log - .calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, "setting doc via http") + .calledWith(doc_id: @doc_id, project_id: @project_id, lines: @lines, source: @source, user_id: @user_id, "setting doc via http") .should.equal true it "should time the request", -> @@ -54,7 +58,7 @@ describe "HttpController - setDoc", -> describe "when an errors occurs", -> beforeEach -> - @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(3, new Error("oops")) + @DocumentManager.setDocWithLock = sinon.stub().callsArgWith(5, new Error("oops")) @HttpController.setDoc(@req, @res, @next) it "should call next with the error", -> From 2d28f1903f8def8357baf1d0281e1698b2bed0f8 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 19 Mar 2014 15:56:44 +0000 Subject: [PATCH 10/16] Flush to the track changes api using the project id as well --- .../app/coffee/DocOpsManager.coffee | 2 +- .../app/coffee/TrackChangesManager.coffee | 14 ++++++------ .../coffee/helpers/MockTrackChangesApi.coffee | 2 +- .../DocOpsManager/DocOpsManagerTests.coffee | 4 ++-- .../TrackChangesManagerTests.coffee | 22 ++++++++++--------- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/services/document-updater/app/coffee/DocOpsManager.coffee b/services/document-updater/app/coffee/DocOpsManager.coffee index 180f1e564b..403488208d 100644 --- a/services/document-updater/app/coffee/DocOpsManager.coffee +++ b/services/document-updater/app/coffee/DocOpsManager.coffee @@ -47,7 +47,7 @@ module.exports = DocOpsManager = pushDocOp: (project_id, doc_id, op, callback = (error) ->) -> RedisManager.pushDocOp doc_id, op, (error, version) -> return callback(error) if error? - TrackChangesManager.pushUncompressedHistoryOp doc_id, op, (error) -> + TrackChangesManager.pushUncompressedHistoryOp project_id, doc_id, op, (error) -> return callback(error) if error? callback null, version diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee index 9c8b514753..65b9762fe5 100644 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -5,13 +5,13 @@ RedisManager = require "./RedisManager" crypto = require("crypto") module.exports = TrackChangesManager = - flushDocChanges: (doc_id, callback = (error) ->) -> + flushDocChanges: (project_id, doc_id, callback = (error) ->) -> if !settings.apis?.trackchanges? logger.warn doc_id: doc_id, "track changes API is not configured, so not flushing" return callback() - url = "#{settings.apis.trackchanges.url}/doc/#{doc_id}/flush" - logger.log doc_id: doc_id, url: url, "flushing doc in track changes api" + url = "#{settings.apis.trackchanges.url}/project/#{project_id}/doc/#{doc_id}/flush" + logger.log project_id: project_id, doc_id: doc_id, url: url, "flushing doc in track changes api" request.post url, (error, res, body)-> if error? return callback(error) @@ -22,7 +22,7 @@ module.exports = TrackChangesManager = return callback(error) FLUSH_EVERY_N_OPS: 50 - pushUncompressedHistoryOp: (doc_id, op, callback = (error) ->) -> + pushUncompressedHistoryOp: (project_id, doc_id, op, callback = (error) ->) -> RedisManager.getHistoryLoadManagerThreshold (error, threshold) -> return callback(error) if error? if TrackChangesManager.getLoadManagerBucket(doc_id) < threshold @@ -31,10 +31,10 @@ module.exports = TrackChangesManager = if length > 0 and length % TrackChangesManager.FLUSH_EVERY_N_OPS == 0 # Do this in the background since it uses HTTP and so may be too # slow to wait for when processing a doc update. - logger.log length: length, doc_id: doc_id, "flushing track changes api" - TrackChangesManager.flushDocChanges doc_id, (error) -> + logger.log length: length, doc_id: doc_id, project_id: project_id, "flushing track changes api" + TrackChangesManager.flushDocChanges project_id, doc_id, (error) -> if error? - logger.error err: error, doc_id: doc_id, "error flushing doc to track changes api" + logger.error err: error, doc_id: doc_id, project_id: project_id, "error flushing doc to track changes api" callback() else callback() diff --git a/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee b/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee index 2fdff0d3ca..43416e37fc 100644 --- a/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee +++ b/services/document-updater/test/acceptance/coffee/helpers/MockTrackChangesApi.coffee @@ -6,7 +6,7 @@ module.exports = MockTrackChangesApi = callback() run: () -> - app.post "/doc/:doc_id/flush", (req, res, next) => + app.post "/project/:project_id/doc/:doc_id/flush", (req, res, next) => @flushDoc req.params.doc_id, (error) -> if error? res.send 500 diff --git a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee index fe13d1cd55..1680a367d2 100644 --- a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee @@ -311,7 +311,7 @@ describe "DocOpsManager", -> beforeEach -> @op = "mock-op" @RedisManager.pushDocOp = sinon.stub().callsArgWith(2, null, @version = 42) - @TrackChangesManager.pushUncompressedHistoryOp = sinon.stub().callsArg(2) + @TrackChangesManager.pushUncompressedHistoryOp = sinon.stub().callsArg(3) @DocOpsManager.pushDocOp @project_id, @doc_id, @op, @callback it "should push the op in to the docOps list", -> @@ -321,7 +321,7 @@ describe "DocOpsManager", -> it "should push the op into the pushUncompressedHistoryOp", -> @TrackChangesManager.pushUncompressedHistoryOp - .calledWith(@doc_id, @op) + .calledWith(@project_id, @doc_id, @op) .should.equal true it "should call the callback with the version", -> diff --git a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee index dd72937c39..148eeb33bb 100644 --- a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee @@ -10,6 +10,7 @@ describe "TrackChangesManager", -> "settings-sharelatex": @Settings = {} "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } "./RedisManager": @RedisManager = {} + @project_id = "mock-project-id" @doc_id = "mock-doc-id" @callback = sinon.stub() @@ -21,11 +22,11 @@ describe "TrackChangesManager", -> describe "successfully", -> beforeEach -> @request.post = sinon.stub().callsArgWith(1, null, statusCode: 204) - @TrackChangesManager.flushDocChanges @doc_id, @callback + @TrackChangesManager.flushDocChanges @project_id, @doc_id, @callback it "should send a request to the track changes api", -> @request.post - .calledWith("#{@Settings.apis.trackchanges.url}/doc/#{@doc_id}/flush") + .calledWith("#{@Settings.apis.trackchanges.url}/project/#{@project_id}/doc/#{@doc_id}/flush") .should.equal true it "should return the callback", -> @@ -34,7 +35,7 @@ describe "TrackChangesManager", -> describe "when the track changes api returns an error", -> beforeEach -> @request.post = sinon.stub().callsArgWith(1, null, statusCode: 500) - @TrackChangesManager.flushDocChanges @doc_id, @callback + @TrackChangesManager.flushDocChanges @project_id, @doc_id, @callback it "should return the callback with an error", -> @callback.calledWith(new Error("track changes api return non-success code: 500")).should.equal true @@ -42,7 +43,7 @@ describe "TrackChangesManager", -> describe "pushUncompressedHistoryOp", -> beforeEach -> @op = "mock-op" - @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(1) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(2) describe "when the doc is under the load manager threshold", -> beforeEach -> @@ -52,7 +53,7 @@ describe "TrackChangesManager", -> describe "pushing the op", -> beforeEach -> @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 1) - @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback it "should push the op into redis", -> @RedisManager.pushUncompressedHistoryOp @@ -69,25 +70,26 @@ describe "TrackChangesManager", -> beforeEach -> @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) - @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback it "should tell the track changes api to flush", -> @TrackChangesManager.flushDocChanges - .calledWith(@doc_id) + .calledWith(@project_id, @doc_id) .should.equal true describe "when TrackChangesManager errors", -> beforeEach -> @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) - @TrackChangesManager.flushDocChanges = sinon.stub().callsArgWith(1, @error = new Error("oops")) - @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + @TrackChangesManager.flushDocChanges = sinon.stub().callsArgWith(2, @error = new Error("oops")) + @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback it "should log out the error", -> @logger.error .calledWith( err: @error doc_id: @doc_id + project_id: @project_id "error flushing doc to track changes api" ) .should.equal true @@ -97,7 +99,7 @@ describe "TrackChangesManager", -> @RedisManager.getHistoryLoadManagerThreshold = sinon.stub().callsArgWith(0, null, 40) @TrackChangesManager.getLoadManagerBucket = sinon.stub().returns(50) @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 1) - @TrackChangesManager.pushUncompressedHistoryOp @doc_id, @op, @callback + @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback it "should not push the op", -> @RedisManager.pushUncompressedHistoryOp.called.should.equal false From c0be3ef37b76a6a358defadf31c573ef9c805aa5 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 21 Mar 2014 12:41:05 +0000 Subject: [PATCH 11/16] Put doc_ids with history changes into project level set --- .../app/coffee/RedisKeyBuilder.coffee | 3 +- .../app/coffee/RedisManager.coffee | 16 ++-- .../app/coffee/TrackChangesManager.coffee | 27 ++---- .../coffee/ApplyingUpdatesToADocTests.coffee | 48 ++++------ .../clearDocFromPendingUpdatesSetTests.coffee | 1 + .../getDocsWithPendingUpdatesTests.coffee | 1 + .../getHistoryLoadManagerThreshold.coffee | 43 --------- .../RedisManager/prependDocOpsTests.coffee | 3 +- .../coffee/RedisManager/pushDocOpTests.coffee | 3 +- .../pushUncompressedHistoryOpTests.coffee | 13 ++- .../TrackChangesManagerTests.coffee | 95 ++++++++----------- 11 files changed, 94 insertions(+), 159 deletions(-) delete mode 100644 services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee diff --git a/services/document-updater/app/coffee/RedisKeyBuilder.coffee b/services/document-updater/app/coffee/RedisKeyBuilder.coffee index de2bc85443..0cfd330721 100644 --- a/services/document-updater/app/coffee/RedisKeyBuilder.coffee +++ b/services/document-updater/app/coffee/RedisKeyBuilder.coffee @@ -8,6 +8,7 @@ DOCLINES = "doclines" DOCOPS = "DocOps" DOCVERSION = "DocVersion" DOCIDSWITHPENDINGUPDATES = "DocsWithPendingUpdates" +DOCSWITHHISTORYOPS = "DocsWithHistoryOps" UNCOMPRESSED_HISTORY_OPS = "UncompressedHistoryOps" module.exports = @@ -25,7 +26,7 @@ module.exports = docsWithPendingUpdates : DOCIDSWITHPENDINGUPDATES combineProjectIdAndDocId: (project_id, doc_id) -> "#{project_id}:#{doc_id}" splitProjectIdAndDocId: (project_and_doc_id) -> project_and_doc_id.split(":") - historyLoadManagerThreshold: "HistoryLoadManagerThreshold" + docsWithHistoryOps: (op) -> DOCSWITHHISTORYOPS + ":" + op.project_id now : (key)-> d = new Date() d.getDate()+":"+(d.getMonth()+1)+":"+d.getFullYear()+":"+key diff --git a/services/document-updater/app/coffee/RedisManager.coffee b/services/document-updater/app/coffee/RedisManager.coffee index 3d8efdbc70..c47e679339 100644 --- a/services/document-updater/app/coffee/RedisManager.coffee +++ b/services/document-updater/app/coffee/RedisManager.coffee @@ -155,9 +155,15 @@ module.exports = jsonOps = ops.map (op) -> JSON.stringify op rclient.lpush keys.docOps(doc_id: doc_id), jsonOps.reverse(), callback - pushUncompressedHistoryOp: (doc_id, op, callback = (error) ->) -> + pushUncompressedHistoryOp: (project_id, doc_id, op, callback = (error, length) ->) -> jsonOp = JSON.stringify op - rclient.rpush keys.uncompressedHistoryOp(doc_id: doc_id), jsonOp, callback + multi = rclient.multi() + multi.rpush keys.uncompressedHistoryOp(doc_id: doc_id), jsonOp + multi.sadd keys.docsWithHistoryOps(project_id: project_id), doc_id + multi.exec (error, results) -> + return callback(error) if error? + [length, _] = results + callback(error, length) getDocOpsLength: (doc_id, callback = (error, length) ->) -> rclient.llen keys.docOps(doc_id: doc_id), callback @@ -165,12 +171,6 @@ module.exports = getDocIdsInProject: (project_id, callback = (error, doc_ids) ->) -> rclient.smembers keys.docsInProject(project_id: project_id), callback - getHistoryLoadManagerThreshold: (callback = (error, threshold) ->) -> - rclient.get keys.historyLoadManagerThreshold, (error, value) -> - return callback(error) if error? - return callback null, 0 if !value? - callback null, parseInt(value, 10) - getDocumentsProjectId = (doc_id, callback)-> rclient.get keys.projectKey({doc_id:doc_id}), (err, project_id)-> diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee index 65b9762fe5..2d887a4f9c 100644 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -23,22 +23,15 @@ module.exports = TrackChangesManager = FLUSH_EVERY_N_OPS: 50 pushUncompressedHistoryOp: (project_id, doc_id, op, callback = (error) ->) -> - RedisManager.getHistoryLoadManagerThreshold (error, threshold) -> + logger.log project_id: project_id, doc_id: doc_id, "pushing history op" + RedisManager.pushUncompressedHistoryOp project_id, doc_id, op, (error, length) -> return callback(error) if error? - if TrackChangesManager.getLoadManagerBucket(doc_id) < threshold - RedisManager.pushUncompressedHistoryOp doc_id, op, (error, length) -> - return callback(error) if error? - if length > 0 and length % TrackChangesManager.FLUSH_EVERY_N_OPS == 0 - # Do this in the background since it uses HTTP and so may be too - # slow to wait for when processing a doc update. - logger.log length: length, doc_id: doc_id, project_id: project_id, "flushing track changes api" - TrackChangesManager.flushDocChanges project_id, doc_id, (error) -> - if error? - logger.error err: error, doc_id: doc_id, project_id: project_id, "error flushing doc to track changes api" - callback() - else - callback() + if length > 0 and length % TrackChangesManager.FLUSH_EVERY_N_OPS == 0 + # Do this in the background since it uses HTTP and so may be too + # slow to wait for when processing a doc update. + logger.log length: length, doc_id: doc_id, project_id: project_id, "flushing track changes api" + TrackChangesManager.flushDocChanges project_id, doc_id, (error) -> + if error? + logger.error err: error, doc_id: doc_id, project_id: project_id, "error flushing doc to track changes api" + callback() - getLoadManagerBucket: (doc_id) -> - hash = crypto.createHash("md5").update(doc_id).digest("hex") - return parseInt(hash.slice(0,4), 16) % 100 diff --git a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee index 7d5ac144e7..6b4010b305 100644 --- a/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee +++ b/services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee @@ -12,7 +12,7 @@ MockWebApi = require "./helpers/MockWebApi" DocUpdaterClient = require "./helpers/DocUpdaterClient" describe "Applying updates to a doc", -> - before (done) -> + before -> @lines = ["one", "two", "three"] @update = doc: @doc_id @@ -22,8 +22,6 @@ describe "Applying updates to a doc", -> }] v: 0 @result = ["one", "one and a half", "two", "three"] - rclient.set "HistoryLoadManagerThreshold", 100, (error) => - done() describe "when the document is not loaded", -> before (done) -> @@ -51,8 +49,13 @@ describe "Applying updates to a doc", -> it "should push the applied updates to the track changes api", (done) -> rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => + throw error if error? JSON.parse(updates[0]).op.should.deep.equal @update.op - done() + rclient.sismember "DocsWithHistoryOps:#{@project_id}", @doc_id, (error, result) => + throw error if error? + result.should.equal 1 + done() + describe "when the document is loaded", -> before (done) -> @@ -81,7 +84,9 @@ describe "Applying updates to a doc", -> it "should push the applied updates to the track changes api", (done) -> rclient.lrange "UncompressedHistoryOps:#{@doc_id}", 0, -1, (error, updates) => JSON.parse(updates[0]).op.should.deep.equal @update.op - done() + rclient.sismember "DocsWithHistoryOps:#{@project_id}", @doc_id, (error, result) => + result.should.equal 1 + done() describe "when the document has been deleted", -> describe "when the ops come in a single linear order", -> @@ -131,7 +136,10 @@ describe "Applying updates to a doc", -> updates = (JSON.parse(u) for u in updates) for appliedUpdate, i in @updates appliedUpdate.op.should.deep.equal updates[i].op - done() + + rclient.sismember "DocsWithHistoryOps:#{@project_id}", @doc_id, (error, result) => + result.should.equal 1 + done() describe "when older ops come in after the delete", -> before -> @@ -235,7 +243,7 @@ describe "Applying updates to a doc", -> done() describe "with enough updates to flush to the track changes api", -> - beforeEach -> + beforeEach (done) -> [@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()] MockWebApi.insertDoc @project_id, @doc_id, { lines: @lines @@ -249,27 +257,13 @@ describe "Applying updates to a doc", -> sinon.spy MockTrackChangesApi, "flushDoc" + DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => + throw error if error? + setTimeout done, 200 + afterEach -> MockTrackChangesApi.flushDoc.restore() - describe "when under the load manager threshold", -> - beforeEach (done) -> - rclient.set "HistoryLoadManagerThreshold", 100, (error) => - throw error if error? - DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => - throw error if error? - setTimeout done, 200 + it "should flush the doc twice", -> + MockTrackChangesApi.flushDoc.calledTwice.should.equal true - it "should flush the doc twice", -> - MockTrackChangesApi.flushDoc.calledTwice.should.equal true - - describe "when over the load manager threshold", -> - beforeEach (done) -> - rclient.set "HistoryLoadManagerThreshold", 0, (error) => - throw error if error? - DocUpdaterClient.sendUpdates @project_id, @doc_id, @updates, (error) => - throw error if error? - setTimeout done, 200 - - it "should not flush the doc", -> - MockTrackChangesApi.flushDoc.called.should.equal false diff --git a/services/document-updater/test/unit/coffee/RedisManager/clearDocFromPendingUpdatesSetTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/clearDocFromPendingUpdatesSetTests.coffee index 676d454167..016d96a2ae 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/clearDocFromPendingUpdatesSetTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/clearDocFromPendingUpdatesSetTests.coffee @@ -12,6 +12,7 @@ describe "RedisManager.clearDocFromPendingUpdatesSet", -> @RedisManager = SandboxedModule.require modulePath, requires: "redis" : createClient: () => @rclient = auth:-> + "logger-sharelatex": {} @rclient.srem = sinon.stub().callsArg(2) @RedisManager.clearDocFromPendingUpdatesSet(@project_id, @doc_id, @callback) diff --git a/services/document-updater/test/unit/coffee/RedisManager/getDocsWithPendingUpdatesTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/getDocsWithPendingUpdatesTests.coffee index 602197ad57..d179b45f9d 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/getDocsWithPendingUpdatesTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/getDocsWithPendingUpdatesTests.coffee @@ -10,6 +10,7 @@ describe "RedisManager.getDocsWithPendingUpdates", -> @RedisManager = SandboxedModule.require modulePath, requires: "redis" : createClient: () => @rclient = auth:-> + "logger-sharelatex": {} @docs = [{ doc_id: "doc-id-1" diff --git a/services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee b/services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee deleted file mode 100644 index d69cec370c..0000000000 --- a/services/document-updater/test/unit/coffee/RedisManager/getHistoryLoadManagerThreshold.coffee +++ /dev/null @@ -1,43 +0,0 @@ -sinon = require('sinon') -chai = require('chai') -should = chai.should() -modulePath = "../../../../app/js/RedisManager.js" -SandboxedModule = require('sandboxed-module') - -describe "RedisManager.getHistoryLoadManagerThreshold", -> - beforeEach -> - @RedisManager = SandboxedModule.require modulePath, requires: - "redis": createClient: () => - @rclient = - auth: () -> - "logger-sharelatex": @logger = {log: sinon.stub()} - @callback = sinon.stub() - - describe "with no value", -> - beforeEach -> - @rclient.get = sinon.stub().callsArgWith(1, null, null) - @RedisManager.getHistoryLoadManagerThreshold @callback - - it "should get the value", -> - @rclient.get - .calledWith("HistoryLoadManagerThreshold") - .should.equal true - - it "should call the callback with 0", -> - @callback.calledWith(null, 0).should.equal true - - describe "with a value", -> - beforeEach -> - @rclient.get = sinon.stub().callsArgWith(1, null, "42") - @RedisManager.getHistoryLoadManagerThreshold @callback - - it "should get the value", -> - @rclient.get - .calledWith("HistoryLoadManagerThreshold") - .should.equal true - - it "should call the callback with the numeric value", -> - @callback.calledWith(null, 42).should.equal true - - - diff --git a/services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee index b4a8192d12..dee8b2f435 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/prependDocOpsTests.coffee @@ -4,13 +4,14 @@ should = chai.should() modulePath = "../../../../app/js/RedisManager" SandboxedModule = require('sandboxed-module') -describe "RedisManager.clearDocFromPendingUpdatesSet", -> +describe "RedisManager.prependDocOps", -> beforeEach -> @doc_id = "document-id" @callback = sinon.stub() @RedisManager = SandboxedModule.require modulePath, requires: "redis" : createClient: () => @rclient = auth:-> + "logger-sharelatex": {} @rclient.lpush = sinon.stub().callsArg(2) @ops = [ diff --git a/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee index 0c76730437..d911af16bd 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/pushDocOpTests.coffee @@ -4,7 +4,7 @@ should = chai.should() modulePath = "../../../../app/js/RedisManager" SandboxedModule = require('sandboxed-module') -describe "RedisManager.getPreviousDocOpsTests", -> +describe "RedisManager.pushDocOp", -> beforeEach -> @callback = sinon.stub() @RedisManager = SandboxedModule.require modulePath, requires: @@ -12,6 +12,7 @@ describe "RedisManager.getPreviousDocOpsTests", -> @rclient = auth: -> multi: => @rclient + "logger-sharelatex": {} @doc_id = "doc-id-123" beforeEach -> diff --git a/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee b/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee index 415f8e4572..d6e19f163e 100644 --- a/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee +++ b/services/document-updater/test/unit/coffee/RedisManager/pushUncompressedHistoryOpTests.coffee @@ -13,20 +13,27 @@ describe "RedisManager.pushUncompressedHistoryOp", -> multi: () => @rclient "logger-sharelatex": @logger = {log: sinon.stub()} @doc_id = "doc-id-123" + @project_id = "project-id-123" @callback = sinon.stub() - @rclient.rpush = sinon.stub() describe "successfully", -> beforeEach -> @op = { op: [{ i: "foo", p: 4 }] } - @rclient.rpush = sinon.stub().callsArgWith(2, null, @length = 42) - @RedisManager.pushUncompressedHistoryOp @doc_id, @op, @callback + @rclient.rpush = sinon.stub() + @rclient.sadd = sinon.stub() + @rclient.exec = sinon.stub().callsArgWith(0, null, [@length = 42, "1"]) + @RedisManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback it "should push the doc op into the doc ops list", -> @rclient.rpush .calledWith("UncompressedHistoryOps:#{@doc_id}", JSON.stringify(@op)) .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 diff --git a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee index 148eeb33bb..8fad5322e2 100644 --- a/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/TrackChangesManager/TrackChangesManagerTests.coffee @@ -45,69 +45,48 @@ describe "TrackChangesManager", -> @op = "mock-op" @TrackChangesManager.flushDocChanges = sinon.stub().callsArg(2) - describe "when the doc is under the load manager threshold", -> + describe "pushing the op", -> beforeEach -> - @RedisManager.getHistoryLoadManagerThreshold = sinon.stub().callsArgWith(0, null, 40) - @TrackChangesManager.getLoadManagerBucket = sinon.stub().returns(30) - - describe "pushing the op", -> - beforeEach -> - @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 1) - @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback - - it "should push the op into redis", -> - @RedisManager.pushUncompressedHistoryOp - .calledWith(@doc_id, @op) - .should.equal true - - it "should call the callback", -> - @callback.called.should.equal true - - it "should not try to flush the op", -> - @TrackChangesManager.flushDocChanges.called.should.equal false - - describe "when there are a multiple of FLUSH_EVERY_N_OPS ops", -> - beforeEach -> - @RedisManager.pushUncompressedHistoryOp = - sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) - @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback - - it "should tell the track changes api to flush", -> - @TrackChangesManager.flushDocChanges - .calledWith(@project_id, @doc_id) - .should.equal true - - describe "when TrackChangesManager errors", -> - beforeEach -> - @RedisManager.pushUncompressedHistoryOp = - sinon.stub().callsArgWith(2, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) - @TrackChangesManager.flushDocChanges = sinon.stub().callsArgWith(2, @error = new Error("oops")) - @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback - - it "should log out the error", -> - @logger.error - .calledWith( - err: @error - doc_id: @doc_id - project_id: @project_id - "error flushing doc to track changes api" - ) - .should.equal true - - describe "when the doc is over the load manager threshold", -> - beforeEach -> - @RedisManager.getHistoryLoadManagerThreshold = sinon.stub().callsArgWith(0, null, 40) - @TrackChangesManager.getLoadManagerBucket = sinon.stub().returns(50) - @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(2, null, 1) + @RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArgWith(3, null, 1) @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback - it "should not push the op", -> - @RedisManager.pushUncompressedHistoryOp.called.should.equal false - - it "should not try to flush the op", -> - @TrackChangesManager.flushDocChanges.called.should.equal false + it "should push the op into redis", -> + @RedisManager.pushUncompressedHistoryOp + .calledWith(@project_id, @doc_id, @op) + .should.equal true it "should call the callback", -> @callback.called.should.equal true + it "should not try to flush the op", -> + @TrackChangesManager.flushDocChanges.called.should.equal false + + describe "when there are a multiple of FLUSH_EVERY_N_OPS ops", -> + beforeEach -> + @RedisManager.pushUncompressedHistoryOp = + sinon.stub().callsArgWith(3, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) + @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback + + it "should tell the track changes api to flush", -> + @TrackChangesManager.flushDocChanges + .calledWith(@project_id, @doc_id) + .should.equal true + + describe "when TrackChangesManager errors", -> + beforeEach -> + @RedisManager.pushUncompressedHistoryOp = + sinon.stub().callsArgWith(3, null, 2 * @TrackChangesManager.FLUSH_EVERY_N_OPS) + @TrackChangesManager.flushDocChanges = sinon.stub().callsArgWith(2, @error = new Error("oops")) + @TrackChangesManager.pushUncompressedHistoryOp @project_id, @doc_id, @op, @callback + + it "should log out the error", -> + @logger.error + .calledWith( + err: @error + doc_id: @doc_id + project_id: @project_id + "error flushing doc to track changes api" + ) + .should.equal true + From 375427bf5eb1cfbe78a8eeb968379052a721f2c4 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 21 Mar 2014 13:15:42 +0000 Subject: [PATCH 12/16] Remove extraneous logging --- services/document-updater/app/coffee/TrackChangesManager.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/services/document-updater/app/coffee/TrackChangesManager.coffee b/services/document-updater/app/coffee/TrackChangesManager.coffee index 2d887a4f9c..90cba86b36 100644 --- a/services/document-updater/app/coffee/TrackChangesManager.coffee +++ b/services/document-updater/app/coffee/TrackChangesManager.coffee @@ -23,7 +23,6 @@ module.exports = TrackChangesManager = FLUSH_EVERY_N_OPS: 50 pushUncompressedHistoryOp: (project_id, doc_id, op, callback = (error) ->) -> - logger.log project_id: project_id, doc_id: doc_id, "pushing history op" RedisManager.pushUncompressedHistoryOp project_id, doc_id, op, (error, length) -> return callback(error) if error? if length > 0 and length % TrackChangesManager.FLUSH_EVERY_N_OPS == 0 From c2ebaaa338b3d546bf422ddb37058f3f505f85ef Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 10 Apr 2014 12:44:46 +0100 Subject: [PATCH 13/16] Split lines on Windows line endings too --- .../app/coffee/ShareJsUpdateManager.coffee | 5 +- .../coffee/ShareJsUpdateManagerTests.coffee | 151 +++++++----------- 2 files changed, 57 insertions(+), 99 deletions(-) diff --git a/services/document-updater/app/coffee/ShareJsUpdateManager.coffee b/services/document-updater/app/coffee/ShareJsUpdateManager.coffee index 9cde95492b..5f3cba4fbc 100644 --- a/services/document-updater/app/coffee/ShareJsUpdateManager.coffee +++ b/services/document-updater/app/coffee/ShareJsUpdateManager.coffee @@ -44,10 +44,7 @@ module.exports = ShareJsUpdateManager = if error? @_sendError(project_id, doc_id, error) return callback(error) - if typeof data.snapshot == "string" - docLines = data.snapshot.split("\n") - else - docLines = data.snapshot.lines + docLines = data.snapshot.split(/\r\n|\n|\r/) callback(null, docLines, data.v) _listenForOps: (model) -> diff --git a/services/document-updater/test/unit/coffee/ShareJsUpdateManagerTests.coffee b/services/document-updater/test/unit/coffee/ShareJsUpdateManagerTests.coffee index af5a475836..20e737fc97 100644 --- a/services/document-updater/test/unit/coffee/ShareJsUpdateManagerTests.coffee +++ b/services/document-updater/test/unit/coffee/ShareJsUpdateManagerTests.coffee @@ -29,113 +29,74 @@ describe "ShareJsUpdateManager", -> @ShareJsUpdateManager.getNewShareJsModel = sinon.stub().returns(@model) @ShareJsUpdateManager._listenForOps = sinon.stub() @ShareJsUpdateManager.removeDocFromCache = sinon.stub().callsArg(1) + @updates = [ + {p: 4, t: "foo"} + {p: 6, t: "bar"} + ] + @updatedDocLines = ["one", "two"] - describe "with a text document", -> - beforeEach -> - @updates = [ - {p: 4, t: "foo"} - {p: 6, t: "bar"} - ] - @updatedDocLines = ["one", "two"] + describe "successfully", -> + beforeEach (done) -> + @model.getSnapshot.callsArgWith(1, null, {snapshot: @updatedDocLines.join("\n"), v: @version}) + @ShareJsUpdateManager.applyUpdates @project_id, @doc_id, @updates, (err, docLines, version) => + @callback(err, docLines, version) + done() - describe "successfully", -> - beforeEach (done) -> - @model.getSnapshot.callsArgWith(1, null, {snapshot: @updatedDocLines.join("\n"), v: @version}) - @ShareJsUpdateManager.applyUpdates @project_id, @doc_id, @updates, (err, docLines, version) => - @callback(err, docLines, version) - done() + it "should create a new ShareJs model", -> + @ShareJsUpdateManager.getNewShareJsModel + .called.should.equal true - it "should create a new ShareJs model", -> - @ShareJsUpdateManager.getNewShareJsModel - .called.should.equal true + it "should listen for ops on the model", -> + @ShareJsUpdateManager._listenForOps + .calledWith(@model) + .should.equal true - it "should listen for ops on the model", -> - @ShareJsUpdateManager._listenForOps - .calledWith(@model) - .should.equal true + it "should send each update to ShareJs", -> + for update in @updates + @model.applyOp + .calledWith("#{@project_id}:#{@doc_id}", update).should.equal true - it "should send each update to ShareJs", -> - for update in @updates - @model.applyOp - .calledWith("#{@project_id}:#{@doc_id}", update).should.equal true + it "should get the updated doc lines", -> + @model.getSnapshot + .calledWith("#{@project_id}:#{@doc_id}") + .should.equal true - it "should get the updated doc lines", -> - @model.getSnapshot - .calledWith("#{@project_id}:#{@doc_id}") - .should.equal true + it "should return the updated doc lines", -> + @callback.calledWith(null, @updatedDocLines, @version).should.equal true - it "should return the updated doc lines", -> - @callback.calledWith(null, @updatedDocLines, @version).should.equal true + describe "when applyOp fails", -> + beforeEach (done) -> + @error = new Error("Something went wrong") + @ShareJsUpdateManager._sendError = sinon.stub() + @model.applyOp = sinon.stub().callsArgWith(2, @error) + @ShareJsUpdateManager.applyUpdates @project_id, @doc_id, @updates, (err, docLines, version) => + @callback(err, docLines, version) + done() - describe "when applyOp fails", -> - beforeEach (done) -> - @error = new Error("Something went wrong") - @ShareJsUpdateManager._sendError = sinon.stub() - @model.applyOp = sinon.stub().callsArgWith(2, @error) - @ShareJsUpdateManager.applyUpdates @project_id, @doc_id, @updates, (err, docLines, version) => - @callback(err, docLines, version) - done() + it "should call sendError with the error", -> + @ShareJsUpdateManager._sendError + .calledWith(@project_id, @doc_id, @error) + .should.equal true - it "should call sendError with the error", -> - @ShareJsUpdateManager._sendError - .calledWith(@project_id, @doc_id, @error) - .should.equal true + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true - it "should call the callback with the error", -> - @callback.calledWith(@error).should.equal true + describe "when getSnapshot fails", -> + beforeEach (done) -> + @error = new Error("Something went wrong") + @ShareJsUpdateManager._sendError = sinon.stub() + @model.getSnapshot.callsArgWith(1, @error) + @ShareJsUpdateManager.applyUpdates @project_id, @doc_id, @updates, (err, docLines, version) => + @callback(err, docLines, version) + done() - describe "when getSnapshot fails", -> - beforeEach (done) -> - @error = new Error("Something went wrong") - @ShareJsUpdateManager._sendError = sinon.stub() - @model.getSnapshot.callsArgWith(1, @error) - @ShareJsUpdateManager.applyUpdates @project_id, @doc_id, @updates, (err, docLines, version) => - @callback(err, docLines, version) - done() + it "should call sendError with the error", -> + @ShareJsUpdateManager._sendError + .calledWith(@project_id, @doc_id, @error) + .should.equal true - it "should call sendError with the error", -> - @ShareJsUpdateManager._sendError - .calledWith(@project_id, @doc_id, @error) - .should.equal true - - it "should call the callback with the error", -> - @callback.calledWith(@error).should.equal true - - describe "with a JSON document", -> - beforeEach -> - @updates = [ - {p: ["lines", 0], dl: { foo: "bar "}} - ] - @docLines = [text: "one", text: "two"] - - describe "successfully", -> - beforeEach (done) -> - @model.getSnapshot.callsArgWith(1, null, {snapshot: {lines: @docLines}, v: @version}) - @ShareJsUpdateManager.applyUpdates @project_id, @doc_id, @updates, (err, docLines, version) => - @callback(err, docLines, version) - done() - - it "should create a new ShareJs model", -> - @ShareJsUpdateManager.getNewShareJsModel - .called.should.equal true - - it "should listen for ops on the model", -> - @ShareJsUpdateManager._listenForOps - .calledWith(@model) - .should.equal true - - it "should send each update to ShareJs", -> - for update in @updates - @model.applyOp - .calledWith("#{@project_id}:#{@doc_id}", update).should.equal true - - it "should get the updated doc lines", -> - @model.getSnapshot - .calledWith("#{@project_id}:#{@doc_id}") - .should.equal true - - it "should return the updated doc lines", -> - @callback.calledWith(null, @docLines, @version).should.equal true + it "should call the callback with the error", -> + @callback.calledWith(@error).should.equal true describe "_listenForOps", -> beforeEach -> From d1434f764614223fcb03958442882a2604ad64de Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 7 May 2014 09:48:29 +0100 Subject: [PATCH 14/16] Increase redis lock expiry time to 30 seconds --- services/document-updater/app/coffee/LockManager.coffee | 4 ++-- .../test/unit/coffee/LockManager/tryLockTests.coffee | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/document-updater/app/coffee/LockManager.coffee b/services/document-updater/app/coffee/LockManager.coffee index 97c06ad721..a43bd84a1b 100644 --- a/services/document-updater/app/coffee/LockManager.coffee +++ b/services/document-updater/app/coffee/LockManager.coffee @@ -10,10 +10,10 @@ logger = require "logger-sharelatex" 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 + REDIS_LOCK_EXPIRY: 30 # seconds. Time until lock auto expires in redis. tryLock : (doc_id, callback = (err, isFree)->)-> - tenSeconds = 10 - rclient.set keys.blockingKey(doc_id: doc_id), "locked", "EX", 10, "NX", (err, gotLock)-> + rclient.set keys.blockingKey(doc_id: doc_id), "locked", "EX", LockManager.REDIS_LOCK_EXPIRY, "NX", (err, gotLock)-> return callback(err) if err? if gotLock == "OK" metrics.inc "doc-not-blocking" diff --git a/services/document-updater/test/unit/coffee/LockManager/tryLockTests.coffee b/services/document-updater/test/unit/coffee/LockManager/tryLockTests.coffee index cff2b9538b..6c2c8972af 100644 --- a/services/document-updater/test/unit/coffee/LockManager/tryLockTests.coffee +++ b/services/document-updater/test/unit/coffee/LockManager/tryLockTests.coffee @@ -21,7 +21,7 @@ describe 'LockManager - trying the lock', -> @LockManager.tryLock @doc_id, @callback it "should set the lock key with an expiry if it is not set", -> - @set.calledWith("Blocking:#{@doc_id}", "locked", "EX", 10, "NX") + @set.calledWith("Blocking:#{@doc_id}", "locked", "EX", 30, "NX") .should.equal true it "should return the callback with true", -> From f511ebd4b63069ca7b1a15151df91ea9b13cbb47 Mon Sep 17 00:00:00 2001 From: James Allen Date: Wed, 7 May 2014 10:05:07 +0100 Subject: [PATCH 15/16] Exit cleanly on SIGINT et al --- services/document-updater/app.coffee | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index 7168017790..ddc438f752 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -22,10 +22,13 @@ app.configure -> app.use app.router rclient.subscribe("pending-updates") -rclient.on "message", (channel, doc_key)-> +rclient.on "message", (channel, doc_key) -> [project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key) - UpdateManager.processOutstandingUpdatesWithLock project_id, doc_id, (error) -> - logger.error err: error, project_id: project_id, doc_id: doc_id, "error processing update" if error? + if !Settings.shuttingDown + UpdateManager.processOutstandingUpdatesWithLock project_id, doc_id, (error) -> + logger.error err: error, project_id: project_id, doc_id: doc_id, "error processing update" if error? + else + logger.log project_id: project_id, doc_id: doc_id, "ignoring incoming update" UpdateManager.resumeProcessing() @@ -47,7 +50,10 @@ app.get '/total', (req, res)-> res.send {total:count} app.get '/status', (req, res)-> - res.send('document updater is alive') + if Settings.shuttingDown + res.send 503 # Service unavailable + else + res.send('document updater is alive') app.use (error, req, res, next) -> logger.error err: error, "request errored" @@ -56,6 +62,19 @@ app.use (error, req, res, next) -> else res.send(500, "Oops, something went wrong") +shutdownCleanly = (signal) -> + return () -> + logger.log signal: signal, "received interrupt, cleaning up" + Settings.shuttingDown = true + setTimeout () -> + logger.log signal: signal, "shutting down" + process.exit() + , 10000 + + port = Settings.internal?.documentupdater?.port or Settings.apis?.documentupdater?.port or 3003 app.listen port, "localhost", -> logger.log("documentupdater-sharelatex server listening on port #{port}") + +for signal in ['SIGINT', 'SIGHUP', 'SIGQUIT', 'SIGUSR1', 'SIGUSR2', 'SIGTERM', 'SIGABRT'] + process.on signal, shutdownCleanly(signal) \ No newline at end of file From 6011ce47837fc086dfa8d7f38889c195a37f1a56 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 8 May 2014 09:28:13 +0100 Subject: [PATCH 16/16] Use new metrics module --- services/document-updater/app.coffee | 15 ++++++------ .../app/coffee/Metrics.coffee | 24 +------------------ services/document-updater/package.json | 1 + 3 files changed, 9 insertions(+), 31 deletions(-) diff --git a/services/document-updater/app.coffee b/services/document-updater/app.coffee index ddc438f752..a52b26f500 100644 --- a/services/document-updater/app.coffee +++ b/services/document-updater/app.coffee @@ -7,7 +7,6 @@ RedisManager = require('./app/js/RedisManager.js') UpdateManager = require('./app/js/UpdateManager.js') Keys = require('./app/js/RedisKeyBuilder') redis = require('redis') -metrics = require('./app/js/Metrics') Errors = require "./app/js/Errors" HttpController = require "./app/js/HttpController" @@ -15,9 +14,14 @@ redisConf = Settings.redis.web rclient = redis.createClient(redisConf.port, redisConf.host) rclient.auth(redisConf.password) +Path = require "path" +Metrics = require "metrics-sharelatex" +Metrics.initialize("doc-updater") +Metrics.mongodb.monitor(Path.resolve(__dirname + "/node_modules/mongojs/node_modules/mongodb"), logger) + app = express() app.configure -> - app.use(express.logger(':remote-addr - [:date] - :user-agent ":method :url" :status - :response-time ms')); + app.use(Metrics.http.monitor(logger)); app.use express.bodyParser() app.use app.router @@ -32,10 +36,6 @@ rclient.on "message", (channel, doc_key) -> UpdateManager.resumeProcessing() -app.use (req, res, next)-> - metrics.inc "http-request" - next() - app.get '/project/:project_id/doc/:doc_id', HttpController.getDoc app.post '/project/:project_id/doc/:doc_id', HttpController.setDoc app.post '/project/:project_id/doc/:doc_id/flush', HttpController.flushDocIfLoaded @@ -44,7 +44,7 @@ app.delete '/project/:project_id', HttpController.deleteProjec app.post '/project/:project_id/flush', HttpController.flushProject app.get '/total', (req, res)-> - timer = new metrics.Timer("http.allDocList") + timer = new Metrics.Timer("http.allDocList") RedisManager.getCountOfDocsInMemory (err, count)-> timer.done() res.send {total:count} @@ -71,7 +71,6 @@ shutdownCleanly = (signal) -> process.exit() , 10000 - port = Settings.internal?.documentupdater?.port or Settings.apis?.documentupdater?.port or 3003 app.listen port, "localhost", -> logger.log("documentupdater-sharelatex server listening on port #{port}") diff --git a/services/document-updater/app/coffee/Metrics.coffee b/services/document-updater/app/coffee/Metrics.coffee index 0b98550c0e..4bf5c6dba5 100644 --- a/services/document-updater/app/coffee/Metrics.coffee +++ b/services/document-updater/app/coffee/Metrics.coffee @@ -1,23 +1 @@ -StatsD = require('lynx') -statsd = new StatsD('localhost', 8125, {on_error:->}) - -buildKey = (key)-> "doc-updater.#{process.env.NODE_ENV}.#{key}" - -module.exports = - set : (key, value, sampleRate = 1)-> - statsd.set buildKey(key), value, sampleRate - - inc : (key, sampleRate = 1)-> - statsd.increment buildKey(key), sampleRate - - Timer : class - constructor :(key, sampleRate = 1)-> - this.start = new Date() - this.key = buildKey(key) - done:-> - timeSpan = new Date - this.start - statsd.timing(this.key, timeSpan, this.sampleRate) - - gauge : (key, value, sampleRate = 1)-> - statsd.gauge key, value, sampleRate - +module.exports = require "metrics-sharelatex" \ No newline at end of file diff --git a/services/document-updater/package.json b/services/document-updater/package.json index fbcad4abc1..25feab87d8 100644 --- a/services/document-updater/package.json +++ b/services/document-updater/package.json @@ -14,6 +14,7 @@ "coffee-script": "1.4.0", "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#master", "logger-sharelatex": "git+https://github.com/sharelatex/logger-sharelatex.git#master", + "metrics-sharelatex": "git+https://github.com/sharelatex/metrics-sharelatex.git#master", "sinon": "~1.5.2", "mongojs": "0.9.11" },