From dbee4a57fb6fe45a3a6b51dd0b48307d75087391 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 23 Aug 2013 14:35:13 +0100 Subject: [PATCH] Compress doc ops in two passes --- services/track-changes/app.coffee | 19 +++++ .../app/coffee/ConcatManager.coffee | 51 ++++++++++- .../app/coffee/ConversionManager.coffee | 34 ++++---- services/track-changes/package.json | 4 +- .../ConcatManager/ConcatManagerTests.coffee | 84 ++++++++++++------- 5 files changed, 145 insertions(+), 47 deletions(-) create mode 100644 services/track-changes/app.coffee diff --git a/services/track-changes/app.coffee b/services/track-changes/app.coffee new file mode 100644 index 0000000000..7398ca8252 --- /dev/null +++ b/services/track-changes/app.coffee @@ -0,0 +1,19 @@ +express = require "express" +app = express() + +ConversoinManager = require "./app/js/ConversionManager" +logger = require "logger-sharelatex" +logger.initialize("history") + +app.post "/doc/:doc_id/flush", (req, res, next) -> + project_id = req.params.project_id + logger.log doc_id: doc_id, "compressing doc history" + ConversionManager.convertOldRawUpdates doc_id, (error) -> + return next(error) if error? + res.send 204 # No content + +app.use (error, req, res, next) -> + logger.error err: error, "an internal error occured" + req.send 500 + +app.listen(3014) diff --git a/services/track-changes/app/coffee/ConcatManager.coffee b/services/track-changes/app/coffee/ConcatManager.coffee index 622616b1cb..67fc387ab8 100644 --- a/services/track-changes/app/coffee/ConcatManager.coffee +++ b/services/track-changes/app/coffee/ConcatManager.coffee @@ -13,9 +13,30 @@ module.exports = ConcatManager = user_id: update.meta.user_id return updates + compressUpdates: (rawUpdates) -> + return [] if rawUpdates.length == 0 + firstPass = [rawUpdates.shift()] + for update in rawUpdates + lastCompressedUpdate = firstPass.pop() + if lastCompressedUpdate? + firstPass = firstPass.concat ConcatManager._concatTwoUpdatesOfTheSameType lastCompressedUpdate, update + else + firstPass.push rawUpdate + + return [] if firstPass.length == 0 + secondPass = [firstPass.shift()] + for update in firstPass + lastCompressedUpdate = secondPass.pop() + if lastCompressedUpdate? + secondPass = secondPass.concat ConcatManager._cancelOppositeInsertsAndDeletes lastCompressedUpdate, update + else + secondPass.push update + + return secondPass + MAX_TIME_BETWEEN_UPDATES: oneMinute = 60 * 1000 - concatTwoUpdates: (firstUpdate, secondUpdate) -> + _concatTwoUpdatesOfTheSameType: (firstUpdate, secondUpdate) -> firstUpdate = op: firstUpdate.op meta: @@ -61,8 +82,34 @@ module.exports = ConcatManager = d: strInject(secondOp.d, firstOp.p - secondOp.p, firstOp.d) ] ] + else + return [firstUpdate, secondUpdate] + + _cancelOppositeInsertsAndDeletes: (firstUpdate, secondUpdate) -> + firstUpdate = + op: firstUpdate.op + meta: + user_id: firstUpdate.meta.user_id or null + start_ts: firstUpdate.meta.start_ts or firstUpdate.meta.ts + end_ts: firstUpdate.meta.end_ts or firstUpdate.meta.ts + secondUpdate = + op: secondUpdate.op + meta: + user_id: secondUpdate.meta.user_id or null + start_ts: secondUpdate.meta.start_ts or secondUpdate.meta.ts + end_ts: secondUpdate.meta.end_ts or secondUpdate.meta.ts + + if firstUpdate.meta.user_id != secondUpdate.meta.user_id + return [firstUpdate, secondUpdate] + + if secondUpdate.meta.start_ts - firstUpdate.meta.end_ts > ConcatManager.MAX_TIME_BETWEEN_UPDATES + return [firstUpdate, secondUpdate] + + firstOp = firstUpdate.op[0] + secondOp = secondUpdate.op[0] + # An insert and then a delete - else if firstOp.i? and secondOp.d? and firstOp.p <= secondOp.p <= (firstOp.p + firstOp.i.length) + if firstOp.i? and secondOp.d? and firstOp.p <= secondOp.p <= (firstOp.p + firstOp.i.length) offset = secondOp.p - firstOp.p insertedText = firstOp.i.slice(offset, offset + secondOp.d.length) if insertedText == secondOp.d diff --git a/services/track-changes/app/coffee/ConversionManager.coffee b/services/track-changes/app/coffee/ConversionManager.coffee index 8ffcb54dfd..188339d150 100644 --- a/services/track-changes/app/coffee/ConversionManager.coffee +++ b/services/track-changes/app/coffee/ConversionManager.coffee @@ -1,8 +1,9 @@ {db, ObjectId} = require "./mongojs" ConcatManager = require "./ConcatManager" +logger = require "logger-sharelatex" module.exports = ConversionManager = - OPS_TO_LEAVE: 10 + OPS_TO_LEAVE: 100 popLatestCompressedUpdate: (doc_id, callback = (error, update) ->) -> db.docHistory.findAndModify @@ -25,12 +26,19 @@ module.exports = ConversionManager = tailVersion = doc.tailVersion or 0 if currentVersion - tailVersion > ConversionManager.OPS_TO_LEAVE db.docOps.findAndModify - query: { doc_id: ObjectId(doc_id), version: currentVersion } - update: { - $push: { docOps: { $each: [], $slice: - ConversionManager.OPS_TO_LEAVE } } - $set: tailVersion: currentVersion - ConversionManager.OPS_TO_LEAVE, - } - fields: { docOps: $slice: currentVersion - tailVersion - ConversionManager.OPS_TO_LEAVE } + query: + doc_id: ObjectId(doc_id) + version: currentVersion + update: + $push: + docOps: + $each: [] + $slice: - ConversionManager.OPS_TO_LEAVE + $set: + tailVersion: currentVersion - ConversionManager.OPS_TO_LEAVE + fields: + docOps: + $slice: currentVersion - tailVersion - ConversionManager.OPS_TO_LEAVE , (error, doc) -> return callback(error) if error? if !doc? @@ -63,15 +71,11 @@ module.exports = ConversionManager = # No saved versions, no raw updates, nothing to do return callback() - compressedUpdates = [lastCompressedUpdate] - for rawUpdate in rawUpdates - lastCompressedUpdate = compressedUpdates.pop() - if lastCompressedUpdate? - compressedUpdates = compressedUpdates.concat ConcatManager.concatTwoUpdates lastCompressedUpdate, rawUpdate - else - compressedUpdates.push rawUpdate + uncompressedUpdates = [lastCompressedUpdate].concat rawUpdates + compressedUpdates = ConcatManager.compressUpdates uncompressedUpdates + ConversionManager.insertCompressedUpdates doc_id, compressedUpdates, (error) -> return callback(error) if error? - console.log doc_id, "Pushed doc ops", length + logger.log doc_id: doc_id, rawOpsLength: length, compressedOpsLength: compressedUpdates.length, "compressed doc ops" callback() diff --git a/services/track-changes/package.json b/services/track-changes/package.json index 5698e2dd3b..e1a8dba4a6 100644 --- a/services/track-changes/package.json +++ b/services/track-changes/package.json @@ -4,9 +4,11 @@ "dependencies": { "async": "", "chai": "", + "express": "3.3.5", "sandboxed-module": "", "sinon": "", "mongojs": "0.7.2", - "settings": "git+ssh://git@bitbucket.org:sharelatex/settings-sharelatex.git#master" + "settings": "git+ssh://git@bitbucket.org:sharelatex/settings-sharelatex.git#master", + "logger": "git+ssh://git@bitbucket.org:sharelatex/logger-sharelatex.git#bunyan" } } diff --git a/services/track-changes/test/unit/coffee/ConcatManager/ConcatManagerTests.coffee b/services/track-changes/test/unit/coffee/ConcatManager/ConcatManagerTests.coffee index 620813c462..93909bc99c 100644 --- a/services/track-changes/test/unit/coffee/ConcatManager/ConcatManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/ConcatManager/ConcatManagerTests.coffee @@ -13,42 +13,42 @@ describe "ConcatManager", -> @ts1 = Date.now() @ts2 = Date.now() + 1000 - describe "concatTwoUpdates", -> + describe "compress", -> describe "insert - insert", -> it "should append one insert to the other", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 6, i: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, i: "foobar" ] meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id }] it "should insert one insert inside the other", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 5, i: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, i: "fobaro" ] meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id }] it "should not append separated inserts", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 9, i: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, i: "foo" ] meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id @@ -59,39 +59,39 @@ describe "ConcatManager", -> describe "delete - delete", -> it "should append one delete to the other", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 3, d: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, d: "foobar" ] meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id }] it "should insert one delete inside the other", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 1, d: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 1, d: "bafoor" ] meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id }] it "should not append separated deletes", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 9, d: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, d: "foo" ] meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id @@ -102,49 +102,49 @@ describe "ConcatManager", -> describe "insert - delete", -> it "should undo a previous insert", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 5, d: "o" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, i: "fo" ] meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id }] it "should remove part of an insert from the middle", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, i: "fobaro" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 5, d: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, i: "foo" ] meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id }] it "should cancel out two opposite updates", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 3, d: "foo" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [] it "should not combine separated updates", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, i: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 9, d: "bar" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, i: "foo" ] meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id @@ -155,26 +155,26 @@ describe "ConcatManager", -> describe "delete - insert", -> it "should redo a previous delete at the beginning", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 3, i: "f" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 4, d: "oo" ] meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id }] it "should redo a previous delete from halfway through", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, d: "foobar" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 3, i: "oo" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, d: "f" ] meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id @@ -183,14 +183,40 @@ describe "ConcatManager", -> meta: start_ts: @ts2, end_ts: @ts2, user_id: @user_id }] + it "should keep words together", -> + expect(@ConcatManager.compressUpdates [{ + op: [ p: 3, d: "abcdefghijklmnopqrstuvwxyz hello world" ] + meta: ts: @ts1, user_id: @user_id + }, { + op: [ p: 3, i: "w" ] + meta: ts: @ts2, user_id: @user_id + }, { + op: [ p: 4, i: "o" ] + meta: ts: @ts2, user_id: @user_id + }, { + op: [ p: 5, i: "r" ] + meta: ts: @ts2, user_id: @user_id + }, { + op: [ p: 6, i: "l" ] + meta: ts: @ts2, user_id: @user_id + }, { + op: [ p: 7, i: "d" ] + meta: ts: @ts2, user_id: @user_id + }]) + .to.deep.equal [{ + op: [ p: 3, d: "abcdefghijklmnopqrstuvwxyz hello " ] + meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id + }] + + it "should not combine the ops if the insert text does not match the delete text", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, d: "foobar" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 3, i: "xy" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal [{ op: [ p: 3, d: "foobar" ] meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id @@ -200,13 +226,13 @@ describe "ConcatManager", -> }] it "should cancel two equal updates", -> - expect(@ConcatManager.concatTwoUpdates({ + expect(@ConcatManager.compressUpdates [{ op: [ p: 3, d: "foo" ] meta: ts: @ts1, user_id: @user_id }, { op: [ p: 3, i: "foo" ] meta: ts: @ts2, user_id: @user_id - })) + }]) .to.deep.equal []