From cde5144c4217837bc8d439a4eec421c37bd79cad Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 19 Nov 2015 09:46:56 +0000 Subject: [PATCH] Retry updates that have not been acknowledged. If we do not get a reply from the server acknowledging our update after 5 seconds, send it again. If it never got to the server, this is like normal. If the update got to the server, but we never received the ack then we need to rely on ShareJs's duplicate handling. We set the dupIfSource parameter on any retried updates which let ShareJs know that it's a dup if we already have an op with this version number and client id. The doc-updater and real-time services need changes to correctly send another ack only to the submitting client in the case of a duplicate update. --- .../public/coffee/ide/editor/Document.coffee | 12 ++++------- .../coffee/ide/editor/ShareJsDoc.coffee | 21 +++++++++++++------ 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/services/web/public/coffee/ide/editor/Document.coffee b/services/web/public/coffee/ide/editor/Document.coffee index d525c7f7bf..14d55b05e9 100644 --- a/services/web/public/coffee/ide/editor/Document.coffee +++ b/services/web/public/coffee/ide/editor/Document.coffee @@ -150,14 +150,10 @@ define [ wantToBeJoined: @wantToBeJoined update: update - if Math.random() < (@ide.disconnectRate or 0) - console.log "Simulating disconnect" - @ide.connectionManager.disconnect() - return - - if Math.random() < (@ide.ignoreRate or 0) - console.log "Simulating lost update" - return + if window.dropAcks? and Math.random() < window.dropAcks + if !update.op? # Only drop our own acks, not collaborator updates + console.log "Simulating a lost ack", update + return if update?.doc == @doc_id and @doc? @doc.processUpdateFromServer update diff --git a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee index 15ba95f56f..83f3d68a77 100644 --- a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee +++ b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee @@ -28,8 +28,13 @@ define [ @connection = { send: (update) => @_startInflightOpTimeout(update) + if window.dropUpdates? and Math.random() < window.dropAcks + console.log "Simulating a lost update", update + return @socket.emit "applyOtUpdate", @doc_id, update state: "ok" + # Unlike ShareJs, our connection.id never changes, even when we disconnect/reconnect. + # This gives this client a unique id used for detecting duplicates ops. id: @socket.socket.sessionid } @@ -90,7 +95,6 @@ define [ updateConnectionState: (state) -> @connection.state = state - @connection.id = @socket.socket.sessionid @_doc.autoOpen = false @_doc._connectionStateChanged(state) @@ -103,13 +107,18 @@ define [ attachToAce: (ace) -> @_doc.attach_ace(ace, false, window.maxDocLength) detachFromAce: () -> @_doc.detach_ace?() - INFLIGHT_OP_TIMEOUT: 10000 + INFLIGHT_OP_TIMEOUT: 5000 _startInflightOpTimeout: (update) -> - meta = - v: update.v - op_sent_at: new Date() timer = setTimeout () => - @trigger "op:timeout", update + # Only send the update again if inflightOp is still populated + # This can be cleared when hard reloading the document in which + # case we don't want to keep trying to send it. + if @_doc.inflightOp? + update.dupIfSource = [@connection.id] + @connection.send(update) + # TODO: Trigger op:timeout only when some max retries have been hit + # and we need to do a full reload. + # @trigger "op:timeout", update , @INFLIGHT_OP_TIMEOUT @_doc.inflightCallbacks.push () => clearTimeout timer