From 05a048db9ab6bfb2beed184e8708cd2b4eff1d0e Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 22 Sep 2016 11:13:26 +0100 Subject: [PATCH 1/2] Add in a little more logging --- services/track-changes/app/coffee/DiffGenerator.coffee | 2 +- services/track-changes/app/coffee/DiffManager.coffee | 2 ++ services/track-changes/app/coffee/DocumentUpdaterManager.coffee | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/services/track-changes/app/coffee/DiffGenerator.coffee b/services/track-changes/app/coffee/DiffGenerator.coffee index b4e0156f02..c9c8e1d4bd 100644 --- a/services/track-changes/app/coffee/DiffGenerator.coffee +++ b/services/track-changes/app/coffee/DiffGenerator.coffee @@ -18,7 +18,7 @@ module.exports = DiffGenerator = if e instanceof ConsistencyError and i = update.op.length - 1 # catch known case where the last op in an array has been # merged into a later op - logger.error {update, op: JSON.stringify(op)}, "marking op as broken" + logger.error {err: e, update, op: JSON.stringify(op)}, "marking op as broken" op.broken = true else throw e # rethrow the execption diff --git a/services/track-changes/app/coffee/DiffManager.coffee b/services/track-changes/app/coffee/DiffManager.coffee index dfdb1a80a2..71692b9d83 100644 --- a/services/track-changes/app/coffee/DiffManager.coffee +++ b/services/track-changes/app/coffee/DiffManager.coffee @@ -49,6 +49,8 @@ module.exports = DiffManager = lastUpdate = updates[0] if lastUpdate? and lastUpdate.v != version - 1 return callback new Error("latest update version, #{lastUpdate.v}, does not match doc version, #{version}") + + logger.log {docVersion: version, lastUpdateVersion: lastUpdate?.v, updateCount: updates.length}, "rewinding updates" tryUpdates = updates.slice().reverse() diff --git a/services/track-changes/app/coffee/DocumentUpdaterManager.coffee b/services/track-changes/app/coffee/DocumentUpdaterManager.coffee index edc2f11c68..625622206e 100644 --- a/services/track-changes/app/coffee/DocumentUpdaterManager.coffee +++ b/services/track-changes/app/coffee/DocumentUpdaterManager.coffee @@ -14,6 +14,7 @@ module.exports = DocumentUpdaterManager = body = JSON.parse(body) catch error return callback(error) + logger.log {project_id, doc_id, version: body.version}, "got doc from document updater" callback null, body.lines.join("\n"), body.version else error = new Error("doc updater returned a non-success status code: #{res.statusCode}") From cce93b95a544d45fe6f98f1eb2233fe6c8b7a701 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 22 Sep 2016 11:18:10 +0100 Subject: [PATCH 2/2] Fetch updates before doc content when building a diff --- services/track-changes/app/coffee/DiffManager.coffee | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/services/track-changes/app/coffee/DiffManager.coffee b/services/track-changes/app/coffee/DiffManager.coffee index 71692b9d83..94e2f0adf6 100644 --- a/services/track-changes/app/coffee/DiffManager.coffee +++ b/services/track-changes/app/coffee/DiffManager.coffee @@ -5,11 +5,12 @@ logger = require "logger-sharelatex" module.exports = DiffManager = getLatestDocAndUpdates: (project_id, doc_id, fromVersion, toVersion, callback = (error, content, version, updates) ->) -> - # retrieve the document before retreiving the updates, - # because updates are written to mongo after the document - DocumentUpdaterManager.getDocument project_id, doc_id, (error, content, version) -> + # Whichever order these are in, there is potential for updates to come in between + # them so that they do not return the same 'latest' versions. + # TODO: If these don't return consistent content, try again. + UpdatesManager.getDocUpdatesWithUserInfo project_id, doc_id, from: fromVersion, to: toVersion, (error, updates) -> return callback(error) if error? - UpdatesManager.getDocUpdatesWithUserInfo project_id, doc_id, from: fromVersion, to: toVersion, (error, updates) -> + DocumentUpdaterManager.getDocument project_id, doc_id, (error, content, version) -> return callback(error) if error? callback(null, content, version, updates)