Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log connectionDetails #294

Merged
merged 9 commits into from
Jun 24, 2016
Merged

Log connectionDetails #294

merged 9 commits into from
Jun 24, 2016

Conversation

SimonWoolf
Copy link
Member

@SimonWoolf SimonWoolf commented Jun 22, 2016

Fixes #293

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')
@paddybyers
Copy link
Member

LGTM

SimonWoolf added a commit that referenced this pull request Jun 24, 2016
@SimonWoolf SimonWoolf merged commit 3143a01 into master Jun 24, 2016
@SimonWoolf SimonWoolf deleted the log-details branch October 11, 2017 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants