From 62b8c30d0bd44f9bd995c35eb676e0547a68579d Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 1 Nov 2016 16:55:28 +0000 Subject: [PATCH 1/7] make pollSavedStatus more robust against failure check last ack timestamp and size of pending op provide method to compute sharejs op size so we can check if pending ops get too big --- .../public/coffee/ide/editor/Document.coffee | 25 ++++++++++++++++--- .../coffee/ide/editor/ShareJsDoc.coffee | 15 +++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/services/web/public/coffee/ide/editor/Document.coffee b/services/web/public/coffee/ide/editor/Document.coffee index 78431b8b6b..7b41efdc12 100644 --- a/services/web/public/coffee/ide/editor/Document.coffee +++ b/services/web/public/coffee/ide/editor/Document.coffee @@ -69,6 +69,12 @@ define [ getPendingOp: () -> @doc?.getPendingOp() + getRecentAck: () -> + @doc?.getRecentAck() + + getOpSize: (op) -> + @doc?.getOpSize(op) + hasBufferedOps: () -> @doc?.hasBufferedOps() @@ -143,24 +149,35 @@ define [ clearChaosMonkey: () -> clearTimeout @_cm + MAX_PENDING_OP_SIZE: 30 # pending ops bigger than this are always + # considered unsaved + pollSavedStatus: () -> # returns false if doc has ops waiting to be acknowledged or # sent that haven't changed since the last time we checked. # Otherwise returns true. inflightOp = @getInflightOp() pendingOp = @getPendingOp() + recentAck = @getRecentAck() + pendingOpSize = pendingOp? && @getOpSize(pendingOp) if !inflightOp? and !pendingOp? - # there's nothing going on + # there's nothing going on, this is ok. saved = true sl_console.log "[pollSavedStatus] no inflight or pending ops" else if inflightOp? and inflightOp == @oldInflightOp # The same inflight op has been sitting unacked since we - # last checked. + # last checked, this is bad. saved = false sl_console.log "[pollSavedStatus] inflight op is same as before" - else + else if pendingOp? and recentAck && pendingOpSize < @MAX_PENDING_OP_SIZE + # There is an op waiting to go to server but it is small and + # within the flushDelay, this is ok for now. saved = true - sl_console.log "[pollSavedStatus] assuming saved (inflightOp?: #{inflightOp?}, pendingOp?: #{pendingOp?})" + sl_console.log "[pollSavedStatus] pending op (small with recent ack) assume ok", pendingOp, pendingOpSize + else + # In any other situation, assume the document is unsaved. + saved = false + sl_console.log "[pollSavedStatus] assuming not saved (inflightOp?: #{inflightOp?}, pendingOp?: #{pendingOp?})" @oldInflightOp = inflightOp return saved diff --git a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee index 27d325676c..8b9623c270 100644 --- a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee +++ b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee @@ -46,6 +46,7 @@ define [ @_doc.on "change", () => @trigger "change" @_doc.on "acknowledge", () => + @lastAcked = new Date() # note time of last ack from server @trigger "acknowledge" @_doc.on "remoteop", () => # As soon as we're working with a collaborator, start sending @@ -101,12 +102,26 @@ define [ @connection.id = @socket.socket.sessionid @_doc.autoOpen = false @_doc._connectionStateChanged(state) + @lastAcked = null # reset the last ack time when connection changes hasBufferedOps: () -> @_doc.inflightOp? or @_doc.pendingOp? getInflightOp: () -> @_doc.inflightOp getPendingOp: () -> @_doc.pendingOp + getRecentAck: () -> + # check if we have received an ack recently (within the flush delay) + @lastAcked? and new Date() - @lastAcked < @_doc._flushDelay + getOpSize: (op) -> + # compute size of an op from its components + # (total number of characters inserted and deleted) + size = 0 + for component in op or [] + if component?.i? + size += component.i.length + if component?.d? + size += component.d.length + return size attachToAce: (ace) -> @_doc.attach_ace(ace, false, window.maxDocLength) detachFromAce: () -> @_doc.detach_ace?() From 87d8192d2961d439f4ad4627240f55c4b78c9dcd Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Nov 2016 12:25:36 +0000 Subject: [PATCH 2/7] decrease reconnect ratelimit for user activity --- .../web/public/coffee/ide/connection/ConnectionManager.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/connection/ConnectionManager.coffee b/services/web/public/coffee/ide/connection/ConnectionManager.coffee index 61d3d1c37d..0791e2ddc4 100644 --- a/services/web/public/coffee/ide/connection/ConnectionManager.coffee +++ b/services/web/public/coffee/ide/connection/ConnectionManager.coffee @@ -226,8 +226,8 @@ define [], () -> @lastConnectionAttempt = new Date() setTimeout (=> @startAutoReconnectCountdown() if !@connected), 2000 - MIN_RETRY_INTERVAL: 1000 # ms - BACKGROUND_RETRY_INTERVAL : 30 * 1000 # ms + MIN_RETRY_INTERVAL: 1000 # ms, rate limit on reconnects for user clicking "try now" + BACKGROUND_RETRY_INTERVAL : 5 * 1000 # ms, rate limit on reconnects for other user activity (e.g. cursor moves) tryReconnectWithRateLimit: (options) -> # bail out if the reconnect is already in progress From 2a8efea29a77b2b7aa83279d2338ec3ec4e2b86c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 3 Nov 2016 12:27:19 +0000 Subject: [PATCH 3/7] use browser 'online' event to trigger reconnect --- .../public/coffee/ide/connection/ConnectionManager.coffee | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/web/public/coffee/ide/connection/ConnectionManager.coffee b/services/web/public/coffee/ide/connection/ConnectionManager.coffee index 0791e2ddc4..3552c335df 100644 --- a/services/web/public/coffee/ide/connection/ConnectionManager.coffee +++ b/services/web/public/coffee/ide/connection/ConnectionManager.coffee @@ -20,6 +20,12 @@ define [], () -> @disconnectIfInactive() , ONEHOUR) + # trigger a reconnect immediately if network comes back online + window.addEventListener 'online', => + sl_console.log "[online] browser notified online" + if !@connected + @tryReconnectWithRateLimit({force:true}) + @userIsLeavingPage = false window.addEventListener 'beforeunload', => @userIsLeavingPage = true From 91a590ad0e33a6574cc7904c265916849e08d840 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 2 Nov 2016 16:29:14 +0000 Subject: [PATCH 4/7] update links from tender to wiki --- .../web/public/coffee/ide/connection/ConnectionManager.coffee | 2 +- services/web/public/coffee/ide/editor/EditorManager.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/connection/ConnectionManager.coffee b/services/web/public/coffee/ide/connection/ConnectionManager.coffee index 3552c335df..cc5087fa6b 100644 --- a/services/web/public/coffee/ide/connection/ConnectionManager.coffee +++ b/services/web/public/coffee/ide/connection/ConnectionManager.coffee @@ -100,7 +100,7 @@ define [], () -> @ide.socket.on "connect_failed", () => @connected = false $scope.$apply () => - @$scope.state.error = "Unable to connect, please view the connection problems guide to fix the issue." + @$scope.state.error = "Unable to connect, please view the connection problems guide to fix the issue." # We can get a "disconnect" event at any point after the # "connect" event. diff --git a/services/web/public/coffee/ide/editor/EditorManager.coffee b/services/web/public/coffee/ide/editor/EditorManager.coffee index 698604c9c7..105313ba4c 100644 --- a/services/web/public/coffee/ide/editor/EditorManager.coffee +++ b/services/web/public/coffee/ide/editor/EditorManager.coffee @@ -109,7 +109,7 @@ define [ @ide.reportError(error, meta) @ide.showGenericMessageModal( "Out of sync" - "Sorry, this file has gone out of sync and we need to do a full refresh.
Please see this help guide for more information" + "Sorry, this file has gone out of sync and we need to do a full refresh.
Please see this help guide for more information" ) @openDoc(doc, forceReopen: true) From e1b54322ee46f6442baedcc28abbc0b1a9b6300c Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 4 Nov 2016 10:18:33 +0000 Subject: [PATCH 5/7] fix whitespace --- services/web/public/coffee/ide/editor/Document.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/editor/Document.coffee b/services/web/public/coffee/ide/editor/Document.coffee index 7b41efdc12..255fc16704 100644 --- a/services/web/public/coffee/ide/editor/Document.coffee +++ b/services/web/public/coffee/ide/editor/Document.coffee @@ -149,8 +149,7 @@ define [ clearChaosMonkey: () -> clearTimeout @_cm - MAX_PENDING_OP_SIZE: 30 # pending ops bigger than this are always - # considered unsaved + MAX_PENDING_OP_SIZE: 30 # pending ops bigger than this are always considered unsaved pollSavedStatus: () -> # returns false if doc has ops waiting to be acknowledged or From c25395d80b8937398f20b01caa7bf9bf7308f064 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 4 Nov 2016 10:25:08 +0000 Subject: [PATCH 6/7] fix links to wiki --- .../web/public/coffee/ide/connection/ConnectionManager.coffee | 2 +- services/web/public/coffee/ide/editor/EditorManager.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/connection/ConnectionManager.coffee b/services/web/public/coffee/ide/connection/ConnectionManager.coffee index cc5087fa6b..afbc656a02 100644 --- a/services/web/public/coffee/ide/connection/ConnectionManager.coffee +++ b/services/web/public/coffee/ide/connection/ConnectionManager.coffee @@ -100,7 +100,7 @@ define [], () -> @ide.socket.on "connect_failed", () => @connected = false $scope.$apply () => - @$scope.state.error = "Unable to connect, please view the connection problems guide to fix the issue." + @$scope.state.error = "Unable to connect, please view the connection problems guide to fix the issue." # We can get a "disconnect" event at any point after the # "connect" event. diff --git a/services/web/public/coffee/ide/editor/EditorManager.coffee b/services/web/public/coffee/ide/editor/EditorManager.coffee index 105313ba4c..f0cbb4c048 100644 --- a/services/web/public/coffee/ide/editor/EditorManager.coffee +++ b/services/web/public/coffee/ide/editor/EditorManager.coffee @@ -109,7 +109,7 @@ define [ @ide.reportError(error, meta) @ide.showGenericMessageModal( "Out of sync" - "Sorry, this file has gone out of sync and we need to do a full refresh.
Please see this help guide for more information" + "Sorry, this file has gone out of sync and we need to do a full refresh.
Please see this help guide for more information" ) @openDoc(doc, forceReopen: true) From d2ba575d85950b5282381e1d562605bc12786ef2 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 4 Nov 2016 10:55:46 +0000 Subject: [PATCH 7/7] note that acks are for our own ops --- services/web/public/coffee/ide/editor/ShareJsDoc.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee index 8b9623c270..1d716ca9d4 100644 --- a/services/web/public/coffee/ide/editor/ShareJsDoc.coffee +++ b/services/web/public/coffee/ide/editor/ShareJsDoc.coffee @@ -46,7 +46,7 @@ define [ @_doc.on "change", () => @trigger "change" @_doc.on "acknowledge", () => - @lastAcked = new Date() # note time of last ack from server + @lastAcked = new Date() # note time of last ack from server for an op we sent @trigger "acknowledge" @_doc.on "remoteop", () => # As soon as we're working with a collaborator, start sending