reduce the number of times we flush project history

This commit is contained in:
Hayden Faulds 2018-02-22 10:01:05 +00:00
parent 3d5740fd7d
commit ea0dd9700b
8 changed files with 64 additions and 31 deletions

View file

@ -89,6 +89,13 @@ module.exports = DocumentManager =
callback null
else
DocumentManager.flushAndDeleteDoc project_id, doc_id, (error) ->
# Flush in the background since it requires a http request. We
# want to flush project history if the previous call only failed
# to delete the doc from Redis. There is no harm in flushing
# project history if the previous call failed to flush at all. So
# do this before checking errors.
HistoryManager.flushProjectChangesAsync project_id
return callback(error) if error?
callback null
@ -118,7 +125,7 @@ module.exports = DocumentManager =
return callback(error) if error?
# Flush in the background since it requires a http request
HistoryManager.flushChangesAsync project_id, doc_id
HistoryManager.flushDocChangesAsync project_id, doc_id
RedisManager.removeDocFromMemory project_id, doc_id, (error) ->
return callback(error) if error?

View file

@ -4,10 +4,8 @@ logger = require "logger-sharelatex"
HistoryRedisManager = require "./HistoryRedisManager"
module.exports = HistoryManager =
flushChangesAsync: (project_id, doc_id) ->
flushDocChangesAsync: (project_id, doc_id) ->
HistoryManager._flushDocChangesAsync project_id, doc_id
if Settings.apis?.project_history?.enabled
HistoryManager.flushProjectChangesAsync project_id
_flushDocChangesAsync: (project_id, doc_id) ->
if !Settings.apis?.trackchanges?
@ -23,7 +21,7 @@ module.exports = HistoryManager =
logger.error { doc_id, project_id }, "track changes api returned a failure status code: #{res.statusCode}"
flushProjectChangesAsync: (project_id) ->
return if !Settings.apis?.project_history?
return if !Settings.apis?.project_history?.enabled
url = "#{Settings.apis.project_history.url}/project/#{project_id}/flush"
logger.log { project_id, url }, "flushing doc in project history api"

View file

@ -1,4 +1,5 @@
DocumentManager = require "./DocumentManager"
HistoryManager = require "./HistoryManager"
ProjectManager = require "./ProjectManager"
Errors = require "./Errors"
logger = require "logger-sharelatex"
@ -106,6 +107,13 @@ module.exports = HttpController =
timer = new Metrics.Timer("http.deleteDoc")
DocumentManager.flushAndDeleteDocWithLock project_id, doc_id, (error) ->
timer.done()
# Flush in the background since it requires a http request. We
# want to flush project history if the previous call only failed
# to delete the doc from Redis. There is no harm in flushing
# project history if the previous call failed to flush at all. So
# do this before checking errors.
HistoryManager.flushProjectChangesAsync project_id
return next(error) if error?
logger.log project_id: project_id, doc_id: doc_id, "deleted doc via http"
res.send 204 # No Content

View file

@ -59,6 +59,12 @@ module.exports = ProjectManager =
logger.log project_id: project_id, doc_ids: doc_ids, "deleting docs"
async.series jobs, () ->
# Flush in the background since it requires a htpt request. If we
# flushed and deleted only some docs successfully then we should still
# flush project history. If no docs succeeded then there is still no
# harm flushing project history. So do this before checking errors.
HistoryManager.flushProjectChangesAsync project_id
if errors.length > 0
callback new Error("Errors deleting docs. See log for details")
else

View file

@ -11,7 +11,9 @@ describe "DocumentManager", ->
@DocumentManager = SandboxedModule.require modulePath, requires:
"./RedisManager": @RedisManager = {}
"./PersistenceManager": @PersistenceManager = {}
"./HistoryManager": @HistoryManager = {}
"./HistoryManager": @HistoryManager =
flushDocChangesAsync: sinon.stub()
flushProjectChangesAsync: sinon.stub()
"logger-sharelatex": @logger = {log: sinon.stub()}
"./DocOpsManager": @DocOpsManager = {}
"./Metrics": @Metrics =
@ -36,7 +38,6 @@ describe "DocumentManager", ->
beforeEach ->
@RedisManager.removeDocFromMemory = sinon.stub().callsArg(2)
@DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2)
@HistoryManager.flushChangesAsync = sinon.stub()
@DocumentManager.flushAndDeleteDoc @project_id, @doc_id, @callback
it "should flush the doc", ->
@ -56,8 +57,8 @@ describe "DocumentManager", ->
@Metrics.Timer::done.called.should.equal true
it "should flush to the history api", ->
@HistoryManager.flushChangesAsync
.calledWith(@project_id, @doc_id)
@HistoryManager.flushDocChangesAsync
.calledWithExactly(@project_id, @doc_id)
.should.equal true
describe "flushDocIfLoaded", ->
@ -243,6 +244,10 @@ describe "DocumentManager", ->
.calledWith(@project_id, @doc_id)
.should.equal true
it "should not flush the project history", ->
@HistoryManager.flushProjectChangesAsync
.called.should.equal false
it "should call the callback", ->
@callback.calledWith(null).should.equal true
@ -259,6 +264,11 @@ describe "DocumentManager", ->
.calledWith(@project_id, @doc_id)
.should.equal true
it "should not flush the project history", ->
@HistoryManager.flushProjectChangesAsync
.calledWithExactly(@project_id)
.should.equal true
describe "without new lines", ->
beforeEach ->
@DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, false, @callback

