Presence wait for sync #295
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM |
See discussion ably/realtime#522 Since we now keep the old transport active until it becomes idle, if a message hangs on the old transport resulting in it failing, when that happens it'll still be the active protocol, so deactivateTransport will requeue that message and clear the message from that protocol (allowing it to become idle and the upgrade transport to take over).
Four main changes: - Treat channelstate operations (attach/detach) and sync separately. This solves the bug where a sync ending would incorrectly remove the lock of a pending detach (#285). - Only set the lock once have actually sent an attaching or detaching message. The purpose of the mechanism is for connectionManager to decide when it's safe to upgrade. Before, channel might be inProgress and in the attaching state without any pending operations, eg if attached while the connection state is connecting. Since the upgrade transport now waits for nopending before activating, this caused a deadlock if an attach request timed out: checkPendingState is waiting for transport.active, which is waiting for nopending. Now, setting inProgress is only done once the a message has been sent. - Additionally we now start the statetimer when the message is sent (if it's not already running) - before, if an attach was queued due to not being connected, the timer would never be set, even on connected. - Checkpendingstate now happens a tick after transport.active, to give connectionManager finish activating the transport and setting the connection state (before, if not on upgrade, it would trigger while connection state was still 'connecting')
SimonWoolf
force-pushed
the
presence-wait-for-sync
branch
from
4955b0c
to
b2de580
Compare
SimonWoolf
added a commit
that referenced
this pull request
Jun 24, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Original by @paddybyers. I added 1fd032c to default it to true if the params object has no
waitForSyncmember.