View file

@ -21,23 +21,6 @@ describe "HistoryManager", ->
@doc_id = "mock-doc-id"
@callback = sinon.stub()
describe "flushChangesAsync", ->
beforeEach ->
@HistoryManager._flushDocChangesAsync = sinon.stub()
@HistoryManager.flushProjectChangesAsync = sinon.stub()
@HistoryManager.flushChangesAsync(@project_id, @doc_id)
it "flushes doc changes", ->
@HistoryManager._flushDocChangesAsync
.calledWith(@project_id, @doc_id)
.should.equal true
it "flushes project changes", ->
@HistoryManager.flushProjectChangesAsync
.calledWith(@project_id)
.should.equal true
describe "_flushDocChangesAsync", ->
beforeEach ->
@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204)

View file

@ -9,6 +9,8 @@ describe "HttpController", ->
beforeEach ->
@HttpController = SandboxedModule.require modulePath, requires:
"./DocumentManager": @DocumentManager = {}
"./HistoryManager": @HistoryManager =
flushProjectChangesAsync: sinon.stub()
"./ProjectManager": @ProjectManager = {}
"logger-sharelatex" : @logger = { log: sinon.stub() }
"./Metrics": @Metrics = {}
@ -275,6 +277,11 @@ describe "HttpController", ->
.calledWith(@project_id, @doc_id)
.should.equal true
it "should flush project history", ->
@HistoryManager.flushProjectChangesAsync
.calledWithExactly(@project_id)
.should.equal true
it "should return a successful No Content response", ->
@res.send
.calledWith(204)
@ -293,6 +300,11 @@ describe "HttpController", ->
@DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArgWith(2, new Error("oops"))
@HttpController.flushAndDeleteDoc(@req, @res, @next)
it "should flush project history", ->
@HistoryManager.flushProjectChangesAsync
.calledWithExactly(@project_id)
.should.equal true
it "should call next with the error", ->
@next
.calledWith(new Error("oops"))

View file

@ -10,7 +10,8 @@ describe "ProjectManager - flushAndDeleteProject", ->
"./RedisManager": @RedisManager = {}
"./DocumentManager": @DocumentManager = {}
"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
"./HistoryManager": @HistoryManager = {}
"./HistoryManager": @HistoryManager =
flushProjectChangesAsync: sinon.stub()
"./Metrics": @Metrics =
Timer: class Timer
done: sinon.stub()
@ -30,13 +31,18 @@ describe "ProjectManager - flushAndDeleteProject", ->
@RedisManager.getDocIdsInProject
.calledWith(@project_id)
.should.equal true
it "should delete each doc in the project", ->
for doc_id in @doc_ids
@DocumentManager.flushAndDeleteDocWithLock
.calledWith(@project_id, doc_id)
.should.equal true
it "should flush project history", ->
@HistoryManager.flushProjectChangesAsync
.calledWithExactly(@project_id)
.should.equal true
it "should call the callback without error", ->
@callback.calledWith(null).should.equal true
@ -55,13 +61,18 @@ describe "ProjectManager - flushAndDeleteProject", ->
@ProjectManager.flushAndDeleteProjectWithLocks @project_id, (error) =>
@callback(error)
done()
it "should still flush each doc in the project", ->
for doc_id in @doc_ids
@DocumentManager.flushAndDeleteDocWithLock
.calledWith(@project_id, doc_id)
.should.equal true
it "should still flush project history", ->
@HistoryManager.flushProjectChangesAsync
.calledWithExactly(@project_id)
.should.equal true
it "should record the error", ->
@logger.error
.calledWith(err: @error, project_id: @project_id, doc_id: "doc-id-1", "error deleting doc")
@ -72,5 +83,3 @@ describe "ProjectManager - flushAndDeleteProject", ->
it "should time the execution", ->
@Metrics.Timer::done.called.should.equal